[meeting] Migrate Meeting 2021-12-23 1400Z [#3255041]

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-01-13 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? Why are you working Xmas week?

danflanagan8	I'm here. I'm not really working. Migrate is fun!
Matroskeen	:wave: Ivan is here.I'm working because we celebrate Christmas at January 7. Here is why. (edited)
benjifisher	Benji here. I am working today because someone should "keep the lights on". Being non-Christian (in a secular but majority Christian country) I figure I will take my vacation some other week.I agree that Migrate is fun!
mikelutz (he/him)	Here, late. Not working this week, or next week. Back to work on the 3rd.
heddn	Lucas here. I'm working on a migration using Soong this week. Next week I'll be off. Celebrating with family and doing some small projects around the house.
quietone	Vicki. I will be around. Otherwise, gardening and small projects for home and Ngaio Playcentre.

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	Here's an issue for which I found some possible duplicates. They are the "Referenced By" issues. #3253959: Menu links migration should run after terms are migrated
danflanagan8	Would a maintainer be interested in closing this one? I made a detailed comment. #2789125: Error when a process plugin gives a multiple value for a subproperty destination
danflanagan8	Here's one I want to discuss with @Matroskeen! #3223182: The "dedupe_entity" plugin does not exist
damienmckenna	I want to offer a thanks to @quietone for the recent work on getting Commerce Migrate up to date, it'll greatly help working on other issues. :clap::clap::clap:
benjifisher	You could say that in the #drupal-thanks channel.

2️⃣ Action items. To be added later.

3️⃣ Statistics

benjifisher	Fixed since last week's meeting: 0 (not counting the issue for the meeting).
benjifisher	RTBC: 9, none of which are Major and 3 that have not been updated in more than a month.
benjifisher	NR: 28, including 4 Major and 2 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, ...

5️⃣ Previous minutes.

benjifisher

Is there anything to follow up? Anything need to be changed in those minutes?

6️⃣ Some RequirementsException messages are not visible in Drush logs

benjifisher	#3254986: Remove requirements array from RequirementsException message
Matroskeen	I believe we should agree on the format of requirements message, returned by RequirementsException::getRequirementsString(). With current approach in the merge request, the result of drush ms -vvv for d7_comment migration with misssing requirements would be: `Migration 'foo' is skipped as its source plugin has missed requirements: missing_source_module: comment` Which is not handy I think :face_with_rolling_eyes: I believe that exception message is a better fit, which is: `The module comment is not enabled in the source site(edited)`
danflanagan8	To me what would be awesome would be to define a set of constants instead of using a camel case string to define the type of requirement that is missing. I think the we'd be able to make the wording of the message be determined by the constant. I'm probably not making much sense though.
Matroskeen	RequirementsException was introduced in [#2321609]Is it helpful to add a comment in the original issue? Maybe someone is still around and will reply.
benjifisher	That issue was opened and closed (fixed) in 2014. Looking at the description, I think it was a big step in the right direction: Migration d6_node did not meet the requirements is not very actionable.
benjifisher	Add a comment to #3254986 and maybe add #2321609 as a related issue.
danflanagan8	I was still a physics teacher in 2014, but my impression is that migrate was under furiously fast development at that point.

7️⃣ Intermittent test failures

benjifisher	A few (3?) of our RTBC issues have a test failure like this one: https://www.drupal.org/pift-ci-job/2273025
benjifisher	We should add a new child issue to #2829040: [meta] Known intermittent, random, and environment-specific test failures
danflanagan8	Yeah, I have like 6 of those in my inbox
Matroskeen	I think more than 30 issues were moved to "Needs Work" today because of these failures.The reason is probably here: #3255749: Composer v2.2 prompts to authorize plugins (edited)
benjifisher	Oh, I think I saw that locally.
benjifisher	It was a fresh install of DDev. Maybe it was using Composer 2.2.0.
benjifisher	I see the issue is already marked Critical. :+1:. I added a comment to the meta issue for intermittent failures.

8️⃣ Menu links migration should run after terms are migratedPrimary tabs

benjifisher	#3253959: Menu links migration should run after terms are migrated
benjifisher	@danflanagan8 wrote,Here's an issue for which I found some possible duplicates. They are the "Referenced By" issues.
xurizaemon	I haven't engaged with that issue, but i feel like the description might be missing a "not"? Running a D7 -> D9 site migration (using migrate_upgrade) results in the menu links migration failing to identify that it should run before taxonomy term migrations, so menu links that point to taxonomy term pages end up failing. The menu migrations should not run before taxonomy migrations, right?
xurizaemon	("failing to identify that it should not do x" is a bit of a double negative but hopefully that makes sense)
danflanagan8	I think you're right. The title is correct.

9️⃣ Error when a process plugin gives a multiple value for a subproperty destination

benjifisher	#2789125: Error when a process plugin gives a multiple value for a subproperty destination
danflanagan8	I'm thinking a maintainer might be interested in closing this one. I left a detailed comment.
benjifisher	I will have a look.
benjifisher	I have only read the IS so far and already I am thinking "won't fix".
danflanagan8	Well then that combined with a readily available workaround and no action for a few years sounds like a slam dunk.
benjifisher	Did I miss your announcement of https://www.drupal.org/project/migrate_sandbox?
danflanagan8	I only spammed the channel twice!
danflanagan8	It's awesome though. Extremely fast triage possibilites
benjifisher	I am flattered that you use the transpose plugin. Maybe I should write a CR for that.
danflanagan8	Did you add transpose to migrate_plus?
danflanagan8	I am humbled to be in your Slack presence
benjifisher	Did you look at the implementation? I gave credit to StackOverflow in the comments.
benjifisher	If you add a child page to https://www.drupal.org/docs/8/api/migrate-api/migrate-process-plugins with the example in your comment, then I will add it to the guide.
benjifisher	Definitely "won't fix". The only question in my mind is whether we can provide a helpful error message. Then the follow-up question is whether to repurpose the existing issue for that, like we did for #3252562: In Callback Migrate process, document how to use functions that accept no argument as callable, or to make a new issue.
quietone	Not sure how we can improve the error message. To do so implies that migrate will be validating the values on the destination row. The responsibility for that is with the entity not migrate. Unless, of course, I have got this completely wrong.
benjifisher	I have not yet thought about it, but the Migrate API "knows" when you are using field/property notation like body/format, right? Can it issue a warning when you try to assign multiple values to that? Make a :paperclip: pop up and say, It looks like you are assigning multiple values to a field property. Maybe you should try using the sub_process plugin instead! This might end up adding as much complexity as the patch on the issue. If so, then it is time to give up.
quietone	It seems to me that to know that it is really a field and not something else someone has on the destination row you would have to look at the entity itself. And then we are back to doing validation that the entity does, solely to provide a migratey type error message.Is the gain worth the work and maintenance? I don't think so. And even more so when there are 1 Critical and 3 migrate critical issues in the migration system.

1️⃣0️⃣ The "dedupe_entity" plugin does not exist

benjifisher	#3223182: The "dedupe_entity" plugin does not exist
Matroskeen	@danflanagan8 I agree with your comment #3223182: The "dedupe_entity" plugin does not exist#comment-14346569I wanted to take a stub of the test and after that reply in the issue. I'm planning to work on it tomorrow.
Matroskeen	I think the initial test can be very straightforward - just make sure migration is not failing.
Matroskeen	The rest can be moved into a follow-up.
danflanagan8	Sweet.
danflanagan8	That bug got me a few days ago and I could not believe there was still a reference to dedupe_entity
benjifisher	Are you two happy or should I have a look, too?
danflanagan8	I think we're happy. I think it would look best for @Matroskeen to do the RTBC since he was the one who threw it back to NW. I'm happy to make the followups. Just let me know!
Matroskeen	Since I contributed to the merge request, I think it best for someone else to review :slightly_smiling_face:
danflanagan8	RTBC. Thanks!
danflanagan8	And it go in the new release! Way to go!

Comments

Comment #1

17 December 2021 at 21:56

quietone created an issue. See original summary.

Comment #2

quietone commented 17 December 2021 at 22:00

Status:

Needs review

» Active

Comment #3

matroskeen

Ukrainian

🇺🇦 Ukraine, Lutsk

commented 22 December 2021 at 18:53

I would like migrate community to take a look at #3254986: Remove requirements array from RequirementsException message and discuss the format of RequirementsException message.