From 3f4671a0b7eb5c300291c2e92082d16b96e6316c Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Mon, 11 May 2015 11:53:03 +1000 Subject: [PATCH] Cleanup session creation 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 --- ironicclient/common/http.py | 94 ++++++++++++++++++++-------- ironicclient/shell.py | 3 - ironicclient/tests/unit/test_http.py | 62 ++++++------------ 3 files changed, 87 insertions(+), 72 deletions(-) diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index ad311bd69..1b41bd24f 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -439,8 +439,18 @@ class VerifiedHTTPSConnection(six.moves.http_client.HTTPSConnection): class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): """HTTP client based on Keystone client session.""" - conflict_max_retries = DEFAULT_MAX_RETRIES - conflict_retry_interval = DEFAULT_RETRY_INTERVAL + def __init__(self, + 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): return self._generic_parse_version_headers(resp.headers.get) @@ -530,29 +540,61 @@ class ResponseBodyIterator(object): raise StopIteration() -def _construct_http_client(*args, **kwargs): - session = kwargs.pop('session', None) - auth = kwargs.pop('auth', None) - +def _construct_http_client(endpoint, + session=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: - service_type = kwargs.pop('service_type', 'baremetal') - interface = kwargs.pop('endpoint_type', None) - region_name = kwargs.pop('region_name', None) - os_ironic_api_version = kwargs.pop('os_ironic_api_version', - DEFAULT_VER) - session_client = SessionClient(session=session, - auth=auth, - interface=interface, - service_type=service_type, - region_name=region_name, - service_name=None, - user_agent='python-ironicclient') - # Append an ironic specific variable to session - session_client.os_ironic_api_version = os_ironic_api_version - session_client.api_version_select_state = ( - kwargs.pop('api_version_select_state', 'default')) - session_client.conflict_max_retries = kwargs.get('max_retries') - session_client.conflict_retry_interval = kwargs.get('retry_interval') - return session_client + kwargs.setdefault('service_type', 'baremetal') + kwargs.setdefault('user_agent', 'python-ironicclient') + kwargs.setdefault('interface', kwargs.pop('endpoint_type', None)) + kwargs.setdefault('endpoint_override', endpoint) + + ignored = {'token': token, + 'auth_ref': auth_ref, + 'timeout': timeout != 600, + 'ca_file': ca_file, + 'cert_file': cert_file, + 'key_file': key_file, + 'insecure': insecure} + + dvars = [k for k, v in six.iteritems(ignored) if v] + + if dvars: + LOG.warn('The following arguments are ignored when using the ' + 'session to construct a client: %s', ', '.join(dvars)) + + 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: - 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) diff --git a/ironicclient/shell.py b/ironicclient/shell.py index 192de33d5..36260a156 100644 --- a/ironicclient/shell.py +++ b/ironicclient/shell.py @@ -515,14 +515,11 @@ class IronicShell(object): endpoint_type = args.os_endpoint_type or 'publicURL' kwargs = { - 'auth_url': args.os_auth_url, 'session': keystone_session, 'auth': keystone_auth, 'service_type': service_type, 'endpoint_type': endpoint_type, 'region_name': args.os_region_name, - 'username': args.os_username, - 'password': args.os_password, } kwargs['os_ironic_api_version'] = os_ironic_api_version if args.max_retries < 0: diff --git a/ironicclient/tests/unit/test_http.py b/ironicclient/tests/unit/test_http.py index f5b9fc6fd..b33631e9e 100644 --- a/ironicclient/tests/unit/test_http.py +++ b/ironicclient/tests/unit/test_http.py @@ -39,6 +39,18 @@ def _get_error_body(faultstring=None, debuginfo=None): 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): def setUp(self): @@ -410,12 +422,7 @@ class SessionClientTest(utils.BaseTestCase): error_body, 500) - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) error = self.assertRaises(exc.InternalServerError, client.json_request, @@ -434,12 +441,7 @@ class SessionClientTest(utils.BaseTestCase): error_body, 500) - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) error = self.assertRaises(exc.InternalServerError, client.json_request, @@ -457,12 +459,7 @@ class SessionClientTest(utils.BaseTestCase): None, 506) expected_result = ('1.1', '1.6') - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) result = client._parse_version_headers(fake_session) self.assertEqual(expected_result, result) @@ -569,13 +566,7 @@ class RetriesTestCase(utils.BaseTestCase): fake_session = mock.Mock(spec=utils.FakeSession) fake_session.request.side_effect = iter((fake_resp, ok_resp)) - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) - + client = _session_client(session=fake_session) client.json_request('GET', '/v1/resources') 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.request.return_value = fake_resp - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) self.assertRaises(exc.Conflict, client.json_request, 'GET', '/v1/resources') @@ -611,12 +597,7 @@ class RetriesTestCase(utils.BaseTestCase): fake_session = mock.Mock(spec=utils.FakeSession) fake_session.request.return_value = fake_resp - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) client.conflict_max_retries = None self.assertRaises(exc.Conflict, client.json_request, @@ -634,12 +615,7 @@ class RetriesTestCase(utils.BaseTestCase): fake_session = mock.Mock(spec=utils.FakeSession) fake_session.request.return_value = fake_resp - client = http.SessionClient(session=fake_session, - auth=None, - interface=None, - service_type='publicURL', - region_name='', - service_name=None) + client = _session_client(session=fake_session) client.conflict_max_retries = http.DEFAULT_MAX_RETRIES + 1 self.assertRaises(exc.Conflict, client.json_request,