-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat: pluggable URL parsing #41
base: master
Are you sure you want to change the base?
Conversation
requests_unixsocket/__init__.py
Outdated
DEFAULT_SCHEME = 'http+unix://' | ||
|
||
def default_urlparse(url): | ||
return UnixAdapter.Settings.ParseResult( |
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.
Seems a bit of a waste to urlparse url three times
how about:
parsed = urlparse(url)
return UnixAdapter...
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.
Yep, you're right. I will definitely update that. Thanks!
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.
Updated. Thanks again!
f43c18d
to
17b07ed
Compare
49b9339
to
cef606f
Compare
Example of usage: import json
from requests.compat import urlparse
from requests_unixsocket import Session, UnixAdapter
def custom_urlparse(url):
parsed_url = urlparse(url)
return UnixAdapter.Settings.ParseResult(
sockpath=parsed_url.path,
reqpath=parsed_url.fragment,
)
session = Session(settings=UnixAdapter.Settings(urlparse=custom_urlparse))
r = session.get('http+unix://sock.localhost/var/run/docker.sock#/info')
registry_config = r.json()['RegistryConfig']
print(json.dumps(registry_config, indent=4)) |
58c4061
to
365b053
Compare
This way people can customize it to their liking, as there a lot of opinions about this, as evidenced by the comments on GH-34. The default parsing is still the same as before, so new versions don't break existing code. But the user has the option of passing in a settings object, which has a `urlparse` attribute that can be set to a custom function that processes the URL and splits it into a `sockpath` and a `reqpath`. Sem-Ver: feature
Example of using customizable `urlparse` function
This new test tests using the `UnixAdapter` directly, like [httpie-unixsocket](https://github.com/httpie/httpie-unixsocket) does. I wrote this test because I found a case, on the `pluggable-urlparse` branch, where tests were passing, but executing: ``` http http+unix://%2Fvar%2Frun%2Fdocker.sock/info ``` was failing.
so it doesn't choke when used directly (e.g.: like [httpie-unixsocket](https://github.com/httpie/httpie-unixsocket) does) and no explicit settings are provided.
81e89a5
to
afbc9fd
Compare
@msabramo import json
from requests.compat import urlparse
from requests_unixsocket import Session, Settings
def custom_urlparse(url):
parsed_url = urlparse(url)
return Settings.ParseResult(
sockpath=parsed_url.path,
reqpath=parsed_url.fragment,
)
session = Session(settings=Settings(urlparse=custom_urlparse))
r = session.get('http+unix://sock.localhost/var/run/docker.sock#/info')
registry_config = r.json().get('RegistryConfig', {})
print(json.dumps(registry_config, indent=4)) |
This way people can customize it to their liking, as there a lot of
opinions about this, as evidenced by the comments on GH-34.
The default parsing is still the same as before, so new version don't
break existing code. But the user has the option of passing in a
settings object, which has a
urlparse
attribute that can be set to acustom function that processes the URL and splits into a
sockpath
anda
reqpath
.Sem-Ver: feature