Skip to content

Conversation

@thekid
Copy link
Owner

@thekid thekid commented Dec 30, 2024

This pull request greatly improves the import command's performance when importing journeys by reducing the number of API calls it makes:

  • Before, one call to the Dialog API was made for each child. Children are now fetched along with the first API call.
  • Google maps API lookups are only performed if the description file has changed.

Performance comparison

(For a run where nothing in the Album/oman-2024 directory has changes since the last import)

Before:

$ time xp import Albums/oman-2024/ $LOCAL
[+] [
  slug => "oman-2024"
  date => 2024-11-07 20:00:00+0100
  title => "Sultanat Oman"
# ...hundreds of lines of output
real    0m18.584s
user    0m0.015s
sys     0m0.030s

After:

$ time xp import Albums/oman-2024/ $LOCAL
[+] de.thekid.dialog.import.Journey<oman-2024>
 => Fetching entry oman-2024: ✓
 => Finished at Mon, 30 Dec 2024 14:38:07 +0000

real    0m0.649s
user    0m0.000s
sys     0m0.031s

Implementation status

  • Journeys
  • Contents
  • Cover image

@thekid thekid added the refactoring Code refactoring label Dec 30, 2024

// Process targets first...
foreach ($this->processing->targets($this->source) as $kind => $target) {
$transfer[$kind]= $target;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to include the following:

/** Executes a given external command and returns its exit code */
private function execute(string $command, array<string> $args, $redirect= null): void {
  $p= new Process($command, $args, null, null, [STDIN, $redirect ?? STDOUT, STDERR]);
  if (0 === ($r= $p->close())) return;

  throw new IllegalStateException($p->getCommandLine().' exited with exit code '.$r);
}

// FIXME: Uploading files that take longer than ~30 seconds is, for some reason,
// broken, and will result in a) the import tool crashing and b) the server ending
// up in an endless blocking loop. Use `curl` for videos instead.
if ('video' === $kind) {
  $this->execute('curl', [
    '-#',
    '-X', 'PUT',
    '-H', 'Authorization: '.$this->api->headers()['Authorization'],
    '-F', $kind.'=@'.strtr($target->getURI(), [DIRECTORY_SEPARATOR => '/']),
    $resource->uri(),
  ], ['null']);
} else {
  $transfer[$kind]= $target;
}

...which we might need to reinstate after integration-testing this PR.

@thekid thekid merged commit fccf361 into main Dec 31, 2024
13 checks passed
@thekid thekid deleted the refactor/import branch December 31, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant