-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 "Nested Transactions" section #355
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Thank you for sharing this article. This article seems to be referring to performance issues caused by nested transactions in PostgreSQL. Please note that this PR is not about whether you should use nested transactions, but only that if you do, you should not use |
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.
This makes total sense.
I apologize for an unrelated reference above.
Here are some related references:
- a PR with a cop corresponding to this guideline Add new
Rails/PrivateTransactionOption
cop rubocop-rails#1236 - nested transactions rspec-rails issue with fruitful discussion Records created outside of transaction block gets rolled back when transaction raises exception rspec/rspec-rails#2598
- ActiveRecord::Base.transaction with requires_new: true is recommended rubocop-rails#400 recommends using requires_new: true
- Fix #400 Add Rails/TransactionRequiresNew cop rubocop-rails#414 a corresponding cop (closed as stale)
@@ -847,6 +847,28 @@ else | |||
end | |||
---- | |||
|
|||
=== Nested Transactions [[nested-transactions]] |
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.
Similar to what I said in the corresponding RuboCop PR, I think we could group this into a section about avoiding private APIs.
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.
Thank you for your suggestion.
However, there may be different reasons why private APIs should not be used, so I'm wondering if it's appropriate to group them all in one section.
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.
I think the section should remain as it is because the focus should be more on the issues that arise with nested transactions rather than the use of private APIs.
@wata727 I’ve been reading your blog, and I was wondering if you could elaborate a bit more on after_commit
in this style guide?
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.
I'm thinking about this PR again now, and I'm beginning to think that the section might not be suitable as a "style guide." I believe it’s worth having RuboCop find the problems caused by joinable: false
, but it might be endless to list all such problems in the style guide.
@koic Is there a chance we could add a cop without adding it to the style guide, or do you think problems like this are worth mentioning in the style guide?
See also rubocop/rubocop-rails#1236