Hello all, it’s time for the weekly migration subsystem meeting. The meeting will take place in slack in various threads
This meeting:
➤ Is for core migrate maintainers and developers and anybody else in the community with an interest in migrations
➤ Usually happens every Thursday and alternates between 1400 and 2100 UTC.
➤ Is done on the #migration channel in Drupal Slack (see www.drupal.org/slack for information).
➤ Happens in threads, which you can follow to be notified of new replies even if you don’t comment in the thread. You may also join the meeting later and participate asynchronously!
➤ Has a public agenda anyone can add to. See the parent issue for an idea of the typical agenda.
➤*Transcript will be exported and posted* to the agenda issue. For anonymous comments, start with a :bust_in_silhouette: emoji. To take a comment or thread off the record, start with a :no_entry_sign: emoji.

Core migration issues:

Agenda Items

Add your items here.

0️⃣ Who is here today? Have you gotten enough exercise this week?

damienmckenna Damien. No, but a few morning walks I've taken have been fun.
benjifisher Benji, one of the maintainers of the migration subsystem. I have not even taken a walk all week. :disappointed: I did shovel a few inches of snow from my driveway 2 days ago.
quietone Vicki, Similar here, I've been waiting for my discount card to go swimming again.
Matroskeen Ivan (catching up in the morning). I’m doing ~10,000 steps / day. This is not enough, but there is at least some activity :man-shrugging:

1️⃣ What should we talk about today? Suggest topics here and I will add threads. I will also check for comments on the issue for today's meeting.

quietone #3193993: [meeting] Migrate Meeting 2021-01-28#comment-13978637  point 2
quietone #3051252: Upgrade path for Multiupload Filefield Widget and Multiupload Imagefield Widget
donquixote from my earlier comment:Is it possible that rollback on a node revision migration deletes random nodes instead of the actual revisions?e.g. to rollback a revision with vid = 123, it will instead delete the node with nid = 123.If this is indeed the case it would be quite a serious bug.. Something to add as an item to the meeting?(I have yet to create the issue in core) (edited)
benjifisher @donquixote: Let's start with the ordinary process. Create a bug report, with steps to reproduce, before bringing it up at a weekly meeting.
donquixote @benjifisher ok, makes sense.

2️⃣ Action items.

quietone NW: #2796755: [PP-1] Deprecate Migration::set() @quietone
quietone Remove migrate critical tag, #3055557: Role migration results in migration permissions that do not exist on the target site @quietone
quietone NW:  Explain why it is NW.  #3184650: ContentEntity migration source adds revision ID as source key, incompatible with Drupal 8.8 and earlier @benjifisher

3️⃣ Statistics

benjifisher Fixed since last week's meeting: 5 (not counting issues for meetings) including one Major.
benjifisher RTBC: 3, including one Major. All three have been updated in the last 15 days.
benjifisher NR: 34, including 7 that have been waiting for more than a month and 4 Major.

4️⃣ How can I help? Comment in this thread if you are looking for ways to help. Give us some idea of what you would like to do: documentation, code review, testing, project management, ...

quietone no discussion

5️⃣ Deprecate Migration::set()

benjifisher #2796755: [PP-1] Deprecate Migration::set()
benjifisher @quietone and I have been working on this issue, with some input from alexpott and longwave.
benjifisher What I want to talk about today is the effect on contrib modules: see #81, #83 on that issue.
benjifisher The following contrib modules may be affected by the deprecation:media_migrationlocation_migrationmigrate_upgrademigrate_tools
benjifisher @damienmckenna, I think I saw your avatar on one of those. Or maybe I am thinking of commerce_migration, which is not affected by this issue.
damienmckenna I've contributed to some of those but don't comaintain them.
benjifisher The most complicated updates are in migrate_tools. It uses set() on objects that are supposed to implementDrupal\migrate\Plugin\MigrationInterfaceThat is, they seem to be core Migration objects. If they are actually migrate_plus entities, then the code is not affected by this issue. @heddn, can you take a look?
quietone I searched for set with the first parameter and I suspect there are more contrib module affected.http://grep.xnddx.ru/search?text=set%28%27source%27&filename= and http://grep.xnddx.ru/search?text=set%28%27destination%27&filename=I haven't checked yet because, well this issue is wearing me down and I need to step away from it for a bit longer.

6️⃣ Role migration results in migration permissions that do not exist on the target site

benjifisher #3055557: Role migration results in migration permissions that do not exist on the target site
benjifisher If I understand the issue, the title should be "migrated" permissions or just "permissions" instead of "migration permissions".
benjifisher Latest comment (from alexpott):I've changed the @todo in #2571235: [regression] Roles should depend on objects that are building the granted permissions into a deprecation and added a special case for migrations. Therefore I'm re-framing this issue to be about the removal of the special case for migrations.(edited)
quietone I just want to confirm that this is 'migrate critical'
benjifisher That seems like a good idea, and it is not tightly coupled to the migration system.There might be other ways to get to a state where phantom permissions are granted to some users. It would be nice to have a way to clean them up.
benjifisher I do not think it is any sort of critical.
benjifisher Oh, but alexpott says it is Critical, so I guess it is. And he tossed it onto our issue queue.
quietone I understood from our earlier meeting the migrate critical was for issues that need to be done by a certain time, i.e. deprecations.
benjifisher OK, here is a half-baked idea.The problem is dependencies. Some permissions depend on configuration, like which content types have been defined. So we do not know what roles exist until at least the config migrations have run. But maybe some of those depend on the user migration.Solution: split the user migration into two parts. One part can be run early, and sets all properties except the roles. The second part updates existing users and only maps the roles.
benjifisher Right, "Migrate critical" applied to things we hoped to get done before D9 was released. Now that we seem to be starting a 3-year cycle for major releases, I think that no longer applies.If the issue's priority is Critical, then I think it is redundant to give it the "Migrate critical" tag.
quietone That is the answer I was hoping for.

7️⃣ Plugins file_mfw and image_miw not migrated

benjifisher #3051252: Upgrade path for Multiupload Filefield Widget and Multiupload Imagefield Widget
quietone This needs review!
quietone Or, manual testing is also helpful. (edited)
benjifisher Are those field widgets related to the D7 file_entity and media modules?
quietone I don't know. How to find out?
benjifisher Check the .info files?
quietone Looked at all the .info files and no they are not related.
benjifisher I am beginning to think that I am confusing these with some other modules.
quietone I think that is a sign that you need a walk :smile:

8️⃣ [Regression] ContentEntity migration source breaks basic IDMap contract since 8.9

benjifisher #3184650: ContentEntity migration source adds revision ID as source key, incompatible with Drupal 8.8 and earlier
benjifisher What was I thinking when I set it back to NW? I did not leave a comment. Did I not notice that the fail was from a test-only patch? I will have another look.

8️⃣.1️⃣ organizing migrations into directories (edited) 

benjifisher Sorry, I forgot to check for suggested topics on the issue for today's meeting.
benjifisher @Matroskeen suggests changing the plugin discovery so that it searches sub-directories of mymodule/migrations. He offers to work on the feature.
benjifisher My initial thought is that it is a good idea. We have lots of subdirectories of src/Plugin/migrate/source/, typically d6/ and d7/, so source plugins can be split into subdirectoties. Of course, those are defined in PHP, not YAML.
benjifisher Is there any reason to say this is not the sort of thing that should be in core? Does it belong in contrib for some reason?
quietone Since it is not needed by core then I would say put it in contrib.
benjifisher Even if it is 5 lines of code in core?
quietone Ah, the slippery slope. There will also be a test to write. Probably should ping mikelutz on this one.
Matroskeen I expect it'll require a few extra lines in plugin discovery. Not sure about the test strategy yet. Maybe just move some test migrations into d6, d7 subdirectories, as we do for plugins. (edited)

8️⃣.2️⃣ Documentation

benjifisher This issue comment explains how to handle content moderation: #3157105: Migrated content becomes Unpublished when Content Moderation Workflow exists for content type#comment-13830538
benjifisher Before Drupal 8, I think content moderation was in contrib. So I do not think we can add it to the core node migrations. @quietone suggested adding an item to https://www.drupal.org/docs/upgrading-drupal/known-issues-when-upgrading.... Any other suggestions?
benjifisher Looking at that page, I think there is a manually (un)maintained table of contents (ToC) at the top of the Body field and an automatically generated one in the sidebar. I think we should remove the first.
quietone I made the changes suggested.
Matroskeen First of all, it doesn’t matter whether it was content moderation in d6 / d7 version.Secondly, if the content migration is executed on d8 website with enabled content moderation, then content items will ended up being unpublished, because the status is ignored in this case.We should explicitly set content_moderation to “published” to migrate them published. (edited)
Matroskeen My first idea was: we should document this somewhere.Now I think that maybe we should handle this out-of-the-box.
Matroskeen When I did a custom migration, I thought it was fine. But only just a few days later, I noticed the content is unpublished and it made me do some research.

9️⃣ Wrap up

benjifisher Thanks for participating! I will update 2️⃣. Please continue to add comments in the threads. In 1-7 days, we will post a transcript for today's meeting.
quietone @benjifisher Thank you!

Participants:

benjifisher, damienmckenna, quietone, Matroskeen, donquixote

Comments

benjifisher created an issue. See original summary.

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented

I'm not sure if I would be able to attend the meeting, but I'd like to bring up the following questions/challenges:

1) I was writing a custom migration and wanted to organize my migration files by some groups/categories. My migrations are placed into /migrations directory of the custom module, so I tried to add sub-directories for each group, but it seems plugin discovery doesn't handle nested directories.
I'd like to work on this feature request but I need a green-light from migrate API maintainers. I'll also appreciate any tips/suggestions.

2) I came across this particular case: #3157105: Migrated content becomes Unpublished when Content Moderation Workflow exists for content type (migrating content with Content Moderation enabled). I wasn't sure why all migrated content is being "Unpublished", and noticed the following snippet: https://www.drupal.org/project/drupal/issues/3157105#comment-13830538

I think it's worth documenting somewhere, but not sure what is the best place. Probably yml files (e.g. d7_node.yml) can be a good option, but maybe somewhere else.

quietone’s picture

quietone commented

There is also the know issues page, would that be suitable?

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented

It might be. We could also add that snippet directly to yml files, so every time someone copies the original file to write its own migration, it would work out-of-the-box. @quietone, let's continue the discussion in the meeting. Hopefully, it'll a separate thread for that :)

quietone credited DamienMcKenna.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Active » Fixed

I was planning to review the action items. Since I did not do that yet, and it is almost time for the next meeting, I will just make sure that open items from the 2021-01-21 meeting are reviewed at today's meeting.

@quietone, thanks for posting the transcript.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.