jenkins-bot has submitted this change. (
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/658058 )
Change subject: [IMPR] return requests.Response with http.request
......................................................................
[IMPR] return requests.Response with http.request
- return requests.Response with http.request; this is a breaking change
but usual not used directly
- modify api.Request._http_request to return the requests.Response object
- modify api.Request._json_loads to use Resonse object and its json()
method directly. Remove old compat code and ignore site.encoding()
- update getImagePageHtml
- remove api_tests.TestAPIMWException; fetch test was never used and
was not functional. Now remove the requests tests too including
utils.DummyHttp and PatchedHttp because both does not work with
requests.Response object and it i to heavy to fix.
Bug: T265206
Change-Id: I4c9e18935cbec8ef3a6f7d974498b0fb140de31b
---
M pywikibot/comms/http.py
M pywikibot/data/api.py
M pywikibot/page/__init__.py
M tests/api_tests.py
M tests/utils.py
5 files changed, 35 insertions(+), 263 deletions(-)
Approvals:
JJMC89: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/comms/http.py b/pywikibot/comms/http.py
index 1c7f6e2..3e2748f 100644
--- a/pywikibot/comms/http.py
+++ b/pywikibot/comms/http.py
@@ -11,7 +11,7 @@
- Basic HTTP error handling
"""
#
-# (C) Pywikibot team, 2007-2020
+# (C) Pywikibot team, 2007-2021
#
# Distributed under the terms of the MIT license.
#
@@ -215,25 +215,26 @@
@deprecated_args(body='data')
-def request(site, uri: Optional[str] = None, headers=None, **kwargs) -> str:
+def request(site,
+ uri: Optional[str] = None,
+ headers: Optional[dict] = None,
+ **kwargs) -> requests.Response:
"""
Request to Site with default error handling and response decoding.
See L{requests.Session.request} for additional parameters.
- If the site argument is provided, the uri is a relative uri from
- and including the document root '/'.
-
- If the site argument is None, the uri must be absolute.
+ The optional uri is a relative uri from site base uri including the
+ document root '/'.
@param site: The Site to connect to
- @type site: L{pywikibot.site.BaseSite}
+ @type: site: pywikibot.site.BaseSite
@param uri: the URI to retrieve
@keyword charset: Either a valid charset (usable for str.decode()) or None
to automatically chose the charset from the returned header (defaults
to latin-1)
@type charset: CodecInfo, str, None
- @return: The received data
+ @return: The received data Response
"""
kwargs.setdefault('verify', site.verify_SSL_certificate())
old_validation = kwargs.pop('disable_ssl_certificate_validation', None)
@@ -254,7 +255,7 @@
baseuri = site.base_url(uri)
r = fetch(baseuri, headers=headers, **kwargs)
site.throttle.retry_after = int(r.headers.get('retry-after', 0))
- return r.text
+ return r
def get_authentication(uri: str) -> Optional[Tuple[str, str]]:
diff --git a/pywikibot/data/api.py b/pywikibot/data/api.py
index bde7b4f..2106b0a 100644
--- a/pywikibot/data/api.py
+++ b/pywikibot/data/api.py
@@ -7,7 +7,6 @@
import datetime
import hashlib
import inspect
-import json
import os
import pickle
import pprint
@@ -1534,16 +1533,17 @@
_logger)
return use_get, uri, body, headers
- def _http_request(self, use_get, uri, body, headers, paramstring) -> tuple:
+ def _http_request(self, use_get: bool, uri: str, data, headers,
+ paramstring) -> tuple:
"""Get or post a http request with exception handling.
- @return: a tuple containing data from request and use_get value
+ @return: a tuple containing requests.Response object from
+ http.request and use_get value
"""
try:
- data = http.request(
- self.site, uri=uri,
- method='GET' if use_get else 'POST',
- data=body, headers=headers)
+ response = http.request(self.site, uri=uri,
+ method='GET' if use_get else 'POST',
+ data=data, headers=headers)
except Server504Error:
pywikibot.log('Caught HTTP 504 error; retrying')
except Server414Error:
@@ -1564,32 +1564,28 @@
pywikibot.error(traceback.format_exc())
pywikibot.log('{}, {}'.format(uri, paramstring))
else:
- return data, use_get
+ return response, use_get
self.wait()
return None, use_get
- def _json_loads(self, data: Union[str, bytes]) -> Optional[dict]:
- """Read source text and return a dict.
+ def _json_loads(self, response) -> Optional[dict]:
+ """Return a dict from requests.Response.
- @param data: raw data string
+ @param response: a requests.Response object
+ @type response: requests.Response
@return: a data dict
@raises APIError: unknown action found
@raises APIError: unknown query result type
"""
- if not isinstance(data, str):
- data = data.decode(self.site.encoding())
- pywikibot.debug(('API response received from {}:\n'
- .format(self.site)) + data, _logger)
- if data.startswith('unknown_action'):
- raise APIError(data[:14], data[16:])
try:
- result = json.loads(data)
+ result = response.json()
except ValueError:
# if the result isn't valid JSON, there must be a server
# problem. Wait a few seconds and try again
pywikibot.warning(
'Non-JSON response received from server {}; '
- 'the server may be down.'.format(self.site))
+ 'the server may be down.\nStatus code:{}'
+ .format(self.site, response.status_code))
# there might also be an overflow, so try a smaller limit
for param in self._params:
if param.endswith('limit'):
@@ -1600,11 +1596,6 @@
pywikibot.output('Set {} = {}'
.format(param, self[param]))
else:
- if result and not isinstance(result, dict):
- raise APIError('Unknown',
- 'Unable to process query response of type {}.'
- .format(type(result)),
- data=result)
return result or {}
self.wait()
return None
@@ -1804,12 +1795,12 @@
use_get, uri, body, headers = self._get_request_params(use_get,
paramstring)
- rawdata, use_get = self._http_request(use_get, uri, body, headers,
- paramstring)
- if rawdata is None:
+ response, use_get = self._http_request(use_get, uri, body, headers,
+ paramstring)
+ if response is None:
continue
- result = self._json_loads(rawdata)
+ result = self._json_loads(response)
if result is None:
continue
diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py
index a803780..7970018 100644
--- a/pywikibot/page/__init__.py
+++ b/pywikibot/page/__init__.py
@@ -2347,17 +2347,16 @@
self.site.loadimageinfo(self, history=True)
return self._file_revisions
- def getImagePageHtml(self):
- """
- Download the file page, and return the HTML, as a string.
+ def getImagePageHtml(self) -> str:
+ """Download the file page, and return the HTML, as a string.
Caches the HTML code, so that if you run this method twice on the
same FilePage object, the page will only be downloaded once.
"""
if not hasattr(self, '_imagePageHtml'):
- path = '%s/index.php?title=%s' \
- % (self.site.scriptpath(), self.title(as_url=True))
- self._imagePageHtml = http.request(self.site, path)
+ path = '{}/index.php?title={}'.format(self.site.scriptpath(),
+ self.title(as_url=True))
+ self._imagePageHtml = http.request(self.site, path).text
return self._imagePageHtml
@deprecated('get_file_url', since='20160609', future_warning=True)
diff --git a/tests/api_tests.py b/tests/api_tests.py
index 8974259..86f36a0 100644
--- a/tests/api_tests.py
+++ b/tests/api_tests.py
@@ -9,7 +9,6 @@
from collections import defaultdict
from contextlib import suppress
-from urllib.parse import unquote_to_bytes
import pywikibot.data.api as api
import pywikibot.family
@@ -27,105 +26,7 @@
DefaultDrySiteTestCase,
)
from tests import patch
-from tests.utils import FakeLoginManager, PatchedHttp
-
-
-class TestAPIMWException(DefaultSiteTestCase):
-
- """Test raising an APIMWException."""
-
- user = True
-
- data = {'error': {'code': 'internal_api_error_fake',
- 'info': 'Fake error message'},
- 'servedby': 'unittest',
- }
-
- def _dummy_request(self, *args, **kwargs):
- self.assertLength(args, 1) # one positional argument for http.request
- site = args[0]
- self.assertIsInstance(site, pywikibot.BaseSite)
- self.assertIn('data', kwargs)
- self.assertIn('uri', kwargs)
- if kwargs['data'] is None:
- # use uri and remove script path
- parameters = kwargs['uri']
- prefix = site.scriptpath() + '/api.php?'
- self.assertEqual(prefix, parameters[:len(prefix)])
- parameters = parameters[len(prefix):]
- else:
- parameters = kwargs['data']
- parameters = parameters.encode('ascii') # it should be bytes anyway
- # Extract parameter data from the body, it's ugly but allows us
- # to verify that we actually test the right request
- parameters = [p.split(b'=', 1) for p in parameters.split(b'&')]
- keys = [p[0].decode('ascii') for p in parameters]
- values = [unquote_to_bytes(p[1]) for p in parameters]
- values = [v.decode(site.encoding()) for v in values]
- values = [v.replace('+', ' ') for v in values]
- values = [set(v.split('|')) for v in values]
- parameters = dict(zip(keys, values))
-
- if 'fake' not in parameters:
- return False # do an actual request
- if self.assert_parameters:
- for param, value in self.assert_parameters.items():
- self.assertIn(param, parameters)
- if value is not None:
- if isinstance(value, str):
- value = value.split('|')
- self.assertLessEqual(set(value), parameters[param])
- return self.data
-
- def setUp(self):
- """Mock warning and error."""
- super().setUp()
- self.warning_patcher = patch.object(pywikibot, 'warning')
- self.error_patcher = patch.object(pywikibot, 'error')
- self.warning_patcher.start()
- self.error_patcher.start()
-
- def tearDown(self):
- """Check warning and error calls."""
- self.warning_patcher.stop()
- self.error_patcher.stop()
- super().tearDown()
-
- def _test_assert_called_with(self, req):
- with self.assertRaises(api.APIMWException):
- req.submit()
- pywikibot.warning.assert_called_with(
- 'API error internal_api_error_fake: Fake error message')
- pywikibot.error.assert_called_with(
- 'Detected MediaWiki API exception internal_api_error_fake: '
- 'Fake error message\n[servedby: unittest]; raising')
-
- def test_API_error(self):
- """Test a static request."""
- req = api.Request(site=self.site, parameters={'action': 'query',
- 'fake': True})
- with PatchedHttp(api, self.data):
- self._test_assert_called_with(req)
-
- def test_API_error_encoding_ASCII(self):
- """Test a Page instance as parameter using ASCII chars."""
- page = pywikibot.page.Page(self.site, 'ASCII')
- req = api.Request(site=self.site, parameters={'action': 'query',
- 'fake': True,
- 'titles': page})
- self.assert_parameters = {'fake': ''}
- with PatchedHttp(api, self._dummy_request):
- self._test_assert_called_with(req)
-
- def test_API_error_encoding_Unicode(self):
- """Test a Page instance as parameter using non-ASCII chars."""
- page = pywikibot.page.Page(self.site, 'Ümlä üt')
- req = api.Request(site=self.site, parameters={'action': 'query',
- 'fake': True,
- 'titles': page})
- self.assert_parameters = {'fake': ''}
- with PatchedHttp(api, self._dummy_request):
- self._test_assert_called_with(req)
+from tests.utils import FakeLoginManager
class TestApiFunctions(DefaultSiteTestCase):
diff --git a/tests/utils.py b/tests/utils.py
index 291f11d..5025dbd 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -5,18 +5,14 @@
# Distributed under the terms of the MIT license.
#
import inspect
-import json
import os
import sys
import warnings
-from collections.abc import Mapping
from contextlib import contextmanager
from subprocess import PIPE, Popen, TimeoutExpired
from types import ModuleType
-from requests import Response
-
import pywikibot
from pywikibot import config
@@ -449,122 +445,6 @@
pass
-class DummyHttp(object):
-
- """A class simulating the http module."""
-
- def __init__(self, wrapper):
- """Initializer with the given PatchedHttp instance."""
- self.__wrapper = wrapper
-
- def request(self, *args, **kwargs):
- """The patched request method."""
- result = self.__wrapper.before_request(*args, **kwargs)
- if result is False:
- result = self.__wrapper._old_http.request(*args, **kwargs)
- elif isinstance(result, Mapping):
- result = json.dumps(result)
- elif not isinstance(result, str):
- raise ValueError('The result is not a valid type '
- '"{0}"'.format(type(result)))
- response = self.__wrapper.after_request(result, *args, **kwargs)
- if response is None:
- response = result
- return response
-
- def fetch(self, *args, **kwargs):
- """The patched fetch method."""
- result = self.__wrapper.before_fetch(*args, **kwargs)
- if result is False:
- result = self.__wrapper._old_http.fetch(*args, **kwargs)
- elif not isinstance(result, Response):
- raise ValueError('The result is not a valid type "{}"'
- .format(type(result)))
- response = self.__wrapper.after_fetch(result, *args, **kwargs)
- if response is None:
- response = result
- return response
-
-
-class PatchedHttp(object):
-
- """
- A ContextWrapper to handle any data going through the http module.
-
- This patches the C{http} import in the given module to a class simulating
- C{request} and C{fetch}. It has a C{data} attribute which is either a
- static value which the requests will return or it's a callable returning
- the data. If it's a callable it'll be called with the same parameters as
- the original function in the L{http} module. For fine grained control it's
- possible to override/monkey patch the C{before_request} and C{before_fetch}
- methods. By default they just return C{data} directory or call it if it's
- callable.
-
- Even though L{http.request} is calling L{http.fetch}, it won't call the
- patched method.
-
- The data returned for C{request} may either be C{False}, a C{str} or a
- C{Mapping} which is converted into a json string. The data returned for
- C{fetch} can only be C{False} or a L{requests.Response}. For both
- variants any other types are not allowed and if it is False it'll use the
- original method and do an actual request.
-
- Afterwards it is always calling C{after_request} or C{after_fetch} with the
- response and given arguments. That can return a different response too, but
- can also return None so that the original response is forwarded.
- """
-
- def __init__(self, module, data=None):
- """
- Initializer.
-
- @param module: The given module to patch. It must have the http module
- imported as http.
- @type module: Module
- @param data: The data returned for any request or fetch.
- @type data: callable or False (or other depending on request/fetch)
- """
- super().__init__()
- self._module = module
- self.data = data
-
- def _handle_data(self, *args, **kwargs):
- """Return the data after it may have been called."""
- if self.data is None:
- raise ValueError('No handler is defined.')
-
- if callable(self.data):
- return self.data(*args, **kwargs)
-
- return self.data
-
- def before_request(self, *args, **kwargs):
- """Return the value which should is returned by request."""
- return self._handle_data(*args, **kwargs)
-
- def before_fetch(self, *args, **kwargs):
- """Return the value which should is returned by fetch."""
- return self._handle_data(*args, **kwargs)
-
- def after_request(self, response, *args, **kwargs):
- """Handle the response after request."""
- pass
-
- def after_fetch(self, response, *args, **kwargs):
- """Handle the response after fetch."""
- pass
-
- def __enter__(self):
- """Patch the http module property."""
- self._old_http = self._module.http
- self._module.http = DummyHttp(self)
- return self
-
- def __exit__(self, exc_type, exc_value, traceback):
- """Reset the http module property."""
- self._module.http = self._old_http
-
-
def execute(command, data_in=None, timeout=None, error=None):
"""
Execute a command and capture outputs.
--
To view, visit https://gerrit.wikimedia.org/r/c/pywikibot/core/+/658058
To unsubscribe, or for help writing mail filters, visit
https://gerrit.wikimedia.org/r/settings
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: I4c9e18935cbec8ef3a6f7d974498b0fb140de31b
Gerrit-Change-Number: 658058
Gerrit-PatchSet: 3
Gerrit-Owner: Xqt <[email protected]>
Gerrit-Reviewer: JJMC89 <[email protected]>
Gerrit-Reviewer: Mpaa <[email protected]>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged
_______________________________________________
Pywikibot-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/pywikibot-commits
