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

Add retry args, no_retry & retry_attempts #3

Closed
wants to merge 3 commits into from

Conversation

codebender
Copy link
Contributor

Hi @didil,

I was working on adding some arguments around retry options

no_retry: tells sidekiq to not retry the job obviously
retry_attempts: tells sidekiq how many times to retry the job, if it fails

I'm not sure if I implemented them in the best way, but hopefully you will agree that they are valuable features to add 😺

I also refactored the specs a little where I was working, I hope that is ok.

Cheers!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1ca089d on MisterBender:feature/add_retry_args into 01b343c on didil:master.

@didil
Copy link
Owner

didil commented Sep 15, 2013

I've checked the sidekiq wiki (https://github.com/mperham/sidekiq/wiki/Advanced-Options#workers) and I think the retry_count and no_retry options are redundant. I think there should be only one option called retry, which can take true (the default which means retry once), false, or the number of retries (5 for example)

@codebender
Copy link
Contributor Author

@didil The reason I made it two options is because of the way the cli gem works. it will cast switch option to be boolean, and then having retry_attempts being it's own option I could cast the input to an Integer. I can easily refactor to one option if you want. It will take a little extra parsing to do this part, but very doable.

@codebender codebender closed this Sep 16, 2013
@codebender codebender reopened this Sep 16, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 92f83b1 on MisterBender:feature/add_retry_args into 01b343c on didil:master.

@codebender
Copy link
Contributor Author

@didil I think I'll work tonight to get to just one option, 'retry' and clean up some things based on your code review. Do you want it all in one commit and create a new pull request? Or can I just push the refactor to my branch and the changes be reflected in the pull request?
Cheers

@didil
Copy link
Owner

didil commented Sep 17, 2013

great thanks, given it's only 2 files where the changes happen, maybe you can create a new branch / pull request so that the commit history stays clean

@codebender codebender closed this Sep 28, 2013
@codebender codebender mentioned this pull request Sep 28, 2013
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.

3 participants