added netrc support #177
added netrc support #177
Conversation
httpx/config.py
Outdated
| @@ -39,9 +39,16 @@ class SSLConfig: | |||
| SSL Configuration. | |||
| """ | |||
|
|
|||
| def __init__(self, *, cert: CertTypes = None, verify: VerifyTypes = True): | |||
| def __init__( | |||
There was a problem hiding this comment.
Let's revert usages of trust_env for SSLConfig. The default behavior in the future should be use system trust stores unless we're specifically told which cert bundle to use.
Although this change reminded me to create another issue for SSL_CERT_... environment variables.
There was a problem hiding this comment.
You're right new approach for environment variables would be nice.
I reverted the last commit.
There was a problem hiding this comment.
So the whole trust_env parameter is kind of a can of worms. Ian, Cory, and @nateprewitt have all dealt with it and there are large issue threads i can dig up about all the issues.
The biggest issue is basically we're going to be embedded into applications/tools that don't expose a way to control how we configure ourselves but users still need to configure us for proxies, authentication, certificates, etc. I think the best way to achieve that is still environment variables but then you get into priorities like "both REQUESTS_CA_CERT and SSL_CERT_FILE ste defined and are different, what do we do?"
We need answers to these questions for sure, maybe a variable that can control order?
httpx/utils.py
Outdated
| @@ -64,3 +66,12 @@ def guess_json_utf(data: bytes) -> typing.Optional[str]: | |||
| return "utf-32-le" | |||
| # Did not detect a valid UTF-32 ascii-range character | |||
| return None | |||
|
|
|||
|
|
|||
| def get_netrc_login(url: str) -> typing.Union[typing.Tuple[str, str, str], None]: | |||
There was a problem hiding this comment.
Signature should either be host or authority here, right?
It's not clear to me from a glance at the netrc docs, which of those two is actually the correct thing to use?
Also we should prefer typing.Optional[typing.Tuple[str, str, str]] as the return annotation.
There was a problem hiding this comment.
machine in the config. I changed it as host I can change again if necessary
httpx/utils.py
Outdated
|
|
||
| def get_netrc_login(url: str) -> typing.Union[typing.Tuple[str, str, str], None]: | ||
| try: | ||
| _netrc = netrc.netrc(os.environ.get("DEFAULT_NETRC_PATH")) # type: ignore |
There was a problem hiding this comment.
I'd probably prefer a variable name that's not underscore prefixed here. netrc_info maybe?
There was a problem hiding this comment.
Is DEFAULT_NETRC_PATH a standardized thing? (Or used by requests or something?)
There was a problem hiding this comment.
I made it up to explain the initial path.
There was a problem hiding this comment.
I'm pretty sure NETRC is the env variable to specify the path
tests/.netrc
Outdated
| @@ -0,0 +1,3 @@ | |||
| machine test_entrc.httpx.com | |||
There was a problem hiding this comment.
Let's use the example.org domain, since it's there for this sort of thing.
There was a problem hiding this comment.
If appropriate, I used netrcexample.org. Because some tests are affected by example.org.
tests/.netrc
Outdated
| @@ -0,0 +1,3 @@ | |||
| machine test_entrc.httpx.com | |||
| login tomchristie | |||
| password password123 | |||
There was a problem hiding this comment.
I'd probably marginally prefer that we use a fake style username here example-username and example-password or something. Just feels marginally cleaner.
|
Thanks, I applied your comments. |
tests/client/test_auth.py
Outdated
| @@ -67,3 +68,27 @@ def auth(request): | |||
|
|
|||
| assert response.status_code == 200 | |||
| assert response.json() == {"auth": "Token 123"} | |||
|
|
|||
|
|
|||
| def test_entrc_auth(): | |||
There was a problem hiding this comment.
Typo: entrc -> netrc
|
Great stuff, thanks! |
No description provided.