[meeting] Migrate Meeting 2024-02-01 2100Z [#3417346]

Hello all, it’s time for the biweekly 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 second 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 2024-04-25

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.

mikelutz (he/him)	#3069776: SQL source plugins: allow defining conditions and join in migration yml
benjifisher	Allow process plugins to flag a row to be skipped[#3247718]
danflanagan8	I’m curious if any migrate maintainer saw this thread? Could/should migrate be used to replace some update hooks? https://drupal.slack.com/archives/C1BMUQ9U6/p1705606149722529
benjifisher	Does :party-thread: trigger the bot to create a thread? If so, can you use a static emoji instead?
mikelutz (he/him)	I could use any emoji I want. I picked one that I’ve never seen used. (Though If I set it up how I think I did, only you or I can trigger a new thread from the emoji.
xurizaemon	Oh no, I got caught by the emoji behaviour too! Lol
xurizaemon	It just made the bot PM me though

0️⃣ Who is here today?

mikelutz (he/him)	Hmm well, the 0️⃣ and 1️⃣ threads are out of order, but I can fix that.. Anyway, I’m here.
benjifisher	I am here, too!
mikelutz (he/him)	Also, I misspelled initiative twice in two different ways.. :rolling_on_the_floor_laughing:
benjifisher	You are so creative!
amaisano	I'm kind of here.
alison	Same - kind of here :wave:
quietone	Late but finally here.
Dan Davis	here
xurizaemon	:wave: Happy new year all

2️⃣ Action items. To be added later.

3️⃣ Statistics

Migrate Inititive Meeting	351 Open issues, none fixed since last meeting. 7 RTBC - 1 Major, and for the first time in recent memory, no issues need review. 15 isssues have been touched in the last month that "need work"
Migrate Inititive Meeting	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 contribute. Give us some idea of what you would like to do: documentation, code review, testing, project management, ...

5️⃣ Previous minutes.

Migrate Inititive Meeting	#3415551: [meeting] Migrate Meeting 2024-01-18 1400Z (edited)
benjifisher	Already Fixed. 🙂
Dan Davis	Did we get all caught up on inputting previous meeting logs?
benjifisher	Maybe?

6️⃣ Announcements

7️⃣ SQL source plugins: allow defining conditions and join in migration yml

Migrate Initiave Meeting	#3069776: SQL source plugins: allow defining conditions and join in migration yml
Migrate Initiave Meeting	(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1706821591482549?thread_ts=... (edited)
mikelutz (he/him)	Haven’t looked at this one in a while, not sure where it’s at, but I still think it’s a good idea.
benjifisher	It would be good to get this one in. People are using the patch, I think.
benjifisher	One of us should look at it today, and figure out the status.
benjifisher	I am taking a look.
alison	(I am someone using the patch -- yes please!)
benjifisher	@alison, it looks as though you are the one who created the issue in the first place.
benjifisher	I am not sure if the issue summary is up to date. If it is, then (looking at the list of Remaining Tasks) it should be postponed on #2833060: SqlBase::prepareQuery() should be called also on count.
benjifisher	There is also some discussion (maybe starting in Comment #82) about whether conditions should be added in `prepareQuery()` or `initializeIterator()`.
alison	I remember last time I commented, I saw that some details are undecided.
benjifisher	@mikelutz (he/him): Does this look familiar? (Part of Comment #90) While I don't know what the ultimate roles of query, prepareQuery, SqlBase:initializeIterator, and tag alters will be, or if we will change these around to better define their roles, I think that at the end of the day there is more in this system to clean up than just simply adding prepareQuery to count, and whatever we do needs to be thought through and designed and named in a better way than it is now. At that point, it's likely that prepareQuery will play the role of a precount_query_alter and the stuff currently in initializeIterator should go in a sort of postcount_query_alter (conditions used for filtering unneeded rows in a particular run while keeping the actual count unaffected).
benjifisher	It is not a super complicated change. We just need to agree on where to put the code.
benjifisher	`prepareQuery()` is a `protected` function. It looks as though there are 8 source plugins in core that call it. Presumably, they all extend `SqlBase`. If we decide to put the new code in a separate method, then we could update `SqlBase` and those 8 extensions and call the new code wherever `prepareQuery()` is called.
benjifisher	Since the current `prepareQuery()` adds the `migrate` tag to the query and adds `$this->migration` as metadata, anyone (custom or contrib module) could implement `hook_query_alter()` or `hook_query_migrate_alter()`. Get the migration from the metadata, then look for the `conditions` key in the configuration.
benjifisher	Pro: the conditions (and joins, or whatever) are automatically added to the count query. I think.
benjifisher	Con: the constructor does not know to check for conditions. So validation would come later in the process, when the query is altered instead of when the source plugin is instantiated.
benjifisher	This feature is not needed for the core migrations. If it can be implemented in a contrib module, then the policy is that we will not do it in core.
mikelutz (he/him)	It can't be implemented in contrib in a way that will be useful, and it will be used in core. We will be able to deprecate and reduce the number of core source plugins with this.
xurizaemon	I believe whereever we've used this patch, we've also used the prepareQuery() count patch. (My recollection anyway - a little dated this time of year!) (edited)
alison	Pro: the conditions (and joins, or whatever) are automatically added to the count query. I think. FWIW: That's what I've observed while using this patch.
alison	Hmmmm I haven't used the prepareQuery() count patch...

7️⃣ Allow process plugins to flag a row to be skipped

Migrate Initiative Meeting	#3247718: Allow process plugins to flag a row to be skipped (edited)
Migrate Initiative Meeting	(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1706821746468399?thread_ts=... (edited)
mikelutz (he/him)	And forgot to change the thread number.. :sight:
mikelutz (he/him)	:sigh:
mikelutz (he/him)	Anyway, this one is still on me for requested changes, additional test coverage, and some extra features I was playing with.
amaisano	i've somewhat dealt with this using my `exclude_on_empty` plugin - unless that's a different thing. i modified skip_on_empty to send the FALSE flag, so that nothing is saved to the database. but it still uses exceptions, so... nvmnd
benjifisher	I just wanted to bring it up to make sure we are not both waiting for the other one to do something.
mikelutz (he/him)	I assigned it to myself.

9️⃣ Using migrate instead of update hooks

Migrate Initiative Meeting	https://drupal.slack.com/archives/C1BMUQ9U6/p1705606149722529
Migrate Initiative Meeting	(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1706822212485449?thread_ts=... (edited)
danflanagan8	Does migrate-in-place make sense?
mikelutz (he/him)	I think in the early days there was some thoughts that this would be the future, but there has been no effort on it. I think the idea was that rather than an update hook where you could do anything, including cause a fatal error, updates would be defined in yml files, and the migrate api would be used to run them, but I don’t think it’s quite practical, even if we built the infrastructure to do it.
mikelutz (he/him)	Think like doctrine migrations in symfony, how they are defined in yml files to do database updates.
danflanagan8	I see there is a `d8_config` source plugin
mikelutz (he/him)	But I think the things that we use update hooks for are so varied, I don’t think it’s reasonable to restrict that freedom.
danflanagan8	And would part of the `migrate` module need to be split out into `lib/Drupal/Core`? Because most sites don’t have migrate enabled
danflanagan8	Or part of the update is switching migrate on and off?
mikelutz (he/him)	Among other things. It would have to be a whole new system built out.
danflanagan8	Cool. Thanks for the chat. 🙂
benjifisher	OTOH there are drawbacks to using update functions. For example, a lot of CI deployments do not finish until the update functions are done. If you have an update function that takes a long time, the CI job might time out and fail. Or your CI provider might charge you mega $$.
benjifisher	In that case you might prefer to write a migration using the `content_entity` source and run `drush mim my_update_migration` whenever it is convenient.
benjifisher	If we remove `migrate_drupal` from core but keep `migrate`, then I will advocate for keeping the `content_entity` source plugin in core. Move it (from `migrate_drupal` to `migrate`) in order to keep it. :wink:
jasonawant	Along this topic, when underlying content models change and there's a need to move content/data around, I've personally shifted over the years from using hook_update_N($sandbox) to hook_post_update_name($sandbox) or using batch operations to using custom drush commands with batch processing to be executed outside of CI deployments.Just recently I started to think that using migrations could be a better alternative to these approaches b/c of better reporting and features, e.g. rollback using migration tools.
benjifisher	Depending on what you are updating, the `migrate_map_%` tables could be useful. For example, if you are converting nodes to Media entities, or something with a lot of attached fields to a custom entity type with a lot of base fields.
benjifisher	Or maybe you want to use some of the DOM process plugins from `migrate_plus`.
jasonawant	Thx for the pointers!

🔟 What emoji should the moderators use to trigger thread creation from a comment

Migrate Initiative Meeting	@mikelutz liked :party-thread: but @benji is more of a :thread: fan..
Migrate Initiative Meeting	(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1706823608459339?thread_ts=... (edited)
mikelutz (he/him)	Spoiler: At the moment, either works. :joy:
alison	I *love* :party-thread: But I can see it being awkward when copied into an issue on d.o Also, I wish they moved not-in-sync........... seeing a bunch moving in sync does weird me out a bit. I'm so glad we're having this important conversation. (edited)
mikelutz (he/him)	:parrotwave2: :parrotwave3: :parrotwave4: :parrotwave5: :parrotwave6: :parrotwave7: We would need multiple emojis for that. (edited)
benjifisher	I find the animated emoji a little annoying, but some people are seriously bothered by them.
alison	I can see that.
quietone	My preference is no animation. For example, while reading this thread I need to put put extra effort into focusing on the words as well as into staying clam. Having said that, if the consensus is animation, I won't block it either.

1️⃣1️⃣ Wrap-up

Migrate Initiative Meeting	Thanks all! see you in 2 weeks!
Migrate Initiative Meeting	(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1706821400152349 (edited)
alison	Thank you Robot! (and @mikelutz (he/him)!)