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

Next video meeting 2022-13-01 2100Z

The hope is that most or all of the maintainers will attend. We will try to focus on longer-term goals than in the weekly meeting.

0️⃣ Who is here today?

danflanagan8 Dan
benjifisher Benji, one of the maintainers of the Migrate API.
Akhildev CS hi
benjifisher @Akhildev CS: Is this your first time at one of these meetings? Welcome!Do you have anything you want to talk about?
Akhildev CS can you please suggest some migration tickets that I can contribute.!(if exists),[i think I was 2ed time here.]
mikelutz (he/him) Hey all
Matroskeen :wave: Ivan, one of the contributors (edited)
steinmb :wave: lurking only
srjosh :wave::skin-tone-2: yo
srjosh @Akhildev CS there is a thread (4️⃣) for that
quietone Hi, catching up.

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.

danflanagan8 This is a recent smallish feature request that I've reviewed and looks good from a code/testing standpoint. I tagged it for subsystem maintainer review: #3252562: In Callback Migrate process, document how to use functions that accept no argument as callable
danflanagan8 Not many takers today. I'll throw this one back out there: #3186449: Rolling back a migration implementing MigrationWithFollowUpInterface does not clear the generated follow up migrations from the cache.

2️⃣ Action items. To be added later.

3️⃣ Statistics

benjifisher Fixed since last week's meeting: none (not counting the issue for the meeting).
benjifisher RTBC: 11, none of which are Major, 4 of which have not been updated in a month.
benjifisher NR: 31, including 4 Major and 4 that have not been updated in more than three months.
benjifisher Google sheet for recording stats: https://docs.google.com/spreadsheets/d/1o0Rjlc1vnnLP5bM5P-SMMyGzqn7258hi...

4️⃣ 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, ...

benjifisher can you please suggest some migration tickets that I can contribute.!(if exists),@Akhildev CS: Let's discuss that in this thread.
benjifisher Are you interested in code review, manual testing, writing tests?
Akhildev CS ok,(sorry for that i wasn't much familiar with slack).yes, I have interested in that.
benjifisher Which one?
benjifisher There are 31 issues that need review: https://www.drupal.org/project/issues/drupal?status=8&categorie[…]onent=migration%20system
Akhildev CS code review& manual testing
Akhildev CS thankyou
benjifisher You could start with one of the newer ones, with not too many comments. The opposite of that is my responsibility, not a good first issue.
benjifisher Here is one for the Migrate Plus module that I would like to get reviewed: #3007709: Add XPath-style filtering ability in JSON data parser plugin.In my opinion, the current support for parsing JSON is not very good.
benjifisher If you have any questions, feel free to ask in the main channel. You can ping me, but there are also lots of other people who are willing to help.
Akhildev CS ok, that was great, Thankyou
quietone @Akhildev CS ping me int he channel as well when you have comments

5️⃣ Previous minutes.

benjifisher Is there anything to follow up? Anything need to be changed in those minutes?
benjifisher #3251316: [meeting] Migrate Meeting 2021-12-02 2100Z

6️⃣ Handling files when migrating from D6 or D7

benjifisher We discussed this question a few days ago in this channel.
benjifisher @dinarcon wrote https://understanddrupal.com/articles/migrating-files-and-images-drupal, but that is not specific to D7->D9 migrations.I think we should add a page to the guide https://www.drupal.org/docs/upgrading-drupal with options for migrating files.
benjifisher The options include
  • Custom migrations based on d7_file to create File and Media entities.
  • Migrate Media Handler (for files referenced in and Only local images are allowed. elements)
  • Migrate File Entities to Media Entities (use case?)
  • Media Migration (migrating from D7 Media)
srjosh -
  • Migrate File Entities to Media Entities (use case?). - This module allows you to migrate Drupal 8.0 file entities to Drupal 8.5 media entities using the migrate module

it's a very limited use case.

srjosh Migrate Media Handler also handles D7 File / Image fields > D8/9 media fields. (edited)
Steve I haven't had a chance to try MMH with multilingual file migrations like we were chatting about, but I'm going to dig into that today.
srjosh Looking forward to hearing your feedback!
Steve For the second step in the documentation
Media Reference: Using the Record Media Ref Plugin

This plugin fills in the field_original_ref data with a hashed file path. This allows later entity migrations to lookup media data by file path.

 process:
   'field_original_ref':
   -
     plugin: migration_lookup
     source: field_old_image
     migration: example_file
     no_stub: true
   -
     plugin: record_media_ref

It's not obvious to me where to process this new field. In my node bundle migrations, I'm just initially migrating the file field with

field_file_attachment:
    -
      plugin: sub_process
      source: field_file_attachment
      process:
        target_id: fid
        display: display
        description: description

so I don't (yet) have a separate media migration, I'm not really migrating media entities, until now I was first migrating the files, then attempting to migrate those into new media entities. Is this the wrong approach?

Steve I did also do a general file and file_instance migration.
srjosh @Steve that plugin's for some pretty specific use cases.  You probably won't need it.
Steve OK, I was just going through the instructions in the README.
srjosh perpetually on my todo list :wink:
Steve So, in my case, you would recommend just using the update_file_to_document plugin on a new media reference field, the value for the source key being the original file field, right?
Steve Is that going to take care of translated files?
srjosh I honestly don't know if it will work on translated files; I think it should.
srjosh but yes.  update_file_to_document is designed to take file field data and convert it to a document media data, doing all the intermediary steps.
Steve With a new media reference field called field_document_media, the following config did not import any media into that
field  field_document_media:    
    -    
      plugin: migration_lookup    
      source: field_file_attachemnt    
      migration: upgrade_d7_file    
      no_stub: true    
    -    
      plugin: update_file_to_document
Steve No errors.
srjosh ah - that's because field_file_attachment is probably an array.
Steve It's a file field.
Steve So, maybe something like-
      plugin: sub_process
-      source: field_file_attachment
-      process:
-        target_id: fid
-        display: display
-        description: description
Steve ?
srjosh the data in that field, by the time it gets to the process plugin, is probably an array.  Note that you used subprocess in your file migration code noted above.
Steve Right, I think I'll update that and try again, good catch.
srjosh well, you'll still need to do both the migration_lookup and the update_file_to_documen tin the sub process
Steve OK, let me see how far I get with this.
srjosh
field_document_media:
  plugin: subprocess
  source: field_file_attachment
  process:
    target_id:
      - 
        plugin: migration_lookup    
        source: field_file_attachemnt    
        migration: upgrade_d7_file    
        no_stub: true  
      -
        plugin: update_file_to_document<
Steve Is the source key necessary for the migration_lookup if I've already used it for the sub_process plugin in the pipeline?
srjosh oh sorry - yes, but it should be fid, not field_file_attachment
Steve Darn, I'm still not getting any media migrated.
Steve
field_document_media:
    -
      plugin: sub_process
      source: field_file_attachemnt
      process:
        target_id:
          -
            plugin: migration_lookup
            source: fid                                                                                                                                                       
            migration: upgrade_d7_file
            no_stub: true
          -
            plugin: update_file_to_document
Steve $value = (null) in the transform() method of the SubProcess class, which is not what I want.
srjosh field_file_attachemntdo you have a typo there?
Steve Yes!
srjosh :smile:
Steve Thank you. (edited)
Steve 'field_original_ref' not found (edited)
Steve I guess I still need that.
srjosh yeah,  you do need to add it; it's used for referencing internally to migrate media handler.  It can be deleted afterwards.  I have an update pending that will auto install that field on install; I think it's in the dev version already.  You will be able to delete it afterwards. It's not necessary to add the migrate ref process to anything.
Steve OK, just fired the migration, and at first look, it appears to have migrated the translated files! Woo hoo!
Steve It definitely did, this is brilliant. Thanks for going through this with me @srjosh, great work on the module!
srjosh thank you! (edited)
srjosh can you DM me the config you used for the translated files? I'd like to work that into the documentation.
srjosh in fact, if you open a documentation issue in the queue I'll make sure you get a contrib credit for it.
benjifisher :+1: for more documentation. That is a great way to show your appreciation!

7️⃣ In Callback Migrate process, allow functions that accept no arguments to be used as callable

benjifisher #3252562: In Callback Migrate process, document how to use functions that accept no argument as callable
benjifisher Nit: why the extra "blank" line
?+ * - no_args: (optional) Whether to pass no arguments to the callback. To be
+ *   used in cases where a function does not accept arguments, such as time().
+ *
  *
  * Examples:
  *
danflanagan8 Is this a feature worth having? It's kind of like the unpack_source thing but in the opposite direction, if you know what I mean. So it seems reasonable based on that.
danflanagan8 At the same time it's weird to have a process plugin that doesn't process anything
danflanagan8 Are there any other plugins that do nothing with $value?
benjifisher Are there any other use cases besides time()?
I think that base fields like created and changed will default to the current time if you do not enter anything for them. For a custom timestamp field, I think it depends on the default settings. But I have not checked.
benjifisher None of the commonly used process plugins ignore the value passed in. I would look at the list of plugins from core modules other than migrate .
danflanagan8 Native php functions with no arguments...rand() pi()
danflanagan8 ooo, phpinfo()
benjifisher ... since I took the trouble to compile that list: https://www.drupal.org/docs/8/api/migrate-api/migrate-process-plugins/list-of-core-migrate-process-plugins#s-process-plugins-p[…]-other-core-modules
danflanagan8 But there's nothing that prevents using a custom function declared in a .module file or a static function on a custom class that takes no params.
benjifisher Ha! The user_update_8002 process plugin ignores the value.
danflanagan8 My impression is that the use cases may be limited, but that it would be undisruptive and technically consistent with the addition of unpack_source. If that option didn't exist, the no_args thing would seem weirder to me.
danflanagan8 user_update_8002And I use that one all the time!
benjifisher Also user_update_7002.
benjifisher Just to summarize the situation:
  • The feature request is not needed for any of the core migrations.
  • It does not add much complexity.
  • Is it useful enough to justify adding it to core?
  • Is there any other way to handle the suggested use cases?
danflanagan8 I think that's perfect.
benjifisher Why does time('') generate a warning? I thought that PHP was happy to ignore additional arguments to a function. Does that apply only to user-defined functions?
danflanagan8 I just tested in a sandbox with php8.0.0 nd got this:
<br />
<b>Fatal error</b>:  Uncaught ArgumentCountError: time() expects exactly 0 arguments, 1 given in [...][...]:4
Stack trace:
#0 [...][...](4): time('')
#1 {main}  thrown in <b>[...][...]</b> on line <b>4</b><br />
danflanagan8 Back in php5 it looks like the extra arg was ignored
danflanagan8 I guess that's "progress"
benjifisher
php > function foo() { return time(); }
php > echo foo();
1639060902
php > echo foo('');
1639060906

That is PHP 7.4.

benjifisher I get something similar with PHP 8.0.
danflanagan8 What!? But try echo time(''). It freaks out. So weird.
benjifisher That suggests that I was right about user-defined functions.
danflanagan8 Ah, good point
benjifisher Luckily, @Matroskeen nudged @heddn to add the service plugin to the migrate_plus module. (Thanks to both of you!) So I bet we can use
field_timestamp:
  plugin: service
  service: datetime.time
  method: getCurrentTime # or getRequestTime
benjifisher I will comment on the issue.
danflanagan8 thanks
mikelutz (he/him) Really the whole thing is moot, since you can pass no arguments using unpack_source and a source that is an empty array…
process:
  time:
    plugin: callback
    callable: time
    unpack_source: true
    source: {  }

Should work just fine with the existing code…

danflanagan8 I'll test that
benjifisher I will add that to my comment.
mikelutz (he/him) If it didn’t work, (and it does) we would be better off making that format work then adding a new configuration key.  And we should probably add a test for it, though we can’t really test against the return value of time(), and I’m not sure what else to test against off the top of my head.
benjifisher Maybe pi(). That is what the current patch uses in its tests. Was that your idea, @danflanagan8?
danflanagan8
If it didn’t work, (and it does)

In case there are any doubters, my test confirmed it works

danflanagan8
Was that your idea

No

danflanagan8 But I like that idea
danflanagan8
'pi' => [
  'pi',
  [],
  pi(),
],
danflanagan8 That can be added to the data provider
mikelutz (he/him) I would use this issue to add that to the test, and update the docBlock in callback to give an example with no arguments and call it good.
danflanagan8 Agreed.
mikelutz (he/him) I can comment, if you haven’t yet, @benjifisher
benjifisher I just submitted my comment. :+1: for repurposing the issue. The guy who opened the issue deserves an issue credit.
mikelutz (he/him) I added a comment as well and changed the issue title.
mikelutz (he/him) I updated the issue summary as well.
mikelutz (he/him) Aaaaaaand I updated the original unpack_source change record with an example of calling with no arguments.
mikelutz (he/him) And I went ahead and added the documentation and test into a new MR.  It NR if anybody has time.
danflanagan8 It looks good. I'll RTBC it after the tests pass.
mikelutz (he/him) Sounds good, FYI, it’s safe to RTBC something while tests are running if it is simple and you are confident.  If the test do end up failing, the system will set it back to NW automatically.  I usually just leave a comment of “LGTM, setting to RTBC pending tests” or something like that.
danflanagan8 Cool. Done.
srjosh accomplishment!
mikelutz (he/him) I love the easy ones, lol.

8️⃣ ContentEntity source plugin shouldn't throw exception when the bundle key is missing: unavoidable in rollback situations

benjifisher #3186449: Rolling back a migration implementing MigrationWithFollowUpInterface does not clear the generated follow up migrations from the cache.
benjifisher Thanks for the reminder, @danflanagan8. I still owe you a response.

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.

Comments

quietone created an issue. See original summary.

benjifisher’s picture

From Slack:

@dinarcon wrote https://understanddrupal.com/articles/migrating-files-and-images-drupal, but that is not specific to D7->D9 migrations.

I think we should add a page to the guide https://www.drupal.org/docs/upgrading-drupal with options for migrating files.

The options include

quietone’s picture

#2 sounds good to me. File migration is a common topic in #migration.

quietone’s picture

The new file migration doc page could include information from #3022910: Prevent migrated files from having an incorrect value at file_managed.filename and then that can be closed.

quietone’s picture

Issue summary: View changes

quietone credited Steve.

quietone credited srjosh.

quietone’s picture

Issue summary: View changes

quietone credited mikelutz.

quietone’s picture

quietone’s picture

Status: Needs review » Fixed

There have been no requests for changes to the minutes in following meetings. Closing as fixed.

Status: Fixed » Closed (fixed)

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