Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db/backup/restore refinements #14921

Closed
wants to merge 4 commits into from
Closed

db/backup/restore refinements #14921

wants to merge 4 commits into from

Conversation

timkelty
Copy link
Contributor

@timkelty timkelty commented May 2, 2024

Description

#14897 tied the format set for backups (backupCommandFormat) with what is used for restoring, which is not ideal.

This PR:

  • separates backupCommandFormat from what is used for restore
  • adds --format options for db/restore and db/backup (for PG)
  • For db/restore, if no --format is passed, it will attempt to detect which pg_restore format should be used (custom, directory, tar)
  • Adds flags to the default restore command to allow postgres dumps to work (including Craft Cloud).
  • Adds some allowances so the path for db/restore can be a directory, which is applicable for pg_restore.

@timkelty timkelty changed the base branch from 5.x to 4.x May 2, 2024 01:08
@timkelty timkelty requested review from brandonkelly and angrybrad May 2, 2024 01:15
->addArg('--single-transaction')
->addArg('{file}');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using pg_restore, these args need to be on the restore, regardless if the backup was created with them.

@@ -172,7 +186,7 @@ public function getDefaultRestoreCommand(): string

return $this->_pgpasswordCommand()
. $command->getExecCommand()
. '< "{file}"';
. ($this->usePgRestore() ? '' : '< "{file}"');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't use STDIN with pg_restore, because directory is a valid input.

if (!is_file($path)) {
$this->stderr("Backup file doesn't exist: $path" . PHP_EOL);
if (!is_readable($path)) {
$this->stderr("Backup path doesn't exist: $path" . PHP_EOL);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Account for a possible directory (pg_restore)

# Conflicts:
#	src/console/controllers/DbController.php
#	src/db/pgsql/Schema.php
@timkelty timkelty closed this May 2, 2024
@timkelty timkelty deleted the bugfix/db-backup-restore branch May 2, 2024 19:17
@timkelty
Copy link
Contributor Author

timkelty commented May 2, 2024

See #14931

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant