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

Compatibility with Sidekiq 2.12.1 Scheduled Jobs #16

Merged
merged 2 commits into from
Jun 17, 2013

Conversation

lsimoneau
Copy link
Contributor

Sidekiq 2.12.1 now calls middleware when queueing scheduled jobs. This causes issues with unique-jobs: a key is stored when the job is initially scheduled, then when the job is removed from the schedule and queued, unique-jobs stops it because they key is present.

This works around the problem by storing a different value (2) in the job hash for jobs that are being scheduled. When a new job is being processed, if it was previously scheduled but is now being queued, we let it through.

@deckchairhq
Copy link

There's another issue hidden under the covers with this version: krasnoukhov/sidekiq-middleware#12 (comment)

@felixyz
Copy link

felixyz commented Jun 17, 2013

Is anyone working on a fix for the issue @thinkgareth linked to? If I understand correctly, the solution (according to @mperham) is that: "Client middleware needs to handle both Class or String as the first argument."

mhenrixon added a commit that referenced this pull request Jun 17, 2013
Compatibility with Sidekiq 2.12.1 Scheduled Jobs
@mhenrixon mhenrixon merged commit 0d19454 into mhenrixon:master Jun 17, 2013
@felixyz
Copy link

felixyz commented Jun 17, 2013

@mhenrixon Are you planning to make a new gem release within a few days with this fix in it? I'd appreciate it, for one... :)

@rajagopals
Copy link

I still get the error mentioned by @thinkgareth : NoMethodError - undefined method `get_sidekiq_options' after applying this fix. I don't think this PR fixes that..

@disbelief
Copy link
Contributor

@rajagopals I too was still hitting this get_sidekiq_options error with the latest. I've submitted PR #17 to deal with string worker_class arguments passed to the client middleware.

@mhenrixon
Copy link
Owner

Thanks to @disbelief we have a new version released that should take care of this.

@rajagopals
Copy link

Thanks!

On Sunday, June 23, 2013, Mikael Henriksson wrote:

Thanks to @disbelief https://github.com/disbelief we have a new version
released that should take care of this.


Reply to this email directly or view it on GitHubhttps://github.com//pull/16#issuecomment-19879758
.

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 this pull request may close these issues.

6 participants