feat: Per-migration transaction configuration#1671
Conversation
|
@lourd is attempting to deploy a commit to the Kysely Team Team on Vercel. A member of the Team first needs to authorize it. |
lourd
changed the title
lourd
force-pushed
the
per-migration-transaction
branch
from
a65e02e to
58d73b0
Compare
|
Hey @igalklebanov, eager to hear your feedback! |
|
Great work @lourd. |
igalklebanov
force-pushed
the
next
branch
4 times, most recently
from
a225c9f to
ebcfe24
Compare
Co-authored-by: Eric So <56284867+ericsodev@users.noreply.github.com> Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
…ely-org#1516) Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
…fier (kysely-org#1615) Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: igalklebanov <igalklebanov@gmail.com>
lourd
force-pushed
the
per-migration-transaction
branch
from
7ba0d09 to
1a9ef42
Compare
|
Hey @igalklebanov, just rebased on |
commit: |
| * Note: Setting this to `true` has no effect if the dialect doesn't support | ||
| * transactional DDL. | ||
| */ | ||
| readonly transaction?: boolean |
There was a problem hiding this comment.
If there wasn't such a strong case to keep knex naming and behavior - familiarity and lots of training data, I'd call this something more explicit and perhaps go beyond boolean.
e.g. transactionMode?: 'never' | 'shared' | 'file-scoped', where "never" means no matter what, never runs this file's changes in a transaction, "shared" means reuse or create "global" transaction (the default mode), "file-scoped" which means always commit previous transaction group if available and start a transaction just for this file.
Not for now. Something to consider after this effort.
There was a problem hiding this comment.
I think that's a great idea, covers more use cases than a boolean can. Happy to make those changes, let me know!
There was a problem hiding this comment.
Not for this iteration. Let's keep it knexy and minimal as possible.
igalklebanov
force-pushed
the
next
branch
3 times, most recently
from
f0934fe to
a859b43
Compare
igalklebanov
added
enhancement
igalklebanov
force-pushed
the
next
branch
3 times, most recently
from
63af1a3 to
0cf6dcb
Compare
igalklebanov
force-pushed
the
next
branch
2 times, most recently
from
ca8e49a to
9f037a2
Compare
igalklebanov
force-pushed
the
next
branch
6 times, most recently
from
53663bf to
e74f592
Compare
igalklebanov
force-pushed
the
next
branch
2 times, most recently
from
ab33123 to
d628e64
Compare
|
@lourd @igalklebanov Anything I can do to help? |
|
@shoooe, thanks for the offer. Last I talked with @igalklebanov this is slated for 0.30, he wanted to get 0.29 out first. I need to rebase on latest and resolve the merge conflicts. |
|
Alright, thanks very much for working on this @lourd @igalklebanov. 🙏 |
13 participants
Closes #352
Creates the ability for individual migrations to opt out of being executed in a transaction, depending on the adapter's support for transactions, the Migrator configuration, and the options given to the migrate method.
Follows pretty closely to the Knex implementation as suggested by @tgriesser.
If transactions are enabled globally and an individual migration opts out, then all of the migrations before it are run in a transaction, then it is run outside of a transaction, then all the migrations after it are run in a transaction.
The behavior is the same when migrating up and down.
I decided that if a transaction is passed to the Migrator (#1480), it results in an error. And if any of the options disabling transactions are used, either globally or per migrate call (#1517), those take precedence.
Basically this feature only takes effect when transactions aren't disabled and the Migrator has responsibility over transactions.