diff --git a/ironicclient/client.py b/ironicclient/client.py index dbcd5c5eb..29cf335b6 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -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) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index a42031008..fc43aa71d 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -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") diff --git a/ironicclient/tests/unit/v1/test_client.py b/ironicclient/tests/unit/v1/test_client.py index bc62863a0..e765cf7f6 100644 --- a/ironicclient/tests/unit/v1/test_client.py +++ b/ironicclient/tests/unit/v1/test_client.py @@ -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) diff --git a/ironicclient/v1/client.py b/ironicclient/v1/client.py index cf729d9d9..254185eb7 100644 --- a/ironicclient/v1/client.py +++ b/ironicclient/v1/client.py @@ -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 diff --git a/releasenotes/notes/client-session-09e6ced1fbc6a9b0.yaml b/releasenotes/notes/client-session-09e6ced1fbc6a9b0.yaml new file mode 100644 index 000000000..376d4fc34 --- /dev/null +++ b/releasenotes/notes/client-session-09e6ced1fbc6a9b0.yaml @@ -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