Retry admin channel setting retrieval and add configurable timeout #665
Conversation
Attempts multiple times to fetch things over the admin channel before giving up.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #665 +/- ##
==========================================
+ Coverage 60.70% 60.95% +0.25%
==========================================
Files 24 24
Lines 3667 3683 +16
==========================================
+ Hits 2226 2245 +19
+ Misses 1441 1438 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Otherwise, semantically, it's off-by-one.
For example, 0x0 will generate an unhandled ValueError. This was caught during a random unit test run for 3.9, so I figure I'd fix it. This is unrelated to the PR otherwise.
| @@ -82,7 +82,7 @@ def fromStr(valstr): | |||
| val = bytes() | |||
| elif valstr.startswith("0x"): | |||
| # if needed convert to string with asBytes.decode('utf-8') | |||
| val = bytes.fromhex(valstr[2:]) | |||
| val = bytes.fromhex(valstr[2:].zfill(2)) | |||
There was a problem hiding this comment.
Added this because fromStr failed on 0x0 as fuzzed input. This is unrelated to the other changes, so I can pull this out if necessary.
There was a problem hiding this comment.
Glad to accept some test fixes alongside other stuff, so no worries there for sure!
|
Thanks @ianmcorvidae for the hints on the CI failures -- made it easy to get familiar with the CI stack on this repo. |
| @@ -82,7 +82,7 @@ def fromStr(valstr): | |||
| val = bytes() | |||
| elif valstr.startswith("0x"): | |||
| # if needed convert to string with asBytes.decode('utf-8') | |||
| val = bytes.fromhex(valstr[2:]) | |||
| val = bytes.fromhex(valstr[2:].zfill(2)) | |||
There was a problem hiding this comment.
Glad to accept some test fixes alongside other stuff, so no worries there for sure!
Couple things I noticed while working with the admin channel with the Python CLI:
I'm not 100% on the wording of the help messages -- it's not my strong suit. I've tested these changes manually,
but didn't make any changes to testsand added (hopefully enough) test coverage. I feel reasonably confident about the changes, but the changes add complexity that wasn't there.I'll note that in 2.5.x, the admin channel, especially with channel info retrieval, seems to be a lot more reliable, so this might be a lot more complexity that could be unnecessary. If you think that's the case, I think just customizing the timeout might be nice, since five minutes is a pretty long period of time.