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

improvement/migrations #7000

Merged
merged 1 commit into from
Jul 14, 2016
Merged

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Jun 16, 2016

closes #6972 and #6574

With this PR each database version update happens in a top level transaction.
So if you have a database version of 004 and you need 005 + 006, if 006 fails in any file, we do a rollback and your db is updated to 005.
This PR also fixes the order of executed migrations. (004 schema updates, 004 fixtues)
This PR should be merged together with #7019. #7019 has more improvements and edge cases like that your blog should not start when a migration failed.

TODO

  • tidy up PR
  • normalize transaction
  • setup 003, run migration on pg
  • setup 003, run migration on mysql
  • setup 003, run migration on sqlite
  • setup 003, simulate a fail in 005, in any db

review notes

  • all migration files are touched, that's why we have so many changes. but the only change in each migration file is that we pass the top level transaction
  • i didn't improve the way we pass params to functions, that's why i just added transaction as last param, in the future we should only use the option pattern

@kirrg001 kirrg001 force-pushed the improvement/migrations branch 9 times, most recently from d8d8647 to ea188bf Compare June 18, 2016 20:39
closes TryGhost#6972, TryGhost#6574

- run each database version as top level transaction
- run migrations in correct order
@kirrg001 kirrg001 force-pushed the improvement/migrations branch from ea188bf to 404a207 Compare June 20, 2016 12:51
@kirrg001 kirrg001 mentioned this pull request Jun 20, 2016
6 tasks
}).then(function () {
return fixtures.ensureDefaultSettings(logger);
return models.Settings.populateDefaults(modelOptions);

This comment was marked as abuse.

@kirrg001
Copy link
Contributor Author

ready to review 👍

@kirrg001 kirrg001 changed the title [WIP] improvement/migrations improvement/migrations Jun 21, 2016
// @TODO:
// - if you require this file before config file was loaded,
// - then this file is cached and you have no chance to connect to the db anymore
// - bring dynamic into this file (db.connect())

This comment was marked as abuse.

This comment was marked as abuse.

@ErisDS ErisDS merged commit 6e1bd28 into TryGhost:master Jul 14, 2016
chris-brown pushed a commit to chris-brown/Ghost that referenced this pull request Aug 14, 2016
closes TryGhost#6972, TryGhost#6574

- run each database version as top level transaction
- run migrations in correct order
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
closes TryGhost#6972, TryGhost#6574

- run each database version as top level transaction
- run migrations in correct order
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.

Migrations: ordering of tasks is incorrect!
2 participants