Add ignore option to skip retries #107
Conversation
omarselim3008
changed the title
kamui
left a comment
kamui
left a comment
There was a problem hiding this comment.
Thanks for the PR, just some comments and also if you could add this to the readme docs.
| ignore = local_config.ignore | ||
|
|
||
| exception_list = on.is_a?(Hash) ? on.keys : on | ||
| exception_list = on.is_a?(Hash) ? on.keys : [on].flatten |
There was a problem hiding this comment.
Why are you wrapping it in an array and flattening? Just in case on is a single Exception?
There was a problem hiding this comment.
That's right yes. I was just making sure that this always returns an array, even if we pass a single object in the params.
lib/retriable.rb
Outdated
| return Timeout.timeout(timeout) { return yield(try) } if timeout | ||
| return yield(try) | ||
| rescue *[*exception_list] => exception | ||
| rescue *[*rescue_list] => exception |
There was a problem hiding this comment.
If we put on and ignore in an array and flatten it, do we still need this array splat expansion? This is here in case on is not an array, but you're with your changes on line 41-42, it's impossible now for this to not be an array.
| end | ||
|
|
||
| if ignore.is_a?(Hash) | ||
| raise if ignore_list.any? do |e| |
There was a problem hiding this comment.
What should happen if the user puts the same exception in both lists? In this implementation, ignore will override on right?
There was a problem hiding this comment.
Yea it should do, also a test here: https://github.com/kamui/retriable/pull/107/files#diff-da878450c719751b2a3bde1a1916ebec1eb23f30c4f9e2d69d95fbfda1184646R162
| end | ||
| end | ||
|
|
||
| if ignore.is_a?(Hash) |
There was a problem hiding this comment.
Should this ignore check happen before the on check? Isn't it generally better if we raise as soon as we can since it'll shortcut checking the other exceptions?
|
@kamui thanks for the review. Updated with your comments and also added the readme. Cheers. |
|
Nice feature to have! |
kamui
force-pushed
the
master
branch
3 times, most recently
from
6196a1e to
c448885
Compare
3 participants
Add
:ignoreoption to retriable so that we can not retry exceptions that do not need one.