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

Unexpected behavior when deploy script is missing #828

Open
vectro opened this issue May 16, 2024 · 2 comments
Open

Unexpected behavior when deploy script is missing #828

vectro opened this issue May 16, 2024 · 2 comments
Labels

Comments

@vectro
Copy link
Contributor

vectro commented May 16, 2024

Steps to reproduce

  1. Set up a Sqitch project where some changes’ deploy scripts are missing.
  2. Run sqitch deploy

Or equivalently, run these commands:

sqitch init ...
sqitch add test1
sqitch add test2
rm deploy/test1.sql
rm deploy/test2.sql
sqitch deploy

Expected behavior

  • Sqitch should fail with an error the first time a plan with a missing deploy script is encountered, preferably before deploying anything.

Actual behavior

  • The first change with a missing deploy script succeeds, leaving a null value in changes.script_hash.
  • The second change with a missing deploy script fails due to the unique constraint on changes.script_hash, with the confusing error message “The deploy script is not unique”.

Other notes

Tested using Sqitch 1.4.1 with the Oracle database driver.
On Oracle (and some other platforms), a unique constraint on a null column allows at most one null value on that column.

@ewie
Copy link
Contributor

ewie commented Jun 5, 2024

sqitch init ...
sqitch add test1
sqitch add test2
rm deploy/test1.sql
rm deploy/test2.sql
sqitch deploy

This will fail for change test1 unless you use sqitch deploy --log which does not check that deploy scripts actually exist:

$self->run_deploy($change->deploy_file) unless $self->log_only;

But I think that Sqitch should check that the deploy and revert scripts exists when log_only is true. In normal deploy mode this check is delegated to the database client.

I came across this very same issue on Postgres today when a team mate reworked a change that was only logged after being restored from a schema dump. Problem was that the original script copies created by sqitch rework were not committed and missing from the CI run. (We regularly "squash" changes by creating a schema dump to restore that instead of incurring the overhead of deploying each change individually in our CI jobs.)

@theory
Copy link
Collaborator

theory commented Jun 6, 2024

Seems like maybe we could use a function to check for the presence of deploy scripts (on deploy) and revert scripts (on revert) to be called alongside check_deploy_dependencies in deploy and check_revert_dependencies in revert.

@theory theory added the todo label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants