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
|
| srjosh | -
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
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 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
|
| 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-
|
| 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 |
|
| 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 |
|
| 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
|
| 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:
|
| 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:
|
| danflanagan8 | Back in php5 it looks like the extra arg was ignored |
| danflanagan8 | I guess that's "progress" |
| benjifisher |
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
|
| 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…
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 |
|
| 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
Comment #2
benjifisherFrom Slack:
The options include
d7_fileto create File and Media entities.<a>and<img>elements)Comment #3
quietone commented#2 sounds good to me. File migration is a common topic in #migration.
Comment #4
quietone commentedDecide if these should be added to core
Comment #5
quietone commentedThe 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.
Comment #6
quietone commentedComment #10
quietone commentedComment #12
quietone commentedComment #13
quietone commentedThere have been no requests for changes to the minutes in following meetings. Closing as fixed.