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.
Problem/Motivation
I wrote a bunch of migrations which migrate a site from D7 to D8 and I need to duplicate meticulously every migration used by the migrate process plugin in migration_dependencies to make sure they run in the right order.
Proposed resolution
Make the mother migration plugin gather the dependencies from the plugin definition. Make it optional to avoid any headaches.
Remaining tasks
None.
User interface changes
None.
API changes
Less work, yay.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#19 | 2750639_13_17_interdiff.txt | 939 bytes | mikeryan |
#17 | 2750639_17.patch | 2.98 KB | chx |
Comments
Comment #2
chx CreditAttribution: chx at Smartsheet commentedComment #3
benjy CreditAttribution: benjy at PreviousNext commentedLooks good to me, remarkably simple really.
Comment #4
chx CreditAttribution: chx at Smartsheet commentedLet's add a test for iterator as well.
Comment #5
benjy CreditAttribution: benjy at PreviousNext commentedMore tests :)
Comment #8
chx CreditAttribution: chx at Smartsheet commentedApparently I forgot to git pull before I posted this...
Comment #10
chx CreditAttribution: chx at Smartsheet commentedCckMigration overrides getProcess and gets getMigrationDependencies into an infinite loop. Neat. Avoiding that is easy.
Comment #11
benjy CreditAttribution: benjy at PreviousNext commentedComment #13
chx CreditAttribution: chx at Smartsheet commentedGood ole' game: keeping up with HEAD.
Comment #14
mikeryanI like the idea, but... What happens with circular migration references?
article.yml:
blog.yml:
The optional dependencies are used in sorting, and I don't know how it would handle circular references - I think we at least need a test for that scenario. And I'm betting we will need to prevent it (do not add a dependency on another migration if that migration already depends on us, directly or indirectly).
Comment #15
chx CreditAttribution: chx at Smartsheet commentedin
Drupal\Component\Graph\Graph::depthFirstSearch
. And no, we do not need a test here. Adding a circular graph test toDrupal\Tests\Component\Graph\GraphTest
is not a bad idea but definitely a separate issue.Comment #16
mikeryanI would feel better testing circular dependencies, but that seems a little more involved than I first imagined... In attempting it, though, I noticed some cruft left over from the original test:
Unused migration still created.
This assertion is now redundant.
Comment #17
chx CreditAttribution: chx at Smartsheet commentedComment #19
mikeryanFinally checked back, interdiff looks good, back to RTBC.
Comment #20
catchComment #22
catchCommitted/pushed to 8.3.x and 8.2.x, thanks!