added test for HTTPVersionConfig type error #253
Conversation
lovelydinosaur
left a comment
lovelydinosaur
left a comment
There was a problem hiding this comment.
Good call, yup!
One minor point for consideration.
httpx/config.py
Outdated
| } | ||
| else: | ||
| self.http_versions = {version.upper() for version in http_versions} | ||
| raise ValueError(f"Unsupported HTTP version {http_versions!r}.") |
There was a problem hiding this comment.
I guess we should probably raise a type error instead. Something like...
TypeError("HTTP version should be a string or list of strings, but got {type(http_versions)}")?
cansarigol
force-pushed
the
version-config-ref
branch
2 times, most recently
from
37f7915 to
1e5055a
Compare
httpx/config.py
Outdated
| @@ -1,5 +1,6 @@ | |||
| import ssl | |||
| import typing | |||
| from collections.abc import Iterable | |||
There was a problem hiding this comment.
Why collections.abc.Iterable instead of typing.Iterable?
https://docs.python.org/3/library/typing.html#typing.Iterable
There was a problem hiding this comment.
I didn't think to use types like this :) thanks
httpx/config.py
Outdated
| else: | ||
| self.http_versions = {version.upper() for version in http_versions} | ||
| raise TypeError( | ||
| f"HTTP version should be a string or list of strings, but got {type(http_versions)}" |
There was a problem hiding this comment.
You can break this into two strings to appease flake8:
| f"HTTP version should be a string or list of strings, but got {type(http_versions)}" | |
| "HTTP version should be a string or list of strings," | |
| f" but got {type(http_versions)}" |
There was a problem hiding this comment.
thanks, I overlooked flake8 error. if flake8 is not successful, nox can be stopped.
cansarigol
force-pushed
the
version-config-ref
branch
from
1e5055a to
52308f4
Compare
sethmlarson
left a comment
sethmlarson
left a comment
There was a problem hiding this comment.
Looks good, thanks @cansarigol!
4 participants
Related #241