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

Rails/FindEach should ignore queries with lock #389

Closed
artplan1 opened this issue Nov 23, 2020 · 2 comments · Fixed by #392
Closed

Rails/FindEach should ignore queries with lock #389

artplan1 opened this issue Nov 23, 2020 · 2 comments · Fixed by #392

Comments

@artplan1
Copy link

Is your feature request related to a problem? Please describe.

find_each doesn't play well with lock in query.

example: https://dev.betterdoc.org/tools/2019/04/05/use-pessimistic-locking-to-make-long-running-tasks-in-rails-free-of-race-conditions.html

Describe the solution you'd like

add lock to IGNORED_METHODS https://github.com/rubocop-hq/rubocop-rails/blob/master/lib/rubocop/cop/rails/find_each.rb#L25

Describe alternatives you've considered

allow to configure ignored_methods for this cop

@koic
Copy link
Member

koic commented Nov 24, 2020

Oddly, it doesn't reproduce on the master branch testing. Can you provide a display for rubocop -V?

@artplan1
Copy link
Author

sorry, I probably didn't explain it well

in our case rubocop-rails proposes to change User.lock.each to User.lock.find_each that leads to unexpected results.
this issue is explained here as well: https://dev.betterdoc.org/tools/2019/04/05/use-pessimistic-locking-to-make-long-running-tasks-in-rails-free-of-race-conditions.html

just in case

rubocop -V                 
1.4.0 (using Parser 2.7.2.0, rubocop-ast 1.1.1, running on ruby 2.7.2 x86_64-darwin19)
  - rubocop-performance 1.9.0
  - rubocop-rails 2.8.1

tejasbubane added a commit to tejasbubane/rubocop-rails that referenced this issue Nov 24, 2020
tejasbubane added a commit to tejasbubane/rubocop-rails that referenced this issue Nov 24, 2020
tejasbubane added a commit to tejasbubane/rubocop-rails that referenced this issue Nov 24, 2020
tejasbubane added a commit to tejasbubane/rubocop-rails that referenced this issue Nov 26, 2020
tejasbubane added a commit to tejasbubane/rubocop-rails that referenced this issue Nov 26, 2020
@koic koic closed this as completed in #392 Nov 26, 2020
koic added a commit that referenced this issue Nov 26, 2020
[Fix #389] Add `IgnoredMethods` configuration option for `Rails/FindEach` cop
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 a pull request may close this issue.

2 participants