Uh oh!
There was an error while loading. Please reload this page.
Port becomes optional - #92
Conversation
yeraydiazdiaz
left a comment
There was a problem hiding this comment.
LGTM, just a minor docstring note
| def __init__( | ||
| self, | ||
| proxy_origin: Origin, | ||
| proxy_url: URL, |
There was a problem hiding this comment.
OMG thanks! 😅
The docstring above needs to be updated.
florimondmanca
left a comment
There was a problem hiding this comment.
Very nice! I initially tackled this separately only to realize this PR already existed. :-) This implementation with the url_to_origin() helper is even better than what I came up with.
| def url_to_origin(url: URL) -> Origin: | ||
| scheme, host, explicit_port = url[:3] | ||
| default_port = {b'http': 80, b'https': 443}[scheme] |
There was a problem hiding this comment.
I was curious why Black didn't cough up on this usage of single quotes, only to realize it's not run in scripts/check.
It's been a while since I've been around here but I assume it (and the isort check) was removed on purpose from there?
Edit: OK, looks like we're planning to introduce black and flake8 back in #67
Tuple[bytes, bytes, int], since we need a canonical form there.proxy_originbecomesproxy_urlfor better consistency.HTTPX would need to update the
proxy_originform it uses toproxy_url.