Problem/Motivation

I did some improvements to the Drush command that should improve performance when using with a lot of files:

  • Consider the preserve_dates option
  • Stop injecting an entity storage class, it's bad practice
  • Apply transformations to command options once at the start instead of again per file
  • Load entities in chunks instead of one at a time, should speed up the process
  • Apply mb_strtolower to extensions passed as argument
  • Remove usage of deprecated method $this->fileSystem->basename($path)
  • Check all field items when the field option is passed instead of only the first one
  • Print any error output if the exiftool call fails

Issue fork metadata_sanitizer-3570393

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

dieterholvoet created an issue. See original summary.

dieterholvoet opened merge request !9

dieterholvoet’s picture

dieterholvoet commented
Status: Active » Needs review
dieterholvoet’s picture

dieterholvoet commented
Issue summary: View changes
zarabatana’s picture

zarabatana commented
Status: Needs review » Needs work

Thanks a lot for working on this — the improvements here are really nice (normalizing options once, chunked loading, preserve_dates support, better error output, and continuing after a single failure).

I spotted one small but important detail in the --field filtering: FileUsageInterface::listUsage() returns entity IDs as array keys (values are counts). Right now we chunk $entity_ids directly, which can end up chunking the counts and make loadMultiple() load the wrong entities, so the field filter might not match.

Would you mind adjusting it like this?

$ids = array_keys($entity_ids);
foreach (array_chunk($ids, 50) as $chunk) {
  foreach ($storage->loadMultiple($chunk) as $entity) {
    …
  }
$storage->resetCache($chunk); }

Once that’s in (and CI is green), I’m happy to mark it RTBC / merge.

dieterholvoet’s picture

dieterholvoet commented
Status: Needs work » Needs review
zarabatana’s picture

zarabatana commented
Status: Needs review » Reviewed & tested by the community

MR !9 is now merged, and CI is green. This resolves the Drush command improvements described here (chunked loading, preserve_dates handling, option normalization, better error output, and correct-- field filtering). Marking the issue fixed/closed.

zarabatana’s picture

zarabatana commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

zarabatana’s picture

zarabatana commented
Status: Fixed » Closed (fixed)