From 992ab47d46a357e6dd39de5a7ef7f3c3b1c16f2c Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 11:30:29 -0800 Subject: [PATCH 01/15] Raise TypeError --- src/treq/client.py | 10 +--------- src/treq/test/test_api.py | 14 ++------------ src/treq/test/test_client.py | 20 ++++++++------------ 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 98b56892..8b144d08 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -228,15 +228,7 @@ def gotResult(result): d.addCallback(_BufferedResponse) if kwargs: - warnings.warn( - ( - "Got unexpected keyword argument: {}." - " treq will ignore this argument," - " but will raise TypeError in the next treq release." - ).format(", ".join(repr(k) for k in kwargs)), - DeprecationWarning, - stacklevel=stacklevel, - ) + raise TypeError("Unexpected keyword arguments: {!r}".format(kwargs)) return d.addCallback(_Response, cookies) diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py index 782a35f4..7f2aa16c 100644 --- a/src/treq/test/test_api.py +++ b/src/treq/test/test_api.py @@ -101,25 +101,15 @@ def test_request_invalid_param(self): This test verifies that stacklevel is set appropriately when issuing the warning. """ - self.failureResultOf( + with self.assertRaises(TypeError) as c: treq.request( "GET", "https://foo.bar", invalid=True, pool=SyntacticAbominationHTTPConnectionPool(), ) - ) - [w] = self.flushWarnings([self.test_request_invalid_param]) - self.assertEqual(DeprecationWarning, w["category"]) - self.assertEqual( - ( - "Got unexpected keyword argument: 'invalid'." - " treq will ignore this argument," - " but will raise TypeError in the next treq release." - ), - w["message"], - ) + self.assertIn("invalid", str(c.exception)) def test_post_json_with_data(self): """ diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py index e4481c62..41006f6d 100644 --- a/src/treq/test/test_client.py +++ b/src/treq/test/test_client.py @@ -656,19 +656,15 @@ def test_request_dict_headers_invalid_values(self): def test_request_invalid_param(self): """ - `HTTPClient.request()` warns that invalid parameters are ignored and - that this is deprecated. + `HTTPClient.request()` rejects invalid keyword parameters with + `TypeError`. """ - self.client.request('GET', b'http://example.com', invalid=True) - - [w] = self.flushWarnings([self.test_request_invalid_param]) - self.assertEqual( - ( - "Got unexpected keyword argument: 'invalid'." - " treq will ignore this argument," - " but will raise TypeError in the next treq release." - ), - w['message'], + self.assertRaises( + TypeError, + self.client.request, + "GET", + b"http://example.com", + invalid=True, ) @with_clock From a3fb01a07f5a2260ac2325e33931d8b9f495c81b Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 11:39:49 -0800 Subject: [PATCH 02/15] Move unbuffered --- src/treq/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 8b144d08..2ea0aca0 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -145,7 +145,7 @@ def delete(self, url, **kwargs): kwargs.setdefault('_stacklevel', 3) return self.request('DELETE', url, **kwargs) - def request(self, method, url, **kwargs): + def request(self, method, url, *, unbuffered=False, **kwargs): """ See :func:`treq.request()`. """ @@ -224,7 +224,7 @@ def gotResult(result): d.addBoth(gotResult) - if not kwargs.pop('unbuffered', False): + if not unbuffered: d.addCallback(_BufferedResponse) if kwargs: From dc6ff45e57217c2edd6ea6c75b01baba041fb149 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 11:41:01 -0800 Subject: [PATCH 03/15] Move allow_redirects and browser_like_redirects --- src/treq/client.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 2ea0aca0..638120d8 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -145,7 +145,7 @@ def delete(self, url, **kwargs): kwargs.setdefault('_stacklevel', 3) return self.request('DELETE', url, **kwargs) - def request(self, method, url, *, unbuffered=False, **kwargs): + def request(self, method, url, *, allow_redirects=True, browser_like_redirects=False, unbuffered=False, **kwargs): """ See :func:`treq.request()`. """ @@ -192,8 +192,7 @@ def request(self, method, url, *, unbuffered=False, **kwargs): cookies = merge_cookies(self._cookiejar, cookies) wrapped_agent = CookieAgent(self._agent, cookies) - browser_like_redirects = kwargs.pop('browser_like_redirects', False) - if kwargs.pop('allow_redirects', True): + if allow_redirects: if browser_like_redirects: wrapped_agent = BrowserLikeRedirectAgent(wrapped_agent) else: From 6e23024984869d0476bb92fabc8a3dc0c4510a6a Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 11:41:39 -0800 Subject: [PATCH 04/15] Sad face --- src/treq/client.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index 638120d8..0c7d752e 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -145,7 +145,16 @@ def delete(self, url, **kwargs): kwargs.setdefault('_stacklevel', 3) return self.request('DELETE', url, **kwargs) - def request(self, method, url, *, allow_redirects=True, browser_like_redirects=False, unbuffered=False, **kwargs): + def request( + self, + method, + url, + *, + allow_redirects=True, + browser_like_redirects=False, + unbuffered=False, + **kwargs + ): """ See :func:`treq.request()`. """ From beda6dc951f6485d82cf5bff1e1a3525373510e2 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 11:42:41 -0800 Subject: [PATCH 05/15] Move reactor --- src/treq/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index 0c7d752e..5e95ad76 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -153,6 +153,7 @@ def request( allow_redirects=True, browser_like_redirects=False, unbuffered=False, + reactor=None, **kwargs ): """ @@ -218,7 +219,6 @@ def request( method, url, headers=headers, bodyProducer=bodyProducer) - reactor = kwargs.pop('reactor', None) if reactor is None: from twisted.internet import reactor timeout = kwargs.pop('timeout', None) From 18e4268ad9805eb0ec26391eb3969eb25b03d25c Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:06:23 -0800 Subject: [PATCH 06/15] Move timeout --- src/treq/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index 5e95ad76..85be1340 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -154,6 +154,7 @@ def request( browser_like_redirects=False, unbuffered=False, reactor=None, + timeout=None, **kwargs ): """ @@ -221,7 +222,6 @@ def request( if reactor is None: from twisted.internet import reactor - timeout = kwargs.pop('timeout', None) if timeout: delayedCall = reactor.callLater(timeout, d.cancel) From efae9e8163c6df8826bfcacf5abbe7b983070880 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:06:33 -0800 Subject: [PATCH 07/15] Move cookies --- src/treq/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 85be1340..fec9dff7 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -150,6 +150,7 @@ def request( method, url, *, + cookies=None, allow_redirects=True, browser_like_redirects=False, unbuffered=False, @@ -195,8 +196,6 @@ def request( if contentType is not None: headers.setRawHeaders(b'Content-Type', [contentType]) - cookies = kwargs.pop('cookies', {}) - if not isinstance(cookies, CookieJar): cookies = cookiejar_from_dict(cookies) From 0a667d54eb119dfc9c6ab95b06c2a35a710c96cf Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:09:21 -0800 Subject: [PATCH 08/15] Move params --- src/treq/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index fec9dff7..132f7f0f 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -150,6 +150,7 @@ def request( method, url, *, + params=None, cookies=None, allow_redirects=True, browser_like_redirects=False, @@ -177,7 +178,6 @@ def request( # Join parameters provided in the URL # and the ones passed as argument. - params = kwargs.pop('params', None) if params: parsed_url = parsed_url.replace( query=parsed_url.query + tuple(_coerced_query_params(params)) From 989fec8cd0edf6b216b7694e2c110330942873fd Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:09:50 -0800 Subject: [PATCH 09/15] Move headers --- src/treq/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index 132f7f0f..cdd155b5 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -151,6 +151,7 @@ def request( url, *, params=None, + headers=None, cookies=None, allow_redirects=True, browser_like_redirects=False, @@ -185,7 +186,7 @@ def request( url = parsed_url.to_uri().to_text().encode('ascii') - headers = self._request_headers(kwargs.pop('headers', None), stacklevel + 1) + headers = self._request_headers(headers, stacklevel + 1) bodyProducer, contentType = self._request_body( data=kwargs.pop('data', None), From 32fc1fff7f1d6dbdbce0247af3dde248b87bf78d Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:12:02 -0800 Subject: [PATCH 10/15] Move data, files, and json --- src/treq/client.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index cdd155b5..0bdf7a80 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -152,6 +152,9 @@ def request( *, params=None, headers=None, + data=None, + files=None, + json=_NOTHING, cookies=None, allow_redirects=True, browser_like_redirects=False, @@ -188,12 +191,8 @@ def request( headers = self._request_headers(headers, stacklevel + 1) - bodyProducer, contentType = self._request_body( - data=kwargs.pop('data', None), - files=kwargs.pop('files', None), - json=kwargs.pop('json', _NOTHING), - stacklevel=stacklevel + 1, - ) + bodyProducer, contentType = self._request_body(data, files, json, + stacklevel=stacklevel + 1) if contentType is not None: headers.setRawHeaders(b'Content-Type', [contentType]) From 2d436a9bc04eae731e2862531e9efcc5e7a375e3 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:15:18 -0800 Subject: [PATCH 11/15] Move stacklevel --- src/treq/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 0bdf7a80..c3a54af2 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -161,13 +161,13 @@ def request( unbuffered=False, reactor=None, timeout=None, + _stacklevel=2, **kwargs ): """ See :func:`treq.request()`. """ method = method.encode('ascii').upper() - stacklevel = kwargs.pop('_stacklevel', 2) if isinstance(url, DecodedURL): parsed_url = url.encoded_url @@ -189,10 +189,10 @@ def request( url = parsed_url.to_uri().to_text().encode('ascii') - headers = self._request_headers(headers, stacklevel + 1) + headers = self._request_headers(headers, _stacklevel + 1) bodyProducer, contentType = self._request_body(data, files, json, - stacklevel=stacklevel + 1) + stacklevel=_stacklevel + 1) if contentType is not None: headers.setRawHeaders(b'Content-Type', [contentType]) From df23052d6a49e325059223881d41f8e56211d2f2 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:15:42 -0800 Subject: [PATCH 12/15] Move auth --- src/treq/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index c3a54af2..b1dda9a3 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -155,6 +155,7 @@ def request( data=None, files=None, json=_NOTHING, + auth=None, cookies=None, allow_redirects=True, browser_like_redirects=False, @@ -211,7 +212,6 @@ def request( wrapped_agent = ContentDecoderAgent(wrapped_agent, [(b'gzip', GzipDecoder)]) - auth = kwargs.pop('auth', None) if auth: wrapped_agent = add_auth(wrapped_agent, auth) From d652b108db72cd0dadaf46899e7172e6a72cc073 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 16 Jan 2021 12:16:34 -0800 Subject: [PATCH 13/15] Remove kwargs --- src/treq/client.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index b1dda9a3..aca2dae6 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -163,7 +163,6 @@ def request( reactor=None, timeout=None, _stacklevel=2, - **kwargs ): """ See :func:`treq.request()`. @@ -234,9 +233,6 @@ def gotResult(result): if not unbuffered: d.addCallback(_BufferedResponse) - if kwargs: - raise TypeError("Unexpected keyword arguments: {!r}".format(kwargs)) - return d.addCallback(_Response, cookies) def _request_headers(self, headers, stacklevel): From 147f75baeeed6550a03559370125a41cf632b6db Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sun, 24 Jan 2021 15:12:00 -0800 Subject: [PATCH 14/15] Juice coverage --- src/treq/test/test_multipart.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/treq/test/test_multipart.py b/src/treq/test/test_multipart.py index fd170e3d..dc47ecdd 100644 --- a/src/treq/test/test_multipart.py +++ b/src/treq/test/test_multipart.py @@ -80,11 +80,17 @@ def test_unknownLength(self): """ class HasSeek: def seek(self, offset, whence): - pass + """ + A C{seek} method that is never called because there is no + matching C{tell} method. + """ class HasTell: def tell(self): - pass + """ + A C{tell} method that is never called because there is no + matching C{seek} method. + """ producer = MultiPartProducer( {"f": ("name", None, FileBodyProducer(HasSeek()))}) From 96bd854955d9c126696ad1126e5509df6d83ba2f Mon Sep 17 00:00:00 2001 From: Tom Most Date: Mon, 1 Feb 2021 21:35:17 -0800 Subject: [PATCH 15/15] Address review feedback --- src/treq/test/test_multipart.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/treq/test/test_multipart.py b/src/treq/test/test_multipart.py index dc47ecdd..d1930373 100644 --- a/src/treq/test/test_multipart.py +++ b/src/treq/test/test_multipart.py @@ -78,14 +78,14 @@ def test_unknownLength(self): passed as a parameter without either a C{seek} or C{tell} method, its C{length} attribute is set to C{UNKNOWN_LENGTH}. """ - class HasSeek: + class CantTell: def seek(self, offset, whence): """ A C{seek} method that is never called because there is no matching C{tell} method. """ - class HasTell: + class CantSeek: def tell(self): """ A C{tell} method that is never called because there is no @@ -93,11 +93,11 @@ def tell(self): """ producer = MultiPartProducer( - {"f": ("name", None, FileBodyProducer(HasSeek()))}) + {"f": ("name", None, FileBodyProducer(CantTell()))}) self.assertEqual(UNKNOWN_LENGTH, producer.length) producer = MultiPartProducer( - {"f": ("name", None, FileBodyProducer(HasTell()))}) + {"f": ("name", None, FileBodyProducer(CantSeek()))}) self.assertEqual(UNKNOWN_LENGTH, producer.length) def test_knownLengthOnFile(self):