Provide a clear error message when using client.Client without a session
Currently we fail with _construct_http_client() takes at least 1 argument.
This change provides a proper TypeError and updates documentation to make
it clear where a session is required. Also provided are explicit unit
tests on passing a session via various means.
Change-Id: I96073dc80d225a9b88fdc12bb058c0145aca623b
(cherry picked from commit e8914a7ef9
)
This commit is contained in:
parent
8188c01489
commit
1220d76857
|
@ -22,7 +22,7 @@ LOG = logging.getLogger(__name__)
|
|||
|
||||
|
||||
def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
||||
max_retries=None, retry_interval=None, **kwargs):
|
||||
max_retries=None, retry_interval=None, session=None, **kwargs):
|
||||
"""Get an authenticated client, based on the credentials.
|
||||
|
||||
:param api_version: the API version to use. Valid value: '1'.
|
||||
|
@ -31,6 +31,8 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
|||
:param max_retries: Maximum number of retries in case of conflict error
|
||||
:param retry_interval: Amount of time (in seconds) between retries in case
|
||||
of conflict error.
|
||||
:param session: An existing keystoneauth session. Will be created from
|
||||
kwargs if not provided.
|
||||
:param kwargs: all the other params that are passed to keystoneauth.
|
||||
"""
|
||||
# TODO(TheJulia): At some point, we should consider possibly noting
|
||||
|
@ -48,7 +50,6 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
|||
auth_type = 'token'
|
||||
else:
|
||||
auth_type = 'password'
|
||||
session = kwargs.get('session')
|
||||
if not session:
|
||||
loader = kaloading.get_plugin_loader(auth_type)
|
||||
loader_options = loader.get_options()
|
||||
|
@ -104,8 +105,22 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
|||
return Client(api_version, **ironicclient_kwargs)
|
||||
|
||||
|
||||
def Client(version, *args, **kwargs):
|
||||
def Client(version, endpoint_override=None, session=None, *args, **kwargs):
|
||||
"""Create a client of an appropriate version.
|
||||
|
||||
This call requires a session. If you want it to be created, use
|
||||
``get_client`` instead.
|
||||
|
||||
:param endpoint_override: A bare metal endpoint to use.
|
||||
:param session: A keystoneauth session to use. This argument is actually
|
||||
required and is marked optional only for backward compatibility.
|
||||
:param args: Other arguments to pass to the HTTP client. Not recommended,
|
||||
use kwargs instead.
|
||||
:param kwargs: Other keyword arguments to pass to the HTTP client (e.g.
|
||||
``insecure``).
|
||||
"""
|
||||
module = importutils.import_versioned_module('ironicclient',
|
||||
version, 'client')
|
||||
client_class = getattr(module, 'Client')
|
||||
return client_class(*args, **kwargs)
|
||||
return client_class(endpoint_override=endpoint_override, session=session,
|
||||
*args, **kwargs)
|
||||
|
|
|
@ -265,3 +265,21 @@ class ClientTest(utils.BaseTestCase):
|
|||
}
|
||||
self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client,
|
||||
'1', **kwargs)
|
||||
|
||||
def test_client_no_session(self):
|
||||
# get_client can create a session, all other calls require it
|
||||
self.assertRaisesRegex(TypeError,
|
||||
"session is required",
|
||||
iroclient.Client,
|
||||
1, "http://example.com")
|
||||
|
||||
def test_client_session_via_posargs(self):
|
||||
session = mock.Mock()
|
||||
session.get_endpoint.return_value = 'http://localhost:35357/v2.0'
|
||||
iroclient.Client('1', "http://example.com", session)
|
||||
|
||||
def test_client_session_via_kwargs(self):
|
||||
session = mock.Mock()
|
||||
session.get_endpoint.return_value = 'http://localhost:35357/v2.0'
|
||||
iroclient.Client('1', session=session,
|
||||
endpoint_override="http://example.com")
|
||||
|
|
|
@ -115,3 +115,25 @@ class ClientTest(utils.BaseTestCase):
|
|||
# is being called in the client and returns a version,
|
||||
# although mocking might need to be restrutured to
|
||||
# properly achieve that.
|
||||
|
||||
def test_client_no_session(self, http_client_mock):
|
||||
self.assertRaisesRegex(TypeError,
|
||||
"session is required",
|
||||
client.Client,
|
||||
"http://example.com")
|
||||
|
||||
def test_client_session_via_posargs(self, http_client_mock):
|
||||
session = mock.Mock()
|
||||
client.Client("http://example.com", session)
|
||||
http_client_mock.assert_called_once_with(
|
||||
session, api_version_select_state='default',
|
||||
endpoint_override="http://example.com",
|
||||
os_ironic_api_version=client.DEFAULT_VER)
|
||||
|
||||
def test_client_session_via_kwargs(self, http_client_mock):
|
||||
session = mock.Mock()
|
||||
client.Client(session=session, endpoint_override="http://example.com")
|
||||
http_client_mock.assert_called_once_with(
|
||||
session, api_version_select_state='default',
|
||||
endpoint_override="http://example.com",
|
||||
os_ironic_api_version=client.DEFAULT_VER)
|
||||
|
|
|
@ -44,6 +44,11 @@ class Client(object):
|
|||
|
||||
def __init__(self, endpoint_override=None, *args, **kwargs):
|
||||
"""Initialize a new client for the Ironic v1 API."""
|
||||
if not args and not kwargs.get('session'):
|
||||
raise TypeError("A session is required for creating a client, "
|
||||
"use ironicclient.client.get_client to create "
|
||||
"it automatically")
|
||||
|
||||
allow_downgrade = kwargs.pop('allow_api_version_downgrade', False)
|
||||
if kwargs.get('os_ironic_api_version'):
|
||||
# TODO(TheJulia): We should sanity check os_ironic_api_version
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fails with a clear TypeError when a session is not provided to
|
||||
``client.Client`` or ``v1.client.Client``. Before we used to throw::
|
||||
|
||||
_construct_http_client() takes at least 1 argument
|
Loading…
Reference in New Issue