openstack
/
python-ironicclient
Code Issues Proposed changes

Remove deprecated endpoint argument 

Removing the endpoint argument to the v1.client.Client
constructor.
Please use the standard keystoneauth argument name
endpoint_override instead.

Change-Id: Ie2b26a667485223029fde05b20c39c5687f9e776
Story: 2006422
Task: 36319
This commit is contained in:
Riccardo Pittau 2019-08-21 10:52:12 +02:00
parent f62758cda5
commit 839d9eff4f
4 changed files with 19 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ironicclient/common/http.py
View File

@ -240,9 +240,7 @@ class VersionNegotiationMixin(object):
LOG.debug('Negotiated API version is %s', negotiated_ver)
# Cache the negotiated version for this server
# TODO(vdrok): get rid of self.endpoint attribute in Stein
endpoint_override = (getattr(self, 'endpoint_override', None) or
getattr(self, 'endpoint', None))
endpoint_override = getattr(self, 'endpoint_override', None)
host, port = get_server(endpoint_override)
filecache.save_data(host=host, port=port, data=negotiated_ver)
@ -373,19 +371,11 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter):
api_version_select_state,
max_retries,
retry_interval,
endpoint=None,
**kwargs):
self.os_ironic_api_version = os_ironic_api_version
self.api_version_select_state = api_version_select_state
self.conflict_max_retries = max_retries
self.conflict_retry_interval = retry_interval
# TODO(vdrok): remove this conditional in Stein
if endpoint and not kwargs.get('endpoint_override'):
LOG.warning('Passing "endpoint" argument to SessionClient '
'constructor is deprecated, such possibility will be '
'removed in Stein. Please use "endpoint_override" '
'instead.')
self.endpoint = endpoint
if isinstance(kwargs.get('endpoint_override'), six.string_types):
kwargs['endpoint_override'] = _trim_endpoint_api_version(
kwargs['endpoint_override'])

70
ironicclient/tests/unit/common/test_http.py
View File

@ -17,7 +17,6 @@ import time
import mock
from oslo_serialization import jsonutils
import six
from six.moves import http_client
from keystoneauth1 import exceptions as kexc
@ -56,8 +55,8 @@ def _session_client(**kwargs):
interface='publicURL',
service_type='baremetal',
region_name='',
endpoint='http://%s:%s' % (DEFAULT_HOST,
DEFAULT_PORT),
endpoint_override='http://%s:%s' % (
DEFAULT_HOST, DEFAULT_PORT),
**kwargs)
@ -68,7 +67,7 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
self.test_object = http.VersionNegotiationMixin()
self.test_object.os_ironic_api_version = '1.6'
self.test_object.api_version_select_state = 'default'
self.test_object.endpoint = "http://localhost:1234"
self.test_object.endpoint_override = "http://localhost:1234"
self.mock_mcu = mock.MagicMock()
self.test_object._make_connection_url = self.mock_mcu
self.response = utils.FakeResponse(
@ -111,7 +110,7 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
result = self.test_object.negotiate_version(mock_conn, self.response)
self.assertEqual(latest_ver, result)
self.assertEqual(1, mock_pvh.call_count)
host, port = http.get_server(self.test_object.endpoint)
host, port = http.get_server(self.test_object.endpoint_override)
mock_save_data.assert_called_once_with(host=host, port=port,
data=latest_ver)
@ -195,7 +194,7 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
result = self.test_object.negotiate_version(mock_conn, self.response)
self.assertEqual(max_ver, result)
self.assertEqual(1, mock_pvh.call_count)
host, port = http.get_server(self.test_object.endpoint)
host, port = http.get_server(self.test_object.endpoint_override)
mock_save_data.assert_called_once_with(host=host, port=port,
data=max_ver)
@ -343,20 +342,11 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
def test_get_server(self):
host = 'ironic-host'
port = '6385'
endpoint = 'http://%s:%s/ironic/v1/' % (host, port)
self.assertEqual((host, port), http.get_server(endpoint))
endpoint_override = 'http://%s:%s/ironic/v1/' % (host, port)
self.assertEqual((host, port), http.get_server(endpoint_override))
class SessionClientTest(utils.BaseTestCase):
@mock.patch.object(http.LOG, 'warning', autospec=True)
def test_session_client_endpoint_deprecation(self, log_mock):
http.SessionClient(os_ironic_api_version=1,
session=utils.mockSession({}),
api_version_select_state='user', max_retries=5,
retry_interval=5, endpoint='abc')
self.assertIn('deprecated', log_mock.call_args[0][0])
def test_server_exception_empty_body(self):
error_body = _get_error_body()
@ -396,52 +386,10 @@ class SessionClientTest(utils.BaseTestCase):
result = client._parse_version_headers(fake_session.request())
self.assertEqual(expected_result, result)
def _test_endpoint_override(self, endpoint):
fake_session = utils.mockSession({'content-type': 'application/json'},
status_code=http_client.NO_CONTENT)
request_mock = mock.Mock()
fake_session.request = request_mock
request_mock.return_value = utils.mockSessionResponse(
headers={'content-type': 'application/json'},
status_code=http_client.NO_CONTENT)
client = _session_client(session=fake_session,
endpoint_override=endpoint)
client.json_request('DELETE', '/v1/nodes/aa/maintenance')
expected_args_dict = {
'headers': {
'Content-Type': 'application/json',
'Accept': 'application/json',
'X-OpenStack-Ironic-API-Version': '1.6'
},
'auth': None, 'user_agent': 'python-ironicclient',
'endpoint_filter': {
'interface': 'publicURL',
'service_type': 'baremetal',
'region_name': ''
}
}
if isinstance(endpoint, six.string_types):
trimmed = http._trim_endpoint_api_version(endpoint)
expected_args_dict['endpoint_override'] = trimmed
request_mock.assert_called_once_with(
'/v1/nodes/aa/maintenance', 'DELETE', raise_exc=False,
**expected_args_dict
)
def test_endpoint_override(self):
self._test_endpoint_override('http://1.0.0.1:6385')
def test_endpoint_override_with_version(self):
self._test_endpoint_override('http://1.0.0.1:6385/v1')
def test_endpoint_override_not_valid(self):
self._test_endpoint_override(True)
def test_make_simple_request(self):
session = utils.mockSession({})
client = _session_client(session=session,
endpoint_override='http://127.0.0.1')
client = _session_client(session=session)
res = client._make_simple_request(session, 'GET', 'url')
session.request.assert_called_once_with(
@ -451,7 +399,7 @@ class SessionClientTest(utils.BaseTestCase):
'service_type': 'baremetal',
'region_name': ''
},
endpoint_override='http://127.0.0.1',
endpoint_override='http://localhost:1234',
user_agent=http.USER_AGENT)
self.assertEqual(res, session.request.return_value)

11
ironicclient/v1/client.py
View File

@ -38,8 +38,6 @@ LOG = logging.getLogger(__name__)
class Client(object):
"""Client for the Ironic v1 API.
:param string endpoint: A user-supplied endpoint URL for the ironic
service. DEPRECATED, use endpoint_override instead.
:param string endpoint_override: A user-supplied endpoint URL for the
ironic service.
:param function token: Provides token for authentication.
@ -47,17 +45,12 @@ class Client(object):
http requests. (optional)
"""
def __init__(self, endpoint=None, endpoint_override=None, *args, **kwargs):
def __init__(self, endpoint_override=None, *args, **kwargs):
"""Initialize a new client for the Ironic v1 API."""
allow_downgrade = kwargs.pop('allow_api_version_downgrade', False)
if endpoint_override is None and endpoint is not None:
LOG.warning('Passing "endpoint" parameter to Client constructor '
'is deprecated, and it will be removed in Stein '
'release. Please use "endpoint_override" instead.')
endpoint_override = endpoint
if kwargs.get('os_ironic_api_version'):
# TODO(TheJulia): We should sanity check os_ironic_api_version
# against our maximum suported version, so the client fails
# against our maximum supported version, so the client fails
# immediately upon an unsupported version being provided.
# This logic should also likely live in common/http.py
if allow_downgrade:

7
releasenotes/notes/remove-deprecated-endpoint-argument-fc0bd8099067e4ca.yaml Normal file
View File

@ -0,0 +1,7 @@
---
upgrade:
- |
Removes deprecated argument ``endpoint`` from the
v1.client.Client constructor.
Please use the standard keystoneauth argument name
``endpoint_override`` instead.