-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add extension for Sequel transactions #21
Conversation
Waiting for #22 |
be0e182
to
c74658a
Compare
Setting back to draft as we'll need to apply changes as in #24 |
4eb7232
to
305c248
Compare
We add a `Dry::Operation::Extensions::Sequel` module that, when included, gives access to a `#transaction` method. This method wraps the yielded steps in a [Sequel](https://sequel.jeremyevans.net/) transaction, rolling back in case one of them returns a failure. The extension expects the including class to define a `#db` method giving access to the Sequel database definition: ```ruby class MyOperation < Dry::Operation include Dry::Operation::Extensions::Sequel attr_reader :db def initialize(db:) @db = db end def call(input) attrs = step validate(input) user = transaction do new_user = step persist(attrs) step assign_initial_role(new_user) new_user end step notify(user) user end # ... end ``` Default options for the `#transaction` options (which delegates to Sequel [transaction method](https://sequel.jeremyevans.net/rdoc/files/doc/transactions_rdoc.html)) can be given both at include time with `include Dry::Operation::Extensions::Sequel[isolation: :serializable]`, and at runtime with `#transaction(isolation: :serializable)`. As with the ActiveRecord extension, savepoints on nested transactions are not supported yet
305c248
to
4195339
Compare
Applying changes as in #25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor things to check, but otherwise looks good!
@@ -156,7 +156,7 @@ def initialize(model) | |||
def call | |||
transaction do | |||
step create_record | |||
transaction(requires_new: true) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this change intended to be part of this PR? It's a test for AR, not Sequel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, not sure how but I confused AR and Sequel options in the tests
instance.() | ||
end | ||
|
||
xit "works with `requires_new` for nested transactions" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is requires_new
also a Sequel thing? I recall seeing this skipped test from the AR extension... is it a copy/paste mistake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same... Sequel uses savepoint
. Fixed. Thanks for spotting it.
Overview
We add a
Dry::Operation::Extensions::Sequel
module that, when included, gives access to a#transaction
method. This method wraps the yielded steps in a Sequel transaction, rolling back in case one of them returns a failure.The extension expects the including class to define a
#db
method giving access to the Sequel database definition:Default options for the
#transaction
options (which delegates to Sequel transaction method) can be given both at include time withinclude Dry::Operation::Extensions::Sequel[isolation: :serializable]
, and at runtime with#transaction(isolation: :serializable)
.As with the ActiveRecord extension, savepoints on nested transactions are not supported yet
Screenshots/Screencasts
Details