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

Deep stringify worker options to account for hash in on_conflict #506

Merged
merged 1 commit into from
May 20, 2020

Conversation

jasonbekolay
Copy link
Contributor

No description provided.

@mhenrixon
Copy link
Owner

mhenrixon commented May 19, 2020

First of all a massive thank you for both PRs! Secondly, would it be possible to get a test that makes this fail unless the deep stringify keys are present?

I'm asking because obviously there was missing test coverage to begin with and I'd like to improve that :)

I can also add it after merging if you are in a rush, would be great to have this fixed once and for all.

In all honesty, I didn't quite understand the problem initially.

spec/sidekiq_unique_jobs/lock_config_spec.rb Outdated Show resolved Hide resolved
"lock" => :until_and_while_executing,
"on_conflict" => {
"client" => :log,
"server" => :reschedule,
Copy link
Owner

Choose a reason for hiding this comment

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

Yup! This test should be good enough!

@jasonbekolay jasonbekolay force-pushed the master branch 2 times, most recently from abeb1c1 to 50d0f51 Compare May 19, 2020 19:51
Copy link
Owner

@mhenrixon mhenrixon left a comment

Choose a reason for hiding this comment

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

Nice work!!

@mhenrixon mhenrixon merged commit 2e79022 into mhenrixon:master May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants