-
Notifications
You must be signed in to change notification settings - Fork 46
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
Allow unlimited tries #95
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (i...
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (in .rubocop.yml). 2.0-compatible analysis was dropped after version 0.50. Supported versions: 2.1, 2.2, 2.3, 2.4, 2.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (i...
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (in .rubocop.yml). 2.0-compatible analysis was dropped after version 0.50. Supported versions: 2.1, 2.2, 2.3, 2.4, 2.5
Update: Apparently this is failing now due to the |
As this gem supports all ruby versions >= 2.0, fix rubocop version to 0.50 (the last version supporting ruby 2.0). Also fix all auto-correctable and/or superficial cops. Remove `Gemfile.lock` from `.gitignore` (per kamui#94) and commit working lockfile.
Tests included. Some refactoring required to implement this as current implementation's ExponentialBackoff class produced an array, and infinite retries are incompatible with a finite # of values. Changed code to support/use (lazy) enumerators instead.
Causing warnings due to conditional logic in gemspec. Also don't seem to be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (i...
Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (in .rubocop.yml). 2.0-compatible analysis was dropped after version 0.50. Supported versions: 2.1, 2.2, 2.3, 2.4, 2.5
- Rubocop - Moved rubocop checks into travis and disabled hound - Hound did not support rubocop 0.50.x, which is the last version to support Ruby v2 - Condensed/simplified .rubocop.yml - Corrected easily fixed cops - Gemfile - Added extra constraints due to encountered bugs - Added pry enhancements - TODO: add to gemspec - Rake - Added Rakefile with tasks to simplify testing (both locally and on travis) - Travis - Upgraded OS to use Ubuntu xenial rather than trusty (end of life) - Script now runs bundle exec rake rather than rspec directly
@mvastola Can I ask what's the use case for needing or wanting unlimited retries? This change will change the API enough that it would likely need a major release. Unlimited tries is not something I'd ever use, especially with exponential backoff as that interval could get so long between tries. When would you not want it to eventually give up? It just seems like it could be dangerous to retry infinitely. |
Any service depending on another service that may go down for undefined amount of time. Like when some websites break the API and aren't hurrying to fix it and my bots deployed once in a year would like to wait until it works again instead of halting and doubling their offline time by waiting for me to restart them. |
So I'm dumb and am only now realizing a PR (#20) already exists for this, but as it's 4+ years old and is based on an older version of the code, maybe this is worth it anyway.
As noted in #20, implementing this requires the use of
Enumerator
s because theExponentialBackoff
class currently produces an array, which is no longer feasible.Thoughts/comments/criticism very welcome.
Note: Fixes #94. Obviously fixes #20 as well.