Browse Source

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
tags/4.0.0^0
Dmitry Tantsur 1 month ago
parent
commit
e8914a7ef9
5 changed files with 71 additions and 4 deletions
  1. +19
    -4
      ironicclient/client.py
  2. +18
    -0
      ironicclient/tests/unit/test_client.py
  3. +22
    -0
      ironicclient/tests/unit/v1/test_client.py
  4. +5
    -0
      ironicclient/v1/client.py
  5. +7
    -0
      releasenotes/notes/client-session-09e6ced1fbc6a9b0.yaml

+ 19
- 4
ironicclient/client.py 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)

+ 18
- 0
ironicclient/tests/unit/test_client.py 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")

+ 22
- 0
ironicclient/tests/unit/v1/test_client.py 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)

+ 5
- 0
ironicclient/v1/client.py 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


+ 7
- 0
releasenotes/notes/client-session-09e6ced1fbc6a9b0.yaml 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

Loading…
Cancel
Save