Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The final stage of #2507607: [META] Replace Cthulhu-forsaken load plugins with migration builders...and just what it says. All load plugins and related infrastructure should be removed from Migrate. This patch does just that -- 1,311 deletions!
Postponed on #2549003: Create a builder for the d6_profile_values migration.
Comment | File | Size | Author |
---|---|---|---|
#34 | interdiff-2549013-33-34.txt | 1.42 KB | phenaproxima |
#34 | 2549013-34.patch | 66.39 KB | phenaproxima |
#33 | interdiff-2549013-25-33.txt | 4.54 KB | phenaproxima |
#33 | 2549013-33.patch | 66 KB | phenaproxima |
#25 | interdiff-2549013-23-25.txt | 2.97 KB | phenaproxima |
Comments
Comment #2
phenaproximaUnblocked!
Comment #4
phenaproximaRe-rolled.
Comment #6
phenaproximaHere we go...fixed a lot of nutty test failures to get to this point, but this patch ices load plugins for-ev-ah!
Comment #7
phenaproximaUm -- this patch.
124 insertions. 1,507 deletions. Yow!
Comment #9
phenaproximaTrying to fix test failures.
Comment #12
phenaproximaRolled in #2552277: Migration d6_comment did not meet the requirements. in the hopes that it will pass testbot...
Comment #15
phenaproximaTurns out that a patch must have a blank line at the end. Go figure.
Comment #16
phenaproximaThis patch is blocked by #2538000: The d6_node plugin should fetch CCK field values because it removes the d6_cck_field_values and d6_cck_field_revision migrations.
EDIT 8/25/15: Both migrations were removed in the aforementioned patch. It made sense, since that patch switches out the load plugin-based CCK handling for something more static and builder-friendly.
Comment #17
phenaproximaPostponing on #2538000: The d6_node plugin should fetch CCK field values.
Comment #18
benjy CreditAttribution: benjy at PreviousNext commentedDon't know about using the word "variant" for these migrations, given we have display variants in Drupal already it could be confusing?
Has all the code from this source already been moved into a builder?
This is a scary looking patch, I'd like to see someone test this on an a real site before getting committed similar to how i did here: https://codedrop.com.au/blog/video-drupal-6-drupal-8-migration
Comment #19
phenaproxima#1: I'm open to changing it, but I'm not sure what word to use. Migrate is far removed enough from display variants, IMO, that the terminology will not collide.
#2: The code from that plugin will not go into a builder, but you make a good point -- #2538000: The d6_node plugin should fetch CCK field values oughta include one in order to prevent a regression.
Agreed that the patch should be tested on a full-site migration before commit.
Comment #20
mikeryanComment #21
mikeryanComment #22
phenaproximaUnblocked.
Comment #23
phenaproximaRe-rolled to reflect what was done in #2538000: The d6_node plugin should fetch CCK field values. This patch was revealing a few left-over bugs with CCK handling, so those are fixed herein as well.
Comment #25
phenaproximaInsert dopey Gandalf jokes here.
Comment #26
mikeryanComment #27
mikeryanI'll be testing this with a real site using migrate_upgrade as well as reviewing the code, but a patch this big should get benjy's oversight as well.
Comment #28
phenaproximaThis is a complex patch that touches a lot of little out-of-the-way corners of Migrate, so huge +1 for @benjy's sign-off. I will also be testing it with a real site.
Comment #29
mikeryanRunning migrate_upgrade on my D6 site, things went well until migrating a node type with an image field:
Comment #30
phenaproximaI tried to reproduce that but was unable to. Can you send me a copy of the site you're migrating?
Comment #31
benjy CreditAttribution: benjy at PreviousNext commentedI think this is looking pretty good reviewing the code. Still NW until #29 is fixed.
A couple of small nitpicks, quite small since we discussed much more in IRC.
This doesn't do anything?
s/schemata/schema
Sounds like we want a follow-up to discuss?
Comment #32
phenaproximaComment #33
phenaproximaFixed #29 and #31-2! Let's get this DONE!
Comment #34
phenaproximaAdded a try-catch to CckFile to deal with the possibility of the Migration process plugin trying to skip entire rows.
Comment #35
mikeryanComment #36
mikeryanOh, should have said - interdiffs look good, last patch works fine with my site (no fatal, node with the dangling file reference successfully migrates).
Comment #37
benjy CreditAttribution: benjy at PreviousNext commentedLooks good.
Comment #38
webchickWalked through this with @phenaproxima, and all looks good from my side, other than it's missing a change record. However, Adam has sworn a Blood Oath that this will be written immediately after we're done talking. ;)
Here are some things that came up in review, nothing blocking.
WAT? :)
Adam explained that this is basically mocking the expected results. The reason this is needed is because now we're migrating a story specifically rather than nodes as a whole.
Uh.. wat? :) Good to see this cleaned up.
I asked about this, this special handling is unique to file fields, only because sometimes those can refer to files that have failed to migrate, and are thus prone to causing issues for remaining migrations, so we do a safety switch here.
Need to make sure we warn people in the change record that we no longer use these files and to use builders now instead.
Same deal; lets make sure this is part of the change record.
Had a question about this, this is just moving code to where it belongs.
Why rename these? The old name was much clearer.
Rationale: getVariantIds() also used for loading, not just dependencies.
Change record.
This was migrate drupal formerly overriding some classes, both of which are gone now, so this is awesome to remove. :)
This was Migrate Drupal's custom override of Migrate's class, because load plugins were needed. Now we don't need those, so we can just use the parent class. Less code FTW!
Change record.
Committed and pushed to 8.0.x. Thanks!
Comment #40
phenaproximaChange record added: https://www.drupal.org/node/2558851