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:

0️⃣ Who is here today? Jelly beans: is licorice the best or the worst?

benjifisher Benji, one of the maintainers of the migration subsystem. Licorice jelly beans are the best!
gaurav mahlawat Hi, Gauravmahlawat from India :flag-in:
benjifisher Would you like to be added to the list of people who are notified of the meeting?
gaurav mahlawat Yes please.
benjifisher Done.
alison Alison here! All licorice is super gross :stuck_out_tongue_closed_eyes:
damienmckenna Damien. i don't mind licorice.
Matroskeen Ivan from Ukraine :wave:
quietone Vicki, I like licorice tea (I rarely taste a jelly bean).

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.

damienmckenna The flip side of the docs issue.. if you're using hook_migrate_prepare_row() what's the best method to modify the $row object?
Matroskeen #3189876: Add documentation for file source plugins

2️⃣ Action items. To be added later.

quietone NW: #2579361: Row->setSourceProperty() docs are misleading @quietone
quietone NR: #3063856: Add ability to view migrate_message table data @benjifisher
quietone NW: #3175953: Cleanup migrate drupal functional tests @quietone
quietone NW: #2687849: Add back rollbacks on migrate_drupal_ui @quietone

3️⃣ Statistics

benjifisher Fixed since last week's meeting: 1 (not counting the issue for the meeting).
benjifisher RTBC: 3, all Normal priority. 2 of the 3 have been waiting more than 3 weeks.
benjifisher NR: 38, including 4 Major and 9 that have not been updated in more than a month.
benjifisher One more issue moved from RTBC to Fixed during the meeting!

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, ...

Matroskeen Just in case anyone is looking for help, we have a meta issue for the documentation: [#3189481]Volunteers needed! :slightly_smiling_face:

5️⃣ Row->setSourceProperty() docs are misleading

benjifisher #2579361: Row->setSourceProperty() docs are misleading
benjifisher This is a documentation issue. The doc block is what will show up on api.drupal.org (a.d.o). Here is the current proposal: * The migration iterates over rows returned by the source plugin. This * method determines the next row which will be processed and imported into * the system. A row will be processed if it has not already been imported, or * the row needs an update, or it is above the highwater mark or the source * row has changed. A row is considered changed only if track_changes is set * on the source plugin and the source values for the row have changed since * the last import. When set to be processed the row is also marked frozen and * no further changes to the row source properties are allowed. * * The method tracks the source and destination IDs using the ID map plugin.
damienmckenna OMG, that explains so many problems I've ran into :exploding_head:
benjifisher I propose this: * The migration iterates over rows returned by the source plugin. This * method determines the next row that will be processed and imported into * the system. A row will be processed in any of these cases: * - the row has not already been imported * - the row needs an update * - the row is above the highwater mark * - the source row has changed * A row is considered changed only if track_changes is set * on the source plugin and the source values for the row have changed since * the last import. When set to be processed, the row is also marked frozen and * no further changes to the row source properties are allowed. * * The method tracks the source and destination IDs using the ID map plugin.Set off the cases as a bulleted list.Change "which" to "that".Add a comma.I did not re-wrap the text at the end.
benjifisher Context: this is the doc block for the SourcePluginBase::next() method. I think. Then Row::setSourceProperty()  gets an @see reference to that, which shows up as a link on a.d.o.
benjifisher I added my proposal as a comment and set the issue to NW.

6️⃣ The flip side of the docs issue.. if you're using hook_migrate_prepare_row() what's the best method to modify the $row object?

benjifisher Is that hook called before or after next() marks the row as frozen?
damienmckenna I'd need to test it and don't have time today..
quietone The hook is called in \Drupal\migrate\Plugin\migrate\source\SourcePluginBase::prepareRow which is called just before the logic that can set the row frozen

7️⃣ Add documentation for file source plugins

benjifisher #3189876: Add documentation for file source plugins
Matroskeen It turns out writing documentation is helpful, because it reveals some interesting things.It was found out that source_base_path  constant is kind of “required”, because the d7_file plugin uses it, but I noticed many people are ignoring it in custom migrations. I posted a link to random blogpost, but I seen already several migrations with another strategy.Also, in my custom migrations I was using it differently. (edited)
Matroskeen I was going to revisit it again, but perhaps someone else can step in and clarify the usage of this constant?
benjifisher To the point about documentation: Jen Hodgdon maintains the user guide, and that leads her to notice various UX fails that need to be fixed.
benjifisher source_base_path is supposed to be the root of your source site, eitherthe file path to a local copythe URL to some other copy or the live site
benjifisher Then the base path, site/default/files by default, is added to source_base_path.
Matroskeen In my first custom migration, I put files into sites/default/files/migrate of my destination site.In this case source_base_path  didn’t make a lot of sense :man-shrugging: (edited)
benjifisher This means that, for a local copy of the files, you are required to make a subdirectory like sites/default/files/ , which I find annoying. That is why we have #3159217: Be more flexible in where files are staged for migration
benjifisher x-post
Matroskeen It’s helpful, thanks.Just one more thing, do you know cases when this is possible?https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/modules/file... mean, the absolute path after replacing the schema. (edited)
benjifisher Sorry, I do not know. It does seem overly complicated, doesn't it?
Matroskeen It looks complicated and not obvious. That’s why I’m trying to bring enough information to document it properly.
quietone The source plugin test \Drupal\Tests\file\Kernel\Plugin\migrate\source\d7\FileTest has an example of an absolute path. The private path is '/home/lillian/subdomains/u2/u2-private-files', which means the source_base_path is '/'.

8️⃣ ContentEntity migration source adds revision ID as source key, incompatible with Drupal 8.8 and earlier

benjifisher #3184650: ContentEntity migration source adds revision ID as source key, incompatible with Drupal 8.8 and earlier
benjifisher This issue needs manual testing. If you have a content_entity migration in Drupal 8.8 or earlier that broke in more recent versions, then try out the patch (and update your migratrion config).

9️⃣ Expose full set of debugging data in migrate_message table (filterable/searchable)

benjifisher #3063856: Add ability to view migrate_message table data
benjifisher NR: I should do this.

1️⃣0️⃣ Deprecate Migration::set()

benjifisher #2796755: [PP-1] Deprecate Migration::set()
benjifisher Previously RTBC, this issue NW to update the tests after recent changes.
benjifisher Or tell me that my objections are silly and we should keep the existing tests.
benjifisher If this issue is fixed, then we will need to update some contrib modules. The most complicated is Migrate Tools. Although $migration->set(...) is usually applied when $migration is a config entity, there are many places where it is declared as \Drupal\migrate\Plugin\MigrationInterface . @heddn, can you look at this?

1️⃣1️⃣ Migrate commands are now part of Drush core

benjifisher https://gitlab.com/drupalspoons/migrate_tools/-/issues/118
benjifisher Drush 10.4.0-rc1 was released about a month ago, and it includes migration commands. We probably need to update Migrate Tools and/or Migrate Plus.

1️⃣2️⃣ 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.

Participants:

benjifisher, gaurav mahlawat, alison, damienmckenna, Matroskeen, quietone

Comments

quietone created an issue. See original summary.

quietone credited DamienMcKenna.

quietone credited Gauravmahlawat.

quietone credited Matroskeen.

quietone credited alisonjo315.

quietone credited benjifisher.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
benjifisher’s picture

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

Status: Fixed » Closed (fixed)

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