jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/648200 )

Change subject: [cleanup] Cleanups and improvements from http.py and HttpRequest ......................................................................

[cleanup] Cleanups and improvements from http.py and HttpRequest

comms.threadedhttp.HttpRequest will be removed in favour of direct requests response. Therefore don't pass parameters to HttpRequest which are redundant to Requests or Response object. The Response contains the Request in its request attribute.

- deprecate all HttpRequest parameters except charset which is still used by encoding method until it is replaced by chardet - deprecate args, body and methods property but retrieve data either from Resonse or Requests object during the deprecation period. - deprecate kwargs property which isn't used anymore - remove uri setter and headers setter which were never used because the attributes where only set by HttpRequest parameters - deprecate the previously introduced all_headers attribute in favour of request.headers - update charset inside encoding method from request.headers - call callbacks in http after Request response directly - update the only HttpRequest() constructor with the remaining parameters - update error_handling_callback. Exceptions from requests contains request and response attributes which can be used - update http_tests.py

Bug: T265206 Change-Id: Icd91cd028a8588210ca33fbe4ebe30c24c54a2cf --- M pywikibot/comms/http.py M pywikibot/comms/threadedhttp.py M tests/http_tests.py 3 files changed, 93 insertions(+), 41 deletions(-)

Approvals: Mpaa: Looks good to me, approved jenkins-bot: Verified

diff --git a/pywikibot/comms/http.py b/pywikibot/comms/http.py index deea88f..3323d20 100644 --- a/pywikibot/comms/http.py +++ b/pywikibot/comms/http.py @@ -19,6 +19,7 @@ import atexit import sys

+from contextlib import suppress from http import cookiejar from string import Formatter from typing import Optional, Union @@ -355,7 +356,9 @@ raise Server414Error('Too long GET request')

if isinstance(request.data, Exception): - error('An error occurred for uri ' + request.url) + with suppress(Exception): + # request.data exception may contain response and request attribute + error('An error occurred for uri ' + request.data.request.url) raise request.data from None

# HTTP status 207 is also a success status for Webdav FINDPROP, @@ -491,8 +494,7 @@ callbacks.append(error_handling_callback)

charset = kwargs.pop('charset', None) - request = threadedhttp.HttpRequest( - uri, method, params, body, headers, callbacks, charset, **kwargs) + request = threadedhttp.HttpRequest(charset=charset)

auth = get_authentication(uri) if auth is not None and len(auth) == 4: @@ -519,7 +521,9 @@ request.data = e else: request.data = response - # error_handling_callback is called in HttpRequest data.setter + + for callback in callbacks: + callback(request)

# if there's no data in the answer we're in trouble try: diff --git a/pywikibot/comms/threadedhttp.py b/pywikibot/comms/threadedhttp.py index 9e22711..1836e7b 100644 --- a/pywikibot/comms/threadedhttp.py +++ b/pywikibot/comms/threadedhttp.py @@ -12,7 +12,12 @@ from urllib.parse import urlparse

import pywikibot -from pywikibot.tools import deprecated, deprecated_args, PYTHON_VERSION +from pywikibot.tools import ( + deprecated, + deprecated_args, + issue_deprecation_warning, + PYTHON_VERSION, +)

if PYTHON_VERSION >= (3, 9): Dict = dict @@ -31,32 +36,51 @@ * an exception """

- @deprecated_args(headers='all_headers', uri='url') - def __init__(self, url, method='GET', params=None, body=None, + @deprecated_args(uri=True, method=True, params=True, body=True, + headers=True, all_headers=True) + def __init__(self, url=None, method=None, params=None, body=None, all_headers=None, callbacks=None, charset=None, **kwargs): """Initializer.""" - self.url = url - self.method = method - self.params = params - self.body = body - self.all_headers = all_headers if isinstance(charset, codecs.CodecInfo): self.charset = charset.name - elif charset: - self.charset = charset - elif all_headers and 'accept-charset' in all_headers: - self.charset = all_headers['accept-charset'] else: - self.charset = None + self.charset = charset

- self.callbacks = callbacks - - self.args = [url, method, body, all_headers] - self.kwargs = kwargs - + self._kwargs = kwargs self._parsed_uri = None self._data = None

+ # deprecate positional parameters + if url: + issue_deprecation_warning("'url' parameter", depth=3, + warning_class=FutureWarning, + since='20201211') + if method: + issue_deprecation_warning("'method' parameter", + warning_class=FutureWarning, + since='20201211') + if params: + issue_deprecation_warning("'params' parameter", + warning_class=FutureWarning, + since='20201211') + if body: + issue_deprecation_warning("'body' parameter", + warning_class=FutureWarning, + since='20201211') + if all_headers: + issue_deprecation_warning("'all_headers' parameter", + warning_class=FutureWarning, + since='20201211') + if callbacks: + issue_deprecation_warning("'callbacks' parameter", + warning_class=FutureWarning, + since='20201211') + if kwargs: + for item in kwargs.items(): + issue_deprecation_warning('{}={!r} parameter'.format(*item), + warning_class=FutureWarning, + since='20201211') + def __getattr__(self, name): """Delegate undefined method calls to request.Response object.""" if self.exception and name in ('content', 'status_code'): @@ -64,30 +88,55 @@ return getattr(self.data, name)

@property + @deprecated(since='20201211', future_warning=True) + def args(self): # pragma: no cover + """DEPRECATED: Return predefined argument list.""" + return [ + self.url, + self.request.method, + self.request.body, + self.all_headers, + ] + + @property + @deprecated('the `request.body` attribute', + since='20201211', future_warning=True) + def body(self): # pragma: no cover + """DEPRECATED: Return request body attribute.""" + return self.request.body + + @property + @deprecated(since='20201211', future_warning=True) + def kwargs(self): # pragma: no cover + """DEPRECATED: Return request body attribute.""" + return self._kwargs + + @property + @deprecated('the `request.method` attribute', + since='20201211', future_warning=True) + def method(self): # pragma: no cover + """DEPRECATED: Return request body attribute.""" + return self.request.method + + @property @deprecated('the `url` attribute', since='20201011', future_warning=True) def uri(self): # pragma: no cover """DEPRECATED. Return the response URL.""" return self.url

- @uri.setter - @deprecated('the `url` attribute', since='20201011', future_warning=True) - def uri(self, value): # pragma: no cover - """DEPRECATED. Set the response URL.""" - self.url = value - @property - @deprecated('the `all_headers` property', since='20201011', + @deprecated('the `request.headers` property', since='20201011', future_warning=True) def headers(self): # pragma: no cover """DEPRECATED. Return the response headers.""" - return self.all_headers + return self.request.headers

- @headers.setter - @deprecated('the `all_headers` property', since='20201011', + @property + @deprecated('the `request.headers` property', since='20201211', future_warning=True) - def headers(self, value): # pragma: no cover - """DEPRECATED. Set the response headers.""" - self.all_headers = value + def all_headers(self): # pragma: no cover + """DEPRECATED. Return the response headers.""" + return self.request.headers

@property def data(self): @@ -106,10 +155,6 @@ """ self._data = value

- if self.callbacks: - for callback in self.callbacks: - callback(self) - @property def exception(self) -> Optional[Exception]: """DEPRECATED. Get the exception, if any. @@ -191,6 +236,9 @@ if hasattr(self, '_encoding'): return self._encoding

+ if self.charset is None and self.request is not None: + self.charset = self.request.headers.get('accept-charset') + if self.charset is None and self.header_encoding is None: pywikibot.log("Http response doesn't contain a charset.") charset = 'latin1' diff --git a/tests/http_tests.py b/tests/http_tests.py index 70d2efe..fbb13ac 100644 --- a/tests/http_tests.py +++ b/tests/http_tests.py @@ -403,11 +403,11 @@ @staticmethod def _create_request(charset=None, data=UTF8_BYTES): """Helper method.""" - req = threadedhttp.HttpRequest('', charset=charset) + req = threadedhttp.HttpRequest(charset=charset) resp = requests.Response() resp.headers = {'content-type': 'charset=utf-8'} resp._content = data[:] - req._data = resp + req.data = resp return req

def test_no_content_type(self):