Add retry logic to PRM watch thread #116
Conversation
Signed-off-by: Michael Honaker <[email protected]>
Signed-off-by: Michael Honaker <[email protected]>
HonakerM
force-pushed
the
add_watch_retry_and_exit
branch
from
6964328 to
83f81c7
Compare
Signed-off-by: Michael Honaker <[email protected]>
Signed-off-by: Michael Honaker <[email protected]>
| list_resource_version = self.kubernetes_watch.resource_version | ||
| self.wait_on_precondition(self.retry_delay.total_seconds()) | ||
| self.attempts_left = self.attempts_left - 1 | ||
| log.info("Restarting watch with %d attempts left", self.attempts_left) |
There was a problem hiding this comment.
This may be because I'm reviewing on my phone and can't see indentation well, but I'm not seeing how this actually performs the retry. There isn't a loop above the try, so it's this being done outside this function?
There was a problem hiding this comment.
There isn't a loop above the try, so it's this being done outside this function?
The try is inside the While True: loop
There was a problem hiding this comment.
This is why I shouldn't review on a phone!
|
|
||
| # Update the resource version to only get new events | ||
| list_resource_version = self.kubernetes_watch.resource_version | ||
| self.wait_on_precondition(self.retry_delay.total_seconds()) |
There was a problem hiding this comment.
This returns a bool, but I'm not seeing it used anywhere. Can the preconditions fail?
There was a problem hiding this comment.
I'm not seeing it used anywhere.
good catch. It should be checked in the retry logic.
Can the preconditions fail?
Yep! If the thread should shutdown then it returns True
| ): | ||
| # Validate leadership on each event | ||
| if not self.check_preconditions(): | ||
| log.debug("Checking preconditions failed. Shuting down") |
There was a problem hiding this comment.
I know this is an indentation change, but Shutting
Signed-off-by: Michael Honaker <[email protected]>
HonakerM
force-pushed
the
add_watch_retry_and_exit
branch
from
9ce0bd8 to
13b7f89
Compare
Related Issue
Supports #?
Related PRs
This PR adds better support for ApiExceptions in the watch thread by adding retry logic and a
sys.exitin-case of failure. In the past if a watch thread failed due to 403/400 errors then the thread would die and nothing would restart it.What this PR does / why we need it
Special notes for your reviewer
If applicable**
What gif most accurately describes how I feel towards this PR?