diff --git a/src/treq/_utils.py b/src/treq/_utils.py index 69719ff9..e715847a 100644 --- a/src/treq/_utils.py +++ b/src/treq/_utils.py @@ -4,6 +4,9 @@ from __future__ import absolute_import, division, print_function +import inspect +import warnings + from twisted.web.client import HTTPConnectionPool @@ -45,3 +48,50 @@ def default_pool(reactor, pool, persistent): set_global_pool(HTTPConnectionPool(reactor, persistent=True)) return get_global_pool() + + +def warn(*args, **kwargs): + """ + Utility function to raise warnings with stack level set automatically to + the first position outside a given namespace. + + Notice that if you pass the stacklevel keyword, it will be increased by 1 + in order to ignore this wrapper function and that will be passed directly + to warnings.warn. That is: it is functionally equivalent to warnings.warn. + + @param namespace: calls within this namespace (aka startswith) will not + be accounted for in the stacklevel variable if possible. + If not defined, the default is the top level package of this code. + @param default_stacklevel: in those cases in which there is no code + outside of namespace in the stack, we will default to this value. + This utility function already takes into account the fact that it is a + wrapper around warnings.warn and secretly passes default_stacklevel+1 + when necessary. + Defaults to 1, which would match the place where this was called from. + """ + # Default to top-level package + namespace = kwargs.pop('namespace', __name__.partition('.')[0]) + default_stacklevel = kwargs.pop('default_stacklevel', 1) + stacklevel = kwargs.pop('stacklevel', None) + + if stacklevel is not None: + # Increase in order to ignore this wrapper function + # Users would not expect this function to count. + stacklevel += 1 + else: + stack = inspect.stack() + + # We skip the first stack frame in order to be able to use this + # utility function from other packages. + for current_stacklevel, frame_info in enumerate(stack[1:], 2): + module = inspect.getmodule(frame_info[0]) + module_name = module.__name__ if module else None + if not (module_name and module_name.startswith(namespace + '.')): + break + stacklevel = current_stacklevel + + if stacklevel is None: + stacklevel = default_stacklevel + 1 + kwargs['stacklevel'] = stacklevel + + warnings.warn(*args, **kwargs) diff --git a/src/treq/api.py b/src/treq/api.py index 2ad09e7d..ba36728b 100644 --- a/src/treq/api.py +++ b/src/treq/api.py @@ -12,7 +12,8 @@ def head(url, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).head(url, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.head(url, **filtered_kwargs) def get(url, headers=None, **kwargs): @@ -21,7 +22,8 @@ def get(url, headers=None, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).get(url, headers=headers, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.get(url, headers=headers, **filtered_kwargs) def post(url, data=None, **kwargs): @@ -30,7 +32,8 @@ def post(url, data=None, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).post(url, data=data, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.post(url, data=data, **filtered_kwargs) def put(url, data=None, **kwargs): @@ -39,7 +42,8 @@ def put(url, data=None, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).put(url, data=data, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.put(url, data=data, **filtered_kwargs) def patch(url, data=None, **kwargs): @@ -48,7 +52,8 @@ def patch(url, data=None, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).patch(url, data=data, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.patch(url, data=data, **filtered_kwargs) def delete(url, **kwargs): @@ -57,7 +62,8 @@ def delete(url, **kwargs): See :py:func:`treq.request` """ - return _client(**kwargs).delete(url, **kwargs) + client, filtered_kwargs = _client(**kwargs) + return client.delete(url, **filtered_kwargs) def request(method, url, **kwargs): @@ -81,6 +87,10 @@ def request(method, url, **kwargs): :param data: Optional request body. :type data: str, file-like, IBodyProducer, or None + :param files: If present, issue a ``multipart/form-data`` request as it + is better suited for cases with files and/or large objects. + :type files: list of (unicode, file-like) for (filename, file). + :param json: Optional JSON-serializable content to pass in body. :type json: dict, list/tuple, int, string/unicode, bool, or None @@ -116,19 +126,18 @@ def request(method, url, **kwargs): :rtype: Deferred that fires with an IResponse provider. """ - return _client(**kwargs).request(method, url, **kwargs) + client, filtered_kwargs = _client(kwargs) + return client.request(method, url, **filtered_kwargs) # # Private API # -def _client(*args, **kwargs): - agent = kwargs.get('agent') +def _client(reactor=None, agent=None, pool=None, persistent=None, **kwargs): if agent is None: - reactor = default_reactor(kwargs.get('reactor')) - pool = default_pool(reactor, - kwargs.get('pool'), - kwargs.get('persistent')) + reactor = default_reactor(reactor) + pool = default_pool(reactor, pool, persistent) agent = Agent(reactor, pool=pool) - return HTTPClient(agent) + + return HTTPClient(agent), dict(kwargs, reactor=reactor) diff --git a/src/treq/client.py b/src/treq/client.py index dca36682..551a8f41 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -29,7 +29,7 @@ from twisted.python.components import registerAdapter from json import dumps as json_dumps -from treq._utils import default_reactor +from treq._utils import default_reactor, warn from treq.auth import add_auth from treq import multipart from treq.response import _Response @@ -40,9 +40,7 @@ def urlencode(query, doseq): return _urlencode(query, doseq).encode('ascii') - from http.cookiejar import CookieJar else: - from cookielib import CookieJar from urlparse import urlunparse from urllib import urlencode @@ -98,6 +96,10 @@ def deliverBody(self, protocol): self._waiters.append(protocol) +# Dummy object to identify data that is not JSON. +_NO_JSON = object() + + class HTTPClient(object): def __init__(self, agent, cookiejar=None, data_to_body_producer=IBodyProducer): @@ -141,15 +143,28 @@ def delete(self, url, **kwargs): """ return self.request('DELETE', url, **kwargs) - def request(self, method, url, **kwargs): + def request(self, method, url, + headers=None, params=None, data=None, files=None, + auth=None, cookies=None, allow_redirects=True, + browser_like_redirects=False, unbuffered=False, + timeout=None, reactor=None, json=_NO_JSON, **kwargs): """ See :func:`treq.request()`. """ + if kwargs: + warn("".join([ + "treq.client.HttpClient.request will only accept ", + "supported documented arguments. ", + "This will raise an Exception in the future. ", + "Unsupported arguments: [", + ",".join(kwargs.keys()), + "]"]), + category=DeprecationWarning, namespace='treq') + method = method.encode('ascii').upper() # Join parameters provided in the URL # and the ones passed as argument. - params = kwargs.get('params') if params: url = _combine_query_params(url, params) @@ -158,7 +173,6 @@ def request(self, method, url, **kwargs): # Convert headers dictionary to # twisted raw headers format. - headers = kwargs.get('headers') if headers: if isinstance(headers, dict): h = Headers({}) @@ -175,15 +189,13 @@ def request(self, method, url, **kwargs): # Here we choose a right producer # based on the parameters passed in. bodyProducer = None - data = kwargs.get('data') - files = kwargs.get('files') # since json=None needs to be serialized as 'null', we need to - # explicitly check kwargs for this key - has_json = 'json' in kwargs + # perform a different kind of check. + has_json = json is not _NO_JSON if files: # If the files keyword is present we will issue a - # multipart/form-data request as it suits better for cases + # multipart/form-data request as it is better suited for cases # with files and/or large objects. files = list(_convert_files(files)) boundary = str(uuid.uuid4()).encode('ascii') @@ -209,20 +221,15 @@ def request(self, method, url, **kwargs): # If data is sent as json, set Content-Type as 'application/json' headers.setRawHeaders( b'content-type', [b'application/json; charset=UTF-8']) - content = kwargs['json'] - json = json_dumps(content, separators=(u',', u':')).encode('utf-8') + json = json_dumps(json, separators=(u',', u':')).encode('utf-8') bodyProducer = self._data_to_body_producer(json) - cookies = kwargs.get('cookies', {}) - - if not isinstance(cookies, CookieJar): - cookies = cookiejar_from_dict(cookies) - + cookies = dict() if cookies is None else cookies cookies = merge_cookies(self._cookiejar, cookies) wrapped_agent = CookieAgent(self._agent, cookies) - if kwargs.get('allow_redirects', True): - if kwargs.get('browser_like_redirects', False): + if allow_redirects: + if browser_like_redirects: wrapped_agent = BrowserLikeRedirectAgent(wrapped_agent) else: wrapped_agent = RedirectAgent(wrapped_agent) @@ -230,7 +237,6 @@ def request(self, method, url, **kwargs): wrapped_agent = ContentDecoderAgent(wrapped_agent, [(b'gzip', GzipDecoder)]) - auth = kwargs.get('auth') if auth: wrapped_agent = add_auth(wrapped_agent, auth) @@ -238,9 +244,8 @@ def request(self, method, url, **kwargs): method, url, headers=headers, bodyProducer=bodyProducer) - timeout = kwargs.get('timeout') if timeout: - delayedCall = default_reactor(kwargs.get('reactor')).callLater( + delayedCall = default_reactor(reactor).callLater( timeout, d.cancel) def gotResult(result): @@ -250,7 +255,7 @@ def gotResult(result): d.addBoth(gotResult) - if not kwargs.get('unbuffered', False): + if not unbuffered: d.addCallback(_BufferedResponse) return d.addCallback(_Response, cookies) @@ -340,9 +345,9 @@ def _guess_content_type(filename): if not _PY3: from StringIO import StringIO + from types import FileType registerAdapter(_from_file, StringIO, IBodyProducer) - # Suppress lint failure on Python 3. - registerAdapter(_from_file, file, IBodyProducer) # noqa: F821 + registerAdapter(_from_file, FileType, IBodyProducer) else: import io # file()/open() equiv on Py3 diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py index eed2e558..15613b39 100644 --- a/src/treq/test/test_client.py +++ b/src/treq/test/test_client.py @@ -420,6 +420,59 @@ def test_request_browser_like_redirects(self): self.assertEqual(self.successResultOf(d).original, final_resp) + def _check_unknown_arguments(self, method, caller, *args, **kwargs): + response = mock.Mock(headers=Headers({})) + self.agent.request.return_value = succeed(response) + + d = getattr(self.client, method)(_idontexist=True, *args, **kwargs) + + self.successResultOf(d) + + warnings = self.flushWarnings(offendingFunctions=[caller]) + + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0]['category'], DeprecationWarning) + self.assertRegex( + warnings[0]['message'], + "^treq.client.HttpClient.request will only accept " + "supported documented arguments. " + "This will raise an Exception in the future. ") + + def test_request_unknown_arguments(self): + self._check_unknown_arguments('request', + self.test_request_unknown_arguments, + 'GET', 'www.http://example.com') + + def test_get_unknown_arguments(self): + self._check_unknown_arguments('get', + self.test_get_unknown_arguments, + 'www.http://example.com') + + def test_put_unknown_arguments(self): + self._check_unknown_arguments('put', + self.test_put_unknown_arguments, + 'www.http://example.com') + + def test_patch_unknown_arguments(self): + self._check_unknown_arguments('patch', + self.test_patch_unknown_arguments, + 'www.http://example.com') + + def test_post_unknown_arguments(self): + self._check_unknown_arguments('post', + self.test_post_unknown_arguments, + 'www.http://example.com') + + def test_head_unknown_arguments(self): + self._check_unknown_arguments('head', + self.test_head_unknown_arguments, + 'www.http://example.com') + + def test_delete_unknown_arguments(self): + self._check_unknown_arguments('delete', + self.test_delete_unknown_arguments, + 'www.http://example.com') + class BodyBufferingProtocolTests(TestCase): def test_buffers_data(self):