openstack/python-ironicclient
Code Issues Proposed changes

Cleanup session creation

Browse Source 
When using the session object we want to pass through unknown kwargs to
the adapter so that keystoneclient can add new parameters as required.

This requires fixing the old options so that we can restrict what is
passed and showing a warning when there are values that are being
ignored.

Change-Id: Ia795dfc8e16c60a08df1636816daefe2ebbd0b03
This commit is contained in:
Jamie Lennox 2015-05-11 11:53:03 +10:00
parent 4f290a8d5e
commit 3f4671a0b7
3 changed files with 87 additions and 72 deletions
ironicclient
common
http.py
shell.py
tests/unit
test_http.py

94
ironicclient/common/http.py

@ -439,8 +439,18 @@ class VerifiedHTTPSConnection(six.moves.http_client.HTTPSConnection):
class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter):
"""HTTP client based on Keystone client session.""" """HTTP client based on Keystone client session."""
conflict_max_retries = DEFAULT_MAX_RETRIES def __init__(self,
conflict_retry_interval = DEFAULT_RETRY_INTERVAL os_ironic_api_version,
api_version_select_state,
max_retries,
retry_interval,
**kwargs):
self.os_ironic_api_version = os_ironic_api_version
self.api_version_select_state = api_version_select_state
self.conflict_max_retries = max_retries
self.conflict_retry_interval = retry_interval
super(SessionClient, self).__init__(**kwargs)
def _parse_version_headers(self, resp): def _parse_version_headers(self, resp):
return self._generic_parse_version_headers(resp.headers.get) return self._generic_parse_version_headers(resp.headers.get)
@ -530,29 +540,61 @@ class ResponseBodyIterator(object):
raise StopIteration() raise StopIteration()
def _construct_http_client(*args, **kwargs): def _construct_http_client(endpoint,
session = kwargs.pop('session', None) session=None,
auth = kwargs.pop('auth', None) token=None,
auth_ref=None,
os_ironic_api_version=DEFAULT_VER,
api_version_select_state='default',
max_retries=DEFAULT_MAX_RETRIES,
retry_interval=DEFAULT_RETRY_INTERVAL,
timeout=600,
ca_file=None,
cert_file=None,
key_file=None,
insecure=None,
**kwargs):
if session: if session:
service_type = kwargs.pop('service_type', 'baremetal') kwargs.setdefault('service_type', 'baremetal')
interface = kwargs.pop('endpoint_type', None) kwargs.setdefault('user_agent', 'python-ironicclient')
region_name = kwargs.pop('region_name', None) kwargs.setdefault('interface', kwargs.pop('endpoint_type', None))
os_ironic_api_version = kwargs.pop('os_ironic_api_version', kwargs.setdefault('endpoint_override', endpoint)
DEFAULT_VER)
session_client = SessionClient(session=session, ignored = {'token': token,
auth=auth, 'auth_ref': auth_ref,
interface=interface, 'timeout': timeout != 600,
service_type=service_type, 'ca_file': ca_file,
region_name=region_name, 'cert_file': cert_file,
service_name=None, 'key_file': key_file,
user_agent='python-ironicclient') 'insecure': insecure}
# Append an ironic specific variable to session
session_client.os_ironic_api_version = os_ironic_api_version dvars = [k for k, v in six.iteritems(ignored) if v]
session_client.api_version_select_state = (
kwargs.pop('api_version_select_state', 'default')) if dvars:
session_client.conflict_max_retries = kwargs.get('max_retries') LOG.warn('The following arguments are ignored when using the '
session_client.conflict_retry_interval = kwargs.get('retry_interval') 'session to construct a client: %s', ', '.join(dvars))
return session_client
return SessionClient(session=session,
os_ironic_api_version=os_ironic_api_version,
api_version_select_state=api_version_select_state,
max_retries=max_retries,
retry_interval=retry_interval,
**kwargs)
else: else:
return HTTPClient(*args, **kwargs) if kwargs:
LOG.warn('The following arguments are being ignored when '
'constructing the client: %s', ', '.join(kwargs))
return HTTPClient(endpoint=endpoint,
token=token,
auth_ref=auth_ref,
os_ironic_api_version=os_ironic_api_version,
api_version_select_state=api_version_select_state,
max_retries=max_retries,
retry_interval=retry_interval,
timeout=timeout,
ca_file=ca_file,
cert_file=cert_file,
key_file=key_file,
insecure=insecure)

3
ironicclient/shell.py

@ -515,14 +515,11 @@ class IronicShell(object):
endpoint_type = args.os_endpoint_type or 'publicURL' endpoint_type = args.os_endpoint_type or 'publicURL'
kwargs = { kwargs = {
'auth_url': args.os_auth_url,
'session': keystone_session, 'session': keystone_session,
'auth': keystone_auth, 'auth': keystone_auth,
'service_type': service_type, 'service_type': service_type,
'endpoint_type': endpoint_type, 'endpoint_type': endpoint_type,
'region_name': args.os_region_name, 'region_name': args.os_region_name,
'username': args.os_username,
'password': args.os_password,
} }
kwargs['os_ironic_api_version'] = os_ironic_api_version kwargs['os_ironic_api_version'] = os_ironic_api_version
if args.max_retries < 0: if args.max_retries < 0:

62
ironicclient/tests/unit/test_http.py

@ -39,6 +39,18 @@ def _get_error_body(faultstring=None, debuginfo=None):
return raw_body return raw_body
def _session_client(**kwargs):
return http.SessionClient(os_ironic_api_version='1.6',
api_version_select_state='default',
max_retries=5,
retry_interval=2,
auth=None,
interface=None,
service_type='publicURL',
region_name='',
**kwargs)
class VersionNegotiationMixinTest(utils.BaseTestCase): class VersionNegotiationMixinTest(utils.BaseTestCase):
def setUp(self): def setUp(self):
@ -410,12 +422,7 @@ class SessionClientTest(utils.BaseTestCase):
error_body, error_body,
500) 500)
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
error = self.assertRaises(exc.InternalServerError, error = self.assertRaises(exc.InternalServerError,
client.json_request, client.json_request,
@ -434,12 +441,7 @@ class SessionClientTest(utils.BaseTestCase):
error_body, error_body,
500) 500)
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
error = self.assertRaises(exc.InternalServerError, error = self.assertRaises(exc.InternalServerError,
client.json_request, client.json_request,
@ -457,12 +459,7 @@ class SessionClientTest(utils.BaseTestCase):
None, None,
506) 506)
expected_result = ('1.1', '1.6') expected_result = ('1.1', '1.6')
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
result = client._parse_version_headers(fake_session) result = client._parse_version_headers(fake_session)
self.assertEqual(expected_result, result) self.assertEqual(expected_result, result)
@ -569,13 +566,7 @@ class RetriesTestCase(utils.BaseTestCase):
fake_session = mock.Mock(spec=utils.FakeSession) fake_session = mock.Mock(spec=utils.FakeSession)
fake_session.request.side_effect = iter((fake_resp, ok_resp)) fake_session.request.side_effect = iter((fake_resp, ok_resp))
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
client.json_request('GET', '/v1/resources') client.json_request('GET', '/v1/resources')
self.assertEqual(2, fake_session.request.call_count) self.assertEqual(2, fake_session.request.call_count)
@ -589,12 +580,7 @@ class RetriesTestCase(utils.BaseTestCase):
fake_session = mock.Mock(spec=utils.FakeSession) fake_session = mock.Mock(spec=utils.FakeSession)
fake_session.request.return_value = fake_resp fake_session.request.return_value = fake_resp
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
self.assertRaises(exc.Conflict, client.json_request, self.assertRaises(exc.Conflict, client.json_request,
'GET', '/v1/resources') 'GET', '/v1/resources')
@ -611,12 +597,7 @@ class RetriesTestCase(utils.BaseTestCase):
fake_session = mock.Mock(spec=utils.FakeSession) fake_session = mock.Mock(spec=utils.FakeSession)
fake_session.request.return_value = fake_resp fake_session.request.return_value = fake_resp
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
client.conflict_max_retries = None client.conflict_max_retries = None
self.assertRaises(exc.Conflict, client.json_request, self.assertRaises(exc.Conflict, client.json_request,
@ -634,12 +615,7 @@ class RetriesTestCase(utils.BaseTestCase):
fake_session = mock.Mock(spec=utils.FakeSession) fake_session = mock.Mock(spec=utils.FakeSession)
fake_session.request.return_value = fake_resp fake_session.request.return_value = fake_resp
client = http.SessionClient(session=fake_session, client = _session_client(session=fake_session)
auth=None,
interface=None,
service_type='publicURL',
region_name='',
service_name=None)
client.conflict_max_retries = http.DEFAULT_MAX_RETRIES + 1 client.conflict_max_retries = http.DEFAULT_MAX_RETRIES + 1
self.assertRaises(exc.Conflict, client.json_request, self.assertRaises(exc.Conflict, client.json_request,