Skip to content

Set higher timeouts for import/export wp-cli commands - #904

Merged
ashfame merged 7 commits into
trunkfrom
wp_cli_higher_timeout_import_export
Feb 17, 2025
Merged

Set higher timeouts for import/export wp-cli commands#904
ashfame merged 7 commits into
trunkfrom
wp_cli_higher_timeout_import_export

Conversation

@ashfame

@ashfame ashfame commented Feb 6, 2025

Copy link
Copy Markdown
Member

Related issues

Proposed Changes

  • Default timeout of 5mins for waiting on wp-cli command to finish is too small for big imports. In this PR, I have added a higher timeout for import/export commands, which gets used by passing an arg longRunning: true to executeWpCliCommand()
  • Show a custom error message when it does timeout (currently set to 24hrs)
Screenshot 2025-02-06 at 22 13 40

Testing Instructions

  • I tested it by lowering the default timeout to small like 5 secs and process a normal backup file, it fails as per the linked issue
  • Then on this branch, keep the default timeout lowered and see it uses the higher timeout set for import/export and finishes up.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@ashfame ashfame self-assigned this Feb 6, 2025
@ashfame
ashfame marked this pull request as ready for review February 7, 2025 08:46
@ashfame
ashfame requested a review from a team February 7, 2025 08:46
Comment thread src/hooks/use-import-export.tsx Outdated
Comment thread src/lib/wp-cli-process.ts Outdated

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes look good. Increasing the import process will allow users to import websites with more content.

To establish reasonable limits, we might need to benchmark the amount of content imported on a typical computer and set a limit based on those metrics.

I’ve added a couple of suggestions, please let me know what you think.

Comment thread src/lib/wp-cli-process.ts Outdated
Comment thread src/lib/import-export/import/importers/importer.ts
@ashfame
ashfame force-pushed the wp_cli_higher_timeout_import_export branch from b82e923 to b427255 Compare February 17, 2025 10:49
Comment thread src/hooks/use-import-export.tsx Outdated
@ashfame
ashfame requested a review from sejas February 17, 2025 10:59

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ashfame, Thanks for considering my suggestions. I like the new approach where wp-cli-process handles selecting the timeout based on the command.

I've tested it and confirmed that the timeout for SQLite export and import processes is 6 hours, while the rest of wp-cli commands still 5 minutes.

Screenshot 2025-02-17 at 15 35 46

I've left a couple of non-blocking comments.

Comment thread src/lib/wp-cli-process.ts Outdated
Comment thread src/hooks/use-import-export.tsx Outdated
ashfame and others added 2 commits February 17, 2025 21:09
@ashfame

ashfame commented Feb 17, 2025

Copy link
Copy Markdown
Member Author

@sejas Thanks for the review! I made both the changes. Will merge shortly 👍

@ashfame
ashfame merged commit 9e369b5 into trunk Feb 17, 2025
@ashfame
ashfame deleted the wp_cli_higher_timeout_import_export branch February 17, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants