Problem/Motivation

Sometimes we need to run migrations in chunks and use the --limit option.

The migration source plugin loads the source rows from the beginning and checks if they have already been migrated, if they need to be updated, etc.

Most of the time, we can skip already migrated rows very early, saving time and improving migration performance.

This is the case when all the following conditions are met:
- the row has already been migrated
- it doesn't need an update
- the track changes feature is disabled
- the row is not above the high water mark

CommentFileSizeAuthor
#8 3407997-8.patch1.23 KBjofitz
#8 interdiff--3407997--5-8.txt518 bytesjofitz
#5 drupal-migrate-skip_migrated_rows_early-3407997-5.patch1.11 KBmartin_klima

Issue fork drupal-3407997

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

martin_klima created an issue. See original summary.

martin_klima opened merge request !5767

cilefen’s picture

cilefen commented
Version: 10.1.x-dev » 11.x-dev
Status: Active » Needs work
martin_klima’s picture

martin_klima
he/his
Primary language Czech
commented

I checked the reason for the failed test.

Spell-checking found 2 issues:
SourcePluginBase.php:416:36 - Unknown word (sourceid)
SourcePluginBase.php:418:25 - Unknown word (Highwater)

in this part of the code:

      if (!empty($row->getIdMap()['sourceid1'])
        && !$row->needsUpdate()
        && !$this->aboveHighwater($row)
        && !$this->trackChanges) {
        continue;
      }

I have no idea how to fix it. Issues found are not words. One is the array key and the second is the method name.
Do you have any hint of what I can do @cilefen?

martin_klima’s picture

martin_klima
he/his
Primary language Czech
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-migrate-skip_migrated_rows_early-3407997-5.patch1.11 KB

I am saving the current patch for reference.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests, +Needs subsystem maintainer review

Thanks for reporting.

Issue summary appears to be missing several sections, recommended to use the standard issue template. Will probably need statistics to show the performance improvement

New feature will require test coverage.

All will most likely need a submaintainer approval.

Thanks.

ghost of drupal past’s picture

ghost of drupal past
he/him
commented

The spelling errors can be skipped by adding the words to core/misc/cspell/dictionary.txt or by using a cspell:ignore line much like core/modules/filter/src/Plugin/migrate/process/FilterID.php does, for example. I am not familiar with current best practices in which one is preferred, but a) core/modules/migrate/tests/src/Kernel/HighWaterTest.php already has a Highwater ignore line b) repeated ignores seemingly are not a problem because there are multiple sourceid ignores in migrate. So: I think you should specifically ignore them instead of editing the dictionary.

jofitz’s picture

jofitz
he/him
commented
StatusFileSize
new interdiff--3407997--5-8.txt518 bytes
new 3407997-8.patch1.23 KB

Added cspell:ignore, as suggested.

Status remains at NW because patch still needs tests etc

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.