-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split matrix server selection into utility function #3427
Conversation
I also don't see the |
Codecov Report
@@ Coverage Diff @@
## master #3427 +/- ##
=========================================
Coverage ? 76.05%
=========================================
Files ? 100
Lines ? 12960
Branches ? 1824
=========================================
Hits ? 9857
Misses ? 2461
Partials ? 642
Continue to review full report at Codecov.
|
Use typing.cast to explain mypy what types we know comes from that account_data dict
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.
Looks good
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.
Lgtm.
One question, below
available_servers = [config['server']] | ||
else: | ||
raise TransportError('Invalid matrix server specified (valid values: "auto" or a URL)') | ||
|
||
def _http_retry_delay() -> Iterable[float]: |
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.
Shouldn’t this be part of the utility?
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.
@ulope this had too much to do with internals and variables of config
, and I wanted to make the utility function as agnostic in relation to config as possible, that's why I kept this outside of it, but can be as easily used outside raiden as any other utility function, so we can say it's already split
Follow-up of #3393 , implement some leftovers from that PR