-
Notifications
You must be signed in to change notification settings - Fork 14
Swap out pester for Retriable; Retry github errors #75
Conversation
delay_interval: 30.seconds, | ||
max_attempts: 10, | ||
on_retry: Pester::Behaviors::Sleep::Constant, | ||
reraise_error_classes: [Aws::S3::Errors::NoSuchKey, Aws::S3::Errors::NoSuchBucket] |
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.
hmm what about these reraise classes?
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.
there's no blacklist in Retriable
, so you'd either have to explicitly specify via whitelist what to retry, or i guess possibly wait for this PR: kamui/retriable#55 that would support an arbitrary Proc
evaluation which could do the black/whitelisting.
what's the reason for these exceptions not being retried? is it still even a valid concern?
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.
i think these are all unrecoverable errors. ie, retrying isn't going to help if you are missing keys or your bucket config is wrong
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.
oh i think this got put in because it was annoying waiting for it to retry when you have the AWS keys misconfigured. i'll put in a tweak to replicate this behavior.
@@ -43,7 +43,7 @@ def self.send_request(verb, url, params={}) | |||
request.content_type = 'application/json' | |||
end | |||
|
|||
http.request(request) | |||
Retriable.retriable { http.request(request) } |
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.
this is a retriable without context? what is the default behavior?
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.
Closing due to inactivity |
No description provided.