Skip to content

Add cop to avoid find in Sidekiq workers

Terri Chu requested to merge tchu-add-worker-find_by_id-cop into master

What does this MR do and why?

Related to #472036 (closed)

This introduces a new Rubocop to avoid use of find in Sidekiq workers

Why

Sidekiq workers that run on records that are deleted will fail and retry many times due to how Sidekiq retries work. It's recommended to use find_by in the documentation, but we should surface that recommendation to developers in the IDE.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

N/A

How to set up and validate locally

  1. create or modify a sidekiq worker to use find method
  2. run rubocop on the worker
➜ rubocop ee/app/workers/search/zoekt/namespace_indexer_worker.rb

Inspecting 1 file
C

Offenses:

ee/app/workers/search/zoekt/namespace_indexer_worker.rb:13:32: C: Style/InlineDisableAnnotation: Inline disabling a cop needs to follow the format of # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency -- Some reason.
See https://docs.gitlab.com/ee/development/rubocop_development_guide.html#disabling-rules-inline.

      data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ee/app/workers/search/zoekt/namespace_indexer_worker.rb:21:21: C: Sidekiq/AvoidFindInSidekiqWorkers: Refrain from using find, use find_by instead.
A job should not fail when the record it was scheduled for is deleted.
See https://docs.gitlab.com/ee/development/sidekiq/#retries.

        namespace = Namespace.find(namespace_id)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected
Edited by Terri Chu

Merge request reports