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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃帹 run database population in transaction #7448

Merged
merged 2 commits into from Sep 30, 2016

Conversation

kirrg001
Copy link
Contributor

refs #6574, refs #7432

Populate database in a transaction.
We had issues in the past, that if a Ghost container get's destroyed before the population could finish, the database was left in a corrupt state.

@kirrg001
Copy link
Contributor Author

FYI: right now i am writing down a migration spec. I will update add the reference to the Github issue later.

@kirrg001
Copy link
Contributor Author

Note: merge into LTS branch

@kirrg001 kirrg001 added the LTS label Sep 27, 2016
@ErisDS ErisDS added this to the 0.11.1 milestone Sep 29, 2016
transaction.rollback();
});
}).catch(function populateDatabaseError() {
reject();

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

refs TryGhost#6574, refs TryGhost#7432

- create transaction for creating tables
- if an error occurs or a container get's destroyed before population finishes, transaction is rolled back
@kirrg001 kirrg001 force-pushed the fix/populate-database-in-transaction branch from eac4d78 to 3ecce35 Compare September 29, 2016 10:33
Copy link
Member

@ErisDS ErisDS left a comment

Choose a reason for hiding this comment

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

I think there is some simplification that can be done here.

});
}).catch(function populateDatabaseError(err) {
reject(err);
});

This comment was marked as abuse.

return new Promise.resolve();
});

transactionStub = sandbox.stub(db.knex, 'transaction', function (transactionStart) {

This comment was marked as abuse.

@@ -56,7 +56,7 @@ createOwner = function createOwner(logger, modelOptions) {
password: coreUtils.uid(50)
};

return models.Role.findOne({name: 'Owner'}).then(function (ownerRole) {
return models.Role.findOne({name: 'Owner'}, modelOptions).then(function (ownerRole) {

This comment was marked as abuse.

@kirrg001
Copy link
Contributor Author

See second commit, i kept it so you can look at it 馃憤

@ErisDS ErisDS merged commit 1867e1a into TryGhost:master Sep 30, 2016
@ErisDS
Copy link
Member

ErisDS commented Sep 30, 2016

This looks 100% awesome to me 馃憤

ErisDS pushed a commit that referenced this pull request Sep 30, 2016
* 馃帹  run database population in transaction

refs #6574, refs #7432

- create transaction for creating tables
- if an error occurs or a container get's destroyed before population finishes, transaction is rolled back

* 馃帹  simplify transaction creation and test
@ErisDS
Copy link
Member

ErisDS commented Sep 30, 2016

Have cherrypicked this directly onto the lts branch as it is a clean merge.

@ErisDS ErisDS mentioned this pull request Oct 5, 2016
22 tasks
@ErisDS ErisDS removed the LTS label Oct 7, 2016
yo1dog pushed a commit to yo1dog/Ghost that referenced this pull request Oct 14, 2016
* 馃帹  run database population in transaction

refs TryGhost#6574, refs TryGhost#7432

- create transaction for creating tables
- if an error occurs or a container get's destroyed before population finishes, transaction is rolled back

* 馃帹  simplify transaction creation and test
mixonic pushed a commit to mixonic/Ghost that referenced this pull request Oct 28, 2016
* 馃帹  run database population in transaction

refs TryGhost#6574, refs TryGhost#7432

- create transaction for creating tables
- if an error occurs or a container get's destroyed before population finishes, transaction is rolled back

* 馃帹  simplify transaction creation and test
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
* 馃帹  run database population in transaction

refs TryGhost#6574, refs TryGhost#7432

- create transaction for creating tables
- if an error occurs or a container get's destroyed before population finishes, transaction is rolled back

* 馃帹  simplify transaction creation and test
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.

None yet

2 participants