Retry storage operations in case of ClientError. #1107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1107 +/- ##
===========================================
+ Coverage 78.74% 89.13% +10.39%
===========================================
Files 38 38
Lines 4803 4833 +30
Branches 719 726 +7
===========================================
+ Hits 3782 4308 +526
+ Misses 920 398 -522
- Partials 101 127 +26
Continue to review full report at Codecov.
|
neuromation/api/storage.py
Outdated
|
|
||
| def retries(msg: str) -> Iterator[ContextManager[None]]: | ||
| sleeptime = 0.0 | ||
| for r in range(ATTEMPTS)[::-1]: |
There was a problem hiding this comment.
Why is reversed iteration here?
I believe that the thing may be implemented simpler:
for r in range(ATTEMPTS):
...
yield retry()
else:
raise last_error
neuromation/api/storage.py
Outdated
| if r: | ||
|
|
||
| @contextlib.contextmanager | ||
| def retry() -> Iterator[None]: |
There was a problem hiding this comment.
Isn't the function should be async?
IIUC an exception raised during the work (not the first coroutine call but in the middle of the coroutine unwinding) is not handled.
It would be nice to have a test for retries() to check this.
There was a problem hiding this comment.
You are right. There is asyncio.sleep() inside, so the context manager should be asynchronous.
|
This pull request introduces 1 alert and fixes 2 when merging 10d2d72 into 44da3a0 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 1 alert and fixes 2 when merging 1caaf90 into 44da3a0 - view on LGTM.com new alerts:
fixed alerts:
|
Can fix the part of #1093.