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. #3456078: [meeting] Migrate Meeting 2024-07-18 2100Z)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 👤 emoji. To take a comment or thread off the record, start with a 🚫 emoji.
| Migrate Initiative Meeting |
. |
| Migrate Initiative Meeting |
(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1738271400075199?thread_ts=... (edited) |
| benjifisher |
That issue is blocked by https://www.drupal.org/project/drupal/issues/2833060. |
| benjifisher |
Good news: https://www.drupal.org/project/drupal/issues/2833060 is now RTBC. |
| benjifisher |
I added https://www.drupal.org/project/drupal/issues/3500698 as a follow-up to https://www.drupal.org/project/drupal/issues/2833060. |
| benjifisher |
@mikelutz (he/him), can you have a look at the issue description? I was trying to capture your ideas from your comment on #2833060. |
| benjifisher |
One little advantage of doing #2833060 first is that we will have a little extra test coverage before we start refactoring. |
| mikelutz (he/him) |
I did take a look at it last week, it seemed fine to me. |
| quietone |
In 2833060, the test is adding a condition to prepareQuery but the doc for that states, "Adds tags and metadata to the query". For me, this adds confusion about where one is to put conditions in a source plugin. (edited) |
| mikelutz (he/him) |
Well, yes. That is the point of the follow-up |
| quietone |
Oh, looking |
| mikelutz (he/him) |
Prepare query was supposed to be for tags and metadata only, hence why it wasn't included in count. |
| mikelutz (he/him) |
But people add tags, and then do query alters on the tags to add conditions, so there isn't anything stopping you from adding conditions in prepareQuery (edited) |
| mikelutz (he/him) |
So the point of the mr is to ensure that conditions added in prepare query are reflected in the count, and while by documentation that should happen by tags and query tag alters, for the purposes of the test it's easier to just add a condition. |
| mikelutz (he/him) |
But the whole thing is confusing, hence the follow up. The immediate issue is that prepareQuery CAN alter the count, but that is not currently reflected in the count. |
| quietone |
Also, I made two comment changes on the MR, one is a fix.#2833060: SqlBase::prepareQuery() should be called also on count |
|
Thanks @mikelutz (he/him). It makes sense now. |
| benjifisher |
I will re-review. |
| quietone |
Ta |
| benjifisher |
@quietone: You made the comment * This method should not be called directly. It is called automatically from
* SqlBase::prepareQuery(). |
| benjifisher |
Should it be * This method should not be called directly. It is called automatically from
* ::prepareQuery().instead? |
| quietone |
Either is fine with me. I didn't change the part of the sentence it was like that before. |
| Migrate Initiative Meeting |
I was thinking about this when I reviewed #2833060. |
| Migrate Initiative Meeting |
(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1738271482810309?thread_ts=... (edited) |
| benjifisher |
SqlBase::prepareQuery() adds tags to the query, so modules can implement hook_query_alter() or hook_query_TAG_alter() to modify the query. |
| benjifisher |
Do we have any examples of this pattern? That is not a hook that I use every day. In fact, I may never have used it until I was testing #2833060. |
| benjifisher |
It might be an opportunity to give an example for how to use OOP hooks. In fact, you cannot implement hook_query_TAG_alter() if the tag is d7_node:article, since : is not a legal character in a function name. Can we use an OOP hook to target that tag? |
| mikelutz (he/him) |
I mean, the documentation for using the hook is here: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Database%... adds the tag “migrate” and “migrate_” by default to queries that extend it and don’t override it in prepareQuery(). I don’t know that we document that we add that query tag. It’s not in the SqlBase file documentation. |
| mikelutz (he/him) |
I’m not sure on the OOP hooks, I confess I haven’t played with those yet. |
| benjifisher |
I think it would be fun to add an example using both. |
| Migrate Initiative Meeting |
There are 7 RTBC issues. One is Major and was last updated over 3 months ago. https://www.drupal.org/project/issues/search/drupal?text=&assigned=&subm... |
| Migrate Initiative Meeting |
(Original Request) https://drupal.slack.com/archives/C226VLXBP/p1738274285293209?thread_ts=... (edited) |
| benjifisher |
I know there is a lot going on. (Did someone say, "Starshot"?) I know the overall RTBC queue for D11 is still very large. (Currently 99 issues.) Irresponsible developers (like me!) keep adding new issues to the queue. |
| benjifisher |
So I am not expecting the core committers to drop everything and start paying attention to my pet (migration) issues. But can we get some visibility into the process/schedule? |
| quietone |
Yes, the committers who can commit these are stretched thin. Probably ask in #core-development for attention to migration issues. |
| quietone |
I was waiting till after Drupal CMS launch, but now there is the twig security thing. |
| benjifisher |
There is always something ... |
| quietone |
and you right, we can't defer any more. Plus, there is still the attributes, which is blocking deprecating migrate drupal land. |
| benjifisher |
Right, the RTBC one is https://www.drupal.org/project/drupal/issues/3258581, and that blocks converting source plugins from annotations to attributes. |
| benjifisher |
And once that is done, we want to move the source_module attribute to migrate_drupal, where it belongs. |
| quietone |
I asked in committer slack for someone to look at https://www.drupal.org/project/drupal/issues/3258581, |
Comments
Comment #2
benjifisherComment #3
ddavisboxleitner commentedComment #4
ddavisboxleitner commentedComment #5
benjifisherComment #8
smustgrave commentedAll threads appear to be captured. I did credit mikelutz and quietone for their participation.