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.

Sign up for GitHub

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

Jump to bottom

Decrement Redis Counter for Errored Sidekiq Jobs #24

Merged
merged 4 commits into from
Jun 26, 2023
Merged

Decrement Redis Counter for Errored Sidekiq Jobs #24

merged 4 commits into from
Jun 26, 2023

Conversation

hkim3162
Copy link
Contributor

@hkim3162 hkim3162 commented May 31, 2023 •
edited
Loading

In Sidekiq Instrument, ensure that in the rescue block of when a job errors that we continue to decrement the counter. When jobs are errored the enqueue increment never gets removed

  • Adds a decrement to the redis count in an ensure block
  • Adds Rubocop as a development dependency (not adding to CI at this time as there are quite a lot of issues)
  • Adds additional rspecs that utilize both server and client middleware
  • cleans up some code/unused tests

@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch 3 times, most recently from be6c334 to 3a3e359 Compare May 31, 2023 19:16
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch 3 times, most recently from 4e9c829 to f94749a Compare June 21, 2023 16:07
@hkim3162 hkim3162 changed the title decrement counter for errored Decrement Redis Counter for Errored Sidekiq Jobs Jun 21, 2023
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch 6 times, most recently from 8baa818 to 652fe7d Compare June 22, 2023 16:39
SheffAtEnova
SheffAtEnova requested changes Jun 22, 2023
View reviewed changes
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/client_middleware_spec.rb Show resolved Hide resolved
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch 2 times, most recently from 8414600 to f551002 Compare June 23, 2023 13:47
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch from f551002 to e9061cc Compare June 26, 2023 13:14
SheffAtEnova
SheffAtEnova requested changes Jun 26, 2023
View reviewed changes
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/client_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch from 1d1fe07 to 3e85f78 Compare June 26, 2023 14:33
@hkim3162 hkim3162 force-pushed the add_decrement_counter_for_errored branch from 3e85f78 to 99efb15 Compare June 26, 2023 14:34
SheffAtEnova
SheffAtEnova requested changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@SheffAtEnova SheffAtEnova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more last bits in line with the rest of the feedback.

spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/worker_spec.rb Outdated Show resolved Hide resolved
@SheffAtEnova SheffAtEnova merged commit e64bf1b into enova:main Jun 26, 2023
12 checks passed
@mokpro mokpro deleted the add_decrement_counter_for_errored branch June 7, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@packrat386 packrat386 packrat386 left review comments

@SheffAtEnova SheffAtEnova SheffAtEnova approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hkim3162 @packrat386 @SheffAtEnova @hkim3163