Skip to content
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

Accept hyperlink.URLs #212

Closed
markrwilliams opened this issue Jan 6, 2018 · 1 comment · Fixed by #279
Closed

Accept hyperlink.URLs #212

markrwilliams opened this issue Jan 6, 2018 · 1 comment · Fixed by #279
Assignees

Comments

@markrwilliams
Copy link
Member

You can't pass treq a URL because of how twisted.web.client.Agent works:

import hyperlink, treq, twisted.internet.task

@twisted.internet.task.react
def run(reactor):
    return treq.get(hyperlink.URL.from_text(u"https://twistedmatrix.com/"))
  Traceback (most recent call last):
  File "/tmp/example.py", line 3, in <module>
    @twisted.internet.task.react
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/internet/task.py", line 908, in react
    finished = main(_reactor, *argv)
  File "/tmp/example.py", line 5, in run
    return treq.get(hyperlink.URL.from_text(u"https://twistedmatrix.com/"))
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/treq/api.py", line 24, in get
    return _client(**kwargs).get(url, headers=headers, **kwargs)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/treq/client.py", line 109, in get
    return self.request('GET', url, **kwargs)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/treq/client.py", line 218, in request
    bodyProducer=bodyProducer)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/web/client.py", line 1973, in request
    deferred = self._agent.request(method, uri, headers, bodyProducer)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/web/client.py", line 2041, in request
    deferred = self._agent.request(method, uri, headers, bodyProducer)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/web/client.py", line 1838, in request
    lastRequest = _FakeUrllib2Request(uri)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/web/client.py", line 1720, in __init__
    self.uri = nativeString(uri)
  File "/home/mrw/.virtualenvs/tmp-963c1ecd0f4cfd34/lib/python2.7/site-packages/twisted/python/compat.py", line 412, in nativeString
    raise TypeError("%r is neither bytes nor unicode" % s)
TypeError: URL.from_text(u'https://twistedmatrix.com/') is neither bytes nor unicode

python-hyper/hyperlink#55 means we can call bytes on URLs to get something we can pass Agent.request.

On the one hand, calling bytes on any old object seems bad, but on the other, we already use URL to convert unicode URLs to bytes.

If there's any API change, I favor adding an else that does url = bytes(url), but we'd also need to fix _combine_query_params.

However, I think I prefer that treq only accept bytes and unicode, and that we tell people to do bytes(theirURL) themselves.

@twm
Copy link
Contributor

twm commented Mar 18, 2020

I definitely want to accept hyperlink.URL directly. When I use treq I often use hyperlink.URL to build the URLs I pass to treq. Since treq uses the class internally to manage IDN (and, once #265 is merged, params), I see no real downside to accepting hyperlink.URL — it's strictly fewer conversions. Plus, accepting the class allows us to highlight hyperlink as the useful and correct library it is. I think that users will often be better-served with hyperlink than any of the functions in urllib.parse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants