Fix working with requests Session object
Creating a session per request does not make much sense - we're not using its benefits like connection pooling. This change creates a session per connector. Also correct sending Content-Type header without content. Change-Id: I597ada4582865d62f8559aef212ff79c7ddf6863
This commit is contained in:
parent
fe07266026
commit
11ec3540ae
|
@ -28,8 +28,14 @@ class Connector(object):
|
|||
|
||||
def __init__(self, url, username=None, password=None, verify=True):
|
||||
self._url = url
|
||||
self._auth = (username, password)
|
||||
self._verify = verify
|
||||
self._session = requests.Session()
|
||||
self._session.verify = verify
|
||||
if username and password:
|
||||
self._session.auth = (username, password)
|
||||
|
||||
def close(self):
|
||||
"""Close this connector and the associated HTTP session."""
|
||||
self._session.close()
|
||||
|
||||
def _op(self, method, path='', data=None, headers=None):
|
||||
"""Generic RESTful request handler.
|
||||
|
@ -46,37 +52,30 @@ class Connector(object):
|
|||
if headers is None:
|
||||
headers = {}
|
||||
|
||||
headers['Content-Type'] = 'application/json'
|
||||
if data is not None:
|
||||
data = json.dumps(data)
|
||||
headers['Content-Type'] = 'application/json'
|
||||
|
||||
with requests.Session() as session:
|
||||
session.headers = headers
|
||||
session.verify = self._verify
|
||||
url = parse.urljoin(self._url, path)
|
||||
# TODO(lucasagomes): We should mask the data to remove sensitive
|
||||
# information
|
||||
LOG.debug('HTTP request: %(method)s %(url)s; '
|
||||
'headers: %(headers)s; body: %(data)s',
|
||||
{'method': method, 'url': url, 'headers': headers,
|
||||
'data': data})
|
||||
try:
|
||||
response = self._session.request(method, url, data=data,
|
||||
headers=headers)
|
||||
except requests.ConnectionError as e:
|
||||
raise exceptions.ConnectionError(url=url, error=e)
|
||||
|
||||
if all(v is not None for v in self._auth):
|
||||
session.auth = self._auth
|
||||
exceptions.raise_for_response(method, url, response)
|
||||
LOG.debug('HTTP response for %(method)s %(url)s: '
|
||||
'status code: %(code)s',
|
||||
{'method': method, 'url': url,
|
||||
'code': response.status_code})
|
||||
|
||||
if data is not None:
|
||||
data = json.dumps(data)
|
||||
|
||||
url = parse.urljoin(self._url, path)
|
||||
# TODO(lucasagomes): We should mask the data to remove sensitive
|
||||
# information
|
||||
LOG.debug('HTTP request: %(method)s %(url)s; '
|
||||
'headers: %(headers)s; body: %(data)s',
|
||||
{'method': method, 'url': url, 'headers': headers,
|
||||
'data': data})
|
||||
try:
|
||||
response = session.request(method, url, data=data)
|
||||
except requests.ConnectionError as e:
|
||||
raise exceptions.ConnectionError(url=url, error=e)
|
||||
|
||||
exceptions.raise_for_response(method, url, response)
|
||||
LOG.debug('HTTP response for %(method)s %(url)s: '
|
||||
'status code: %(code)s',
|
||||
{'method': method, 'url': url,
|
||||
'code': response.status_code})
|
||||
|
||||
return response
|
||||
return response
|
||||
|
||||
def get(self, path='', data=None, headers=None):
|
||||
"""HTTP GET method.
|
||||
|
@ -113,3 +112,9 @@ class Connector(object):
|
|||
:raises: HTTPError
|
||||
"""
|
||||
return self._op('PATCH', path, data, headers)
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *_args):
|
||||
self.close()
|
||||
|
|
|
@ -15,7 +15,6 @@
|
|||
|
||||
import json
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
import requests
|
||||
|
||||
|
@ -62,32 +61,28 @@ class ConnectorOpTestCase(base.TestCase):
|
|||
password='pass', verify=True)
|
||||
self.data = {'fake': 'data'}
|
||||
self.headers = {'X-Fake': 'header'}
|
||||
self.session_fixture = self.useFixture(
|
||||
fixtures.MockPatchObject(connector.requests, 'Session',
|
||||
autospec=True)
|
||||
)
|
||||
self.session = (
|
||||
self.session_fixture.mock.return_value.__enter__.return_value)
|
||||
self.session = mock.Mock(spec=requests.Session)
|
||||
self.conn._session = self.session
|
||||
self.request = self.session.request
|
||||
|
||||
def _test_op(self, headers, expected_headers):
|
||||
self.request.return_value.status_code = 200
|
||||
|
||||
self.conn._op('GET', path='fake/path', data=self.data,
|
||||
headers=headers)
|
||||
def test_ok_get(self):
|
||||
expected_headers = self.headers.copy()
|
||||
|
||||
self.conn._op('GET', path='fake/path', headers=self.headers)
|
||||
self.request.assert_called_once_with(
|
||||
'GET', 'http://foo.bar:1234/fake/path',
|
||||
data='{"fake": "data"}')
|
||||
self.assertEqual(expected_headers, self.session.headers)
|
||||
data=None, headers=expected_headers)
|
||||
|
||||
def test_ok_with_headers(self):
|
||||
def test_ok_post(self):
|
||||
expected_headers = self.headers.copy()
|
||||
expected_headers['Content-Type'] = 'application/json'
|
||||
self._test_op(self.headers, expected_headers)
|
||||
|
||||
def test_ok_no_headers(self):
|
||||
expected_headers = {'Content-Type': 'application/json'}
|
||||
self._test_op(None, expected_headers)
|
||||
self.conn._op('POST', path='fake/path', data=self.data,
|
||||
headers=self.headers)
|
||||
self.request.assert_called_once_with(
|
||||
'POST', 'http://foo.bar:1234/fake/path',
|
||||
data=json.dumps(self.data), headers=expected_headers)
|
||||
|
||||
def test_connection_error(self):
|
||||
self.request.side_effect = requests.exceptions.ConnectionError
|
||||
|
|
|
@ -12,7 +12,6 @@ oslotest>=1.10.0 # Apache-2.0
|
|||
testrepository>=0.0.18 # Apache-2.0/BSD
|
||||
testscenarios>=0.4 # Apache-2.0/BSD
|
||||
testtools>=1.4.0 # MIT
|
||||
fixtures>=3.0.0 # Apache-2.0/BSD
|
||||
|
||||
# releasenotes
|
||||
reno>=1.8.0 # Apache-2.0
|
||||
|
|
Loading…
Reference in New Issue