Make it clear that a Session is required for v1.client.Client

Change-Id: Icfb5f11a59a6b7cc65dcda0b87240c57c120e228
This commit is contained in:
Dmitry Tantsur
2019-09-24 13:31:43 +02:00
parent afce87ae8c
commit 66a9c87c39
4 changed files with 28 additions and 22 deletions

View File

@@ -481,7 +481,7 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter):
return self._http_request(url, method, **kwargs) return self._http_request(url, method, **kwargs)
def _construct_http_client(session=None, def _construct_http_client(session,
token=None, token=None,
auth_ref=None, auth_ref=None,
os_ironic_api_version=DEFAULT_VER, os_ironic_api_version=DEFAULT_VER,

View File

@@ -22,69 +22,67 @@ from ironicclient.v1 import client
@mock.patch.object(http, '_construct_http_client', autospec=True) @mock.patch.object(http, '_construct_http_client', autospec=True)
class ClientTest(utils.BaseTestCase): class ClientTest(utils.BaseTestCase):
session = mock.sentinel.session
def test_client_user_api_version(self, http_client_mock): def test_client_user_api_version(self, http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
os_ironic_api_version = '1.15' os_ironic_api_version = '1.15'
client.Client(endpoint, token=token, client.Client(endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version) os_ironic_api_version=os_ironic_api_version)
http_client_mock.assert_called_once_with( http_client_mock.assert_called_once_with(
endpoint_override=endpoint, token=token, endpoint_override=endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version, os_ironic_api_version=os_ironic_api_version,
api_version_select_state='user') api_version_select_state='user')
def test_client_user_api_version_with_downgrade(self, http_client_mock): def test_client_user_api_version_with_downgrade(self, http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
os_ironic_api_version = '1.15' os_ironic_api_version = '1.15'
client.Client(endpoint, token=token, client.Client(endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version, os_ironic_api_version=os_ironic_api_version,
allow_api_version_downgrade=True) allow_api_version_downgrade=True)
http_client_mock.assert_called_once_with( http_client_mock.assert_called_once_with(
token=token, endpoint_override=endpoint, endpoint_override=endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version, os_ironic_api_version=os_ironic_api_version,
api_version_select_state='default') api_version_select_state='default')
def test_client_user_api_version_latest_with_downgrade(self, def test_client_user_api_version_latest_with_downgrade(self,
http_client_mock): http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
os_ironic_api_version = 'latest' os_ironic_api_version = 'latest'
self.assertRaises(ValueError, client.Client, endpoint, self.assertRaises(ValueError, client.Client, endpoint,
token=token, allow_api_version_downgrade=True, session=self.session,
allow_api_version_downgrade=True,
os_ironic_api_version=os_ironic_api_version) os_ironic_api_version=os_ironic_api_version)
@mock.patch.object(filecache, 'retrieve_data', autospec=True) @mock.patch.object(filecache, 'retrieve_data', autospec=True)
def test_client_cache_api_version(self, cache_mock, http_client_mock): def test_client_cache_api_version(self, cache_mock, http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
os_ironic_api_version = '1.15' os_ironic_api_version = '1.15'
cache_mock.return_value = os_ironic_api_version cache_mock.return_value = os_ironic_api_version
client.Client(endpoint, token=token) client.Client(endpoint, session=self.session)
cache_mock.assert_called_once_with(host='ironic', port='6385') cache_mock.assert_called_once_with(host='ironic', port='6385')
http_client_mock.assert_called_once_with( http_client_mock.assert_called_once_with(
endpoint_override=endpoint, token=token, endpoint_override=endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version, os_ironic_api_version=os_ironic_api_version,
api_version_select_state='cached') api_version_select_state='cached')
@mock.patch.object(filecache, 'retrieve_data', autospec=True) @mock.patch.object(filecache, 'retrieve_data', autospec=True)
def test_client_default_api_version(self, cache_mock, http_client_mock): def test_client_default_api_version(self, cache_mock, http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
cache_mock.return_value = None cache_mock.return_value = None
client.Client(endpoint, token=token) client.Client(endpoint, session=self.session)
cache_mock.assert_called_once_with(host='ironic', port='6385') cache_mock.assert_called_once_with(host='ironic', port='6385')
http_client_mock.assert_called_once_with( http_client_mock.assert_called_once_with(
endpoint_override=endpoint, token=token, endpoint_override=endpoint, session=self.session,
os_ironic_api_version=client.DEFAULT_VER, os_ironic_api_version=client.DEFAULT_VER,
api_version_select_state='default') api_version_select_state='default')
@@ -95,7 +93,7 @@ class ClientTest(utils.BaseTestCase):
insecure=True) insecure=True)
def test_client_initialized_managers(self, http_client_mock): def test_client_initialized_managers(self, http_client_mock):
cl = client.Client('http://ironic:6385', token='safe_token', cl = client.Client('http://ironic:6385', session=self.session,
os_ironic_api_version='1.15') os_ironic_api_version='1.15')
self.assertIsInstance(cl.node, client.node.NodeManager) self.assertIsInstance(cl.node, client.node.NodeManager)
@@ -105,15 +103,14 @@ class ClientTest(utils.BaseTestCase):
def test_negotiate_api_version(self, http_client_mock): def test_negotiate_api_version(self, http_client_mock):
endpoint = 'http://ironic:6385' endpoint = 'http://ironic:6385'
token = 'safe_token'
os_ironic_api_version = 'latest' os_ironic_api_version = 'latest'
cl = client.Client(endpoint, token=token, cl = client.Client(endpoint, session=self.session,
os_ironic_api_version=os_ironic_api_version) os_ironic_api_version=os_ironic_api_version)
cl.negotiate_api_version() cl.negotiate_api_version()
http_client_mock.assert_called_once_with( http_client_mock.assert_called_once_with(
api_version_select_state='user', endpoint_override=endpoint, api_version_select_state='user', endpoint_override=endpoint,
os_ironic_api_version='latest', token=token) os_ironic_api_version='latest', session=self.session)
# TODO(TheJulia): We should verify that negotiate_version # TODO(TheJulia): We should verify that negotiate_version
# is being called in the client and returns a version, # is being called in the client and returns a version,
# although mocking might need to be restrutured to # although mocking might need to be restrutured to

View File

@@ -40,9 +40,8 @@ class Client(object):
:param string endpoint_override: A user-supplied endpoint URL for the :param string endpoint_override: A user-supplied endpoint URL for the
ironic service. ironic service.
:param function token: Provides token for authentication. :param session: A keystoneauth Session object (must be provided as
:param integer timeout: Allows customization of the timeout for client a keyword argument).
http requests. (optional)
""" """
def __init__(self, endpoint_override=None, *args, **kwargs): def __init__(self, endpoint_override=None, *args, **kwargs):

View File

@@ -0,0 +1,10 @@
---
fixes:
- |
Fixes a confusing error message when a session is not provided for
``ironicclient.v1.client.Client``.
- |
With the removal of the ``HTTPClient`` class in the release 3.0.0, it is
now mandatory to pass a session into ``ironicclient.v1.client.Client``.
The helper call ``ironicclient.client.get_client`` can also be used to
construct a session implicitly.