Problem/Motivation
Created a list of Configuration migrations to run using 'drush migrate:template:list --tag=Configuration' and edited out the drupal 6 configuration. Then ran that using drush mm using a Drupal 7 source. I was surprised to see 'd7_user' being run. d7_user is definitely Content. Yet there is was.
So, grepped for migrations with d7_user and Configuration and found 6 migrations, 5 which are valid say, d7_user_role, which is configuration. That left d7_shortcut_set_users, which is tagged Configuration since it is a config entity but is dependent on d7_user which is Content.
That means that when one gets a list of Configuration migrations to run you also have to check the dependencies to see what will actually run. That is certainly extra work. I don't know if other Configuration migrations are dependent on Content migration as it is still running. And I don't see a way around this since ShortcutSet is a config entity. Well, it could be move to Content since it is dependent on content. I don't know. At least document this behavior. Note the original issue did not add documentation #2711099: Categorize migrations according to their type
Marking as Task since this seems more likely to be covered in documentation.
id: d7_shortcut_set_users
label: Shortcut set user mapping
migration_tags:
- Drupal 7
- Configuration
source:
plugin: d7_shortcut_set_users
process:
uid:
-
plugin: migration_lookup
migration: d7_user
source: uid
-
plugin: skip_on_empty
method: row
set_name:
plugin: migration_lookup
migration: d7_shortcut_set
source: set_name
destination:
plugin: shortcut_set_users
migration_dependencies:
required:
- d7_shortcut_set
- d7_userProposed resolution
A) Tag Configuration migrations as Content if they depend on Content
B) Document the yml where this occurs and maybe on d.o where ever the Content/Configuration tags are explained
This occurs in 3 configuration migrations
- d6_block.yml
- d7_block.yml
- d7_shortcut_set_users.yml
Remaining tasks
Patch
review
commit
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3014629-11.patch | 1.29 KB | quietone |
Comments
Comment #4
xjmComment #5
wim leersComment #6
wim leersComment #7
wim leersLinked here from #2711099-107: Categorize migrations according to their type since this is technically a follow-up to that. Also updated the change record at https://www.drupal.org/node/2944527 to point to this issue too, since it's crucial information that may otherwise lead to significant lost time and confusion.
Comment #11
quietone commentedNo running tests because this is only documentation.
Comment #13
xurizaemonLGTM @quietone, and thanks - I learned a thing here :)
Comment #14
quietone commented@xurizaemon, thanks for the RTBC!
I've started the tests.
Comment #15
mikelutzI confess, I don’t completely understand the need here. The dependencies for the migrations are already self documented in the yml. I didn’t realize that some people had an expectation when we tagged the migrations as config or content that there wouldn’t be any config migrations that were dependent on content, and I think that underlying assumption is just flawed.
There’s always been config that depends on content in Drupal, with the block system being the obvious one in core, but I think there are more examples in contrib, though I confess I can’t think of one off the top of my head. Yes, most of the time, configs aren’t dependent on content, but it’s never been a hard and fast rule. I don’t oppose to adding the note here, but I wonder if we shouldn’t do more. We should make sure anywhere we have documentation on the content and config migrations that we point out that there is nothing that prevents config migrations from depending on content migrations either in core or contrib if this misunderstanding is prevalent.
Comment #16
quietone commented@mikelutz, this came about because of #2826742: Expose migration types in Migrate UI where the goal is to be able to run only configuration or only content migrations from the UI. I was unaware of that content depended on config so it surprised me and thought it is worth documenting in the migrations.
I do agree that adding more documentation is a good idea. I'd rather see a new issue that addresses the situation for all of core. I don't have a recommendation as to where to put such documentation.
Comment #18
catchAgree with @mikelutz here except if you hadn't realised this, I think it could be very confusing. So let's add the specific docs here and then figure out somewhere more generic to put a general note in a follow-up.