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

Use existing transaction if provided instead of creating a new one #29

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

elashpear
Copy link
Contributor

Right now it's not possible for a single migration using this library to be completely atomic inside a single transaction as the library always creates a new transaction whenever the function is called. This change allows an existing transaction to be supplied in sequelizeOptions, allowing a migration to place all changes under a single transaction.

@abelosorio
Copy link
Owner

Thanks for your contribution @elashpear

@abelosorio abelosorio merged commit 1ef606e into abelosorio:master Oct 28, 2022
@cyrilchapon
Copy link

cyrilchapon commented Oct 29, 2022

This is a mistake, the library already supported the use of a "passed-down transaction" through sequelizeOptions.

await replaceEnum({
  queryInterface,
  sequelizeOptions: { transaction: t }, // Pass-down HERE
  tableName: 'eventRecurrence',
  columnName: 'recurrenceType',
  defaultValue: 'weekly',
  newValues: ['weekly', 'monthly', 'yearly', 'on-demand'],
  enumName: 'enum_event_recurrence_recurrence_type'
})

When doing so, it will create a subtransaction to group all replaceEnum queries. This behavior brings the best of 2 worlds : you have a default Error-and-revert-transaction bubbling if you don't catch; but also brings the ability to only catch replaceEnum Errors.

In other words, what you was trying to achieve is already implemented (and unit-tested); with a more robust solution than this PR.

It was not documented though; so I added documentation for it in my last PR #38 . I also reverted this current PR, because this implementation hides the more robust sub-transaction implementation.

@cyrilchapon cyrilchapon mentioned this pull request Oct 31, 2022
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.

3 participants