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:
Dmitry Tantsur 2020-02-10 11:02:09 +01:00
parent 8188c01489
commit 1220d76857
5 changed files with 71 additions and 4 deletions

View File

@ -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)

View File

@ -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")

View File

@ -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)

View File

@ -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

View File

@ -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