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

Wrap all up/down DB migration scripts in a transaction #772

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

arielshaqed
Copy link
Contributor

This prevents half-successful migration scripts from leaving the DB in an unusable state.

Bad migration scripts (e.g. missing DDL removal in a down script) may still break things and
require manual fixing. To fix that we would need to use the single-command CREATE TABLE form
with FOREIGN KEYs and CONSTRAINTs. We initially made an undocumented decision not to do so,
and this commit continues that tradition.

This prevents half-successful migration scripts from leaving the DB in an unusable state.

Bad migration scripts (e.g. missing DDL removal in a down script) may still break things and
require manual fixing.  To fix that we would need to use the single-command `CREATE TABLE` form
with `FOREIGN KEY`s and `CONSTRAINT`s.  We initially made an undocumented decision not to do so,
and this commit continues that tradition.
@arielshaqed arielshaqed requested a review from nopcoder October 6, 2020 12:19
@arielshaqed arielshaqed added the pr/merge-if-approved Reviewer: please feel free to merge if no major comments label Oct 6, 2020
Copy link
Contributor

@nopcoder nopcoder left a comment

Choose a reason for hiding this comment

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

cool

@nopcoder nopcoder merged commit be24c89 into master Oct 6, 2020
@nopcoder nopcoder deleted the bugfix/idempotent-ddl branch October 6, 2020 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/merge-if-approved Reviewer: please feel free to merge if no major comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants