openstack/python-keystoneclient
Code Issues Proposed changes

Proper deprecation for HTTPClient.request methods

Browse Source 
HTTPClient.request and related methods weren't properly
deprecated since they were only mentioned in the docstrings.
Proper deprecation requires use of warnings/debtcollector and
documentation.

Also, fixed places where the deprecated request method was called.

bp deprecations

Change-Id: I0a16933252937ca046793bb6eb2e5cc5da03c9ae
This commit is contained in:
Brant Knudson
2015-07-24 15:52:57 -05:00
parent ada04acf4d
commit 0c2fef51d2
6 changed files with 85 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
keystoneclient/generic/client.py
View File

@@ -84,9 +84,9 @@ class Client(httpclient.HTTPClient):
def _check_keystone_versions(self, url):
"""Calls Keystone URL and detects the available API versions."""
try:
resp, body = self.request(url, "GET",
headers={'Accept':
'application/json'})
resp, body = self._request(url, "GET",
headers={'Accept':
'application/json'})
# Multiple Choices status code is returned by the root
# identity endpoint, with references to one or more
# Identity API versions -- v3 spec
@@ -148,9 +148,9 @@ class Client(httpclient.HTTPClient):
try:
if not url.endswith("/"):
url += '/'
resp, body = self.request("%sextensions" % url, "GET",
headers={'Accept':
'application/json'})
resp, body = self._request("%sextensions" % url, "GET",
headers={'Accept':
'application/json'})
if resp.status_code in (200, 204): # some cases we get No Content
if 'extensions' in body and 'values' in body['extensions']:
# Parse correct format (per contract)

70
keystoneclient/httpclient.py
View File

@@ -696,6 +696,7 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
def serialize(self, entity):
return jsonutils.dumps(entity)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def request(self, *args, **kwargs):
"""Send an http request with the specified characteristics.
@@ -703,10 +704,15 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
setting headers, JSON encoding/decoding, and error handling.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used only by the managers and the managers now receive an
adapter so this function is no longer on the standard request path.
This may be removed in the 2.0.0 release.
"""
return self._request(*args, **kwargs)
def _request(self, *args, **kwargs):
kwargs.setdefault('authenticated', False)
return self._adapter.request(*args, **kwargs)
@@ -715,15 +721,14 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
concatenating self.management_url and url and passing in method and
any associated kwargs.
"""
# NOTE(jamielennox): This is deprecated and is no longer a part of the
# standard client request path. It now goes via the adapter instead.
if not management:
endpoint_filter = kwargs.setdefault('endpoint_filter', {})
endpoint_filter.setdefault('interface', 'public')
kwargs.setdefault('authenticated', None)
return self.request(url, method, **kwargs)
return self._request(url, method, **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def get(self, url, **kwargs):
"""Perform an authenticated GET request.
@@ -731,12 +736,16 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'GET', **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def head(self, url, **kwargs):
"""Perform an authenticated HEAD request.
@@ -744,12 +753,16 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'HEAD', **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def post(self, url, **kwargs):
"""Perform an authenticate POST request.
@@ -757,12 +770,16 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'POST', **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def put(self, url, **kwargs):
"""Perform an authenticate PUT request.
@@ -770,12 +787,16 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'PUT', **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def patch(self, url, **kwargs):
"""Perform an authenticate PATCH request.
@@ -783,12 +804,16 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
an authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'PATCH', **kwargs)
@removals.remove(version='1.7.0', removal_version='2.0.0')
def delete(self, url, **kwargs):
"""Perform an authenticate DELETE request.
@@ -796,9 +821,12 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
an authentication token if one is available.
.. warning::
*DEPRECATED*: This function is no longer used. It was designed to
be used by the managers and the managers now receive an adapter so
this function is no longer on the standard request path.
*DEPRECATED*: This function is no longer used and is deprecated as
of the 1.7.0 release and may be removed in the 2.0.0 release. It
was designed to be used by the managers and the managers now
receive an adapter so this function is no longer on the standard
request path.
"""
return self._cs_request(url, 'DELETE', **kwargs)

12
keystoneclient/tests/unit/test_http.py
View File

@@ -67,7 +67,8 @@ class ClientTest(utils.TestCase):
self.stub_url('GET', text=RESPONSE_BODY)
resp, body = cl.get("/hi")
with self.deprecations.expect_deprecations_here():
resp, body = cl.get("/hi")
self.assertEqual(self.requests_mock.last_request.method, 'GET')
self.assertEqual(self.requests_mock.last_request.url, self.TEST_URL)
@@ -96,7 +97,8 @@ class ClientTest(utils.TestCase):
self.stub_url('GET', status_code=400, json=err_response)
exc_raised = False
try:
cl.get('/hi')
with self.deprecations.expect_deprecations_here():
cl.get('/hi')
except exceptions.BadRequest as exc:
exc_raised = True
self.assertEqual(exc.message, "Error message string")
@@ -106,7 +108,8 @@ class ClientTest(utils.TestCase):
cl = get_authed_client()
self.stub_url('POST')
cl.post("/hi", body=[1, 2, 3])
with self.deprecations.expect_deprecations_here():
cl.post("/hi", body=[1, 2, 3])
self.assertEqual(self.requests_mock.last_request.method, 'POST')
self.assertEqual(self.requests_mock.last_request.body, '[1, 2, 3]')
@@ -123,7 +126,8 @@ class ClientTest(utils.TestCase):
self.stub_url('GET')
cl.request(self.TEST_URL, 'GET')
with self.deprecations.expect_deprecations_here():
cl.request(self.TEST_URL, 'GET')
forwarded = "for=%s;by=%s" % (ORIGINAL_IP, httpclient.USER_AGENT)
self.assertRequestHeaderEqual('Forwarded', forwarded)

9
keystoneclient/tests/unit/test_https.py
View File

@@ -47,7 +47,8 @@ class ClientTest(utils.TestCase):
MOCK_REQUEST.return_value = FAKE_RESPONSE
cl = get_authed_client()
resp, body = cl.get("/hi")
with self.deprecations.expect_deprecations_here():
resp, body = cl.get("/hi")
# this may become too tightly couple later
mock_args, mock_kwargs = MOCK_REQUEST.call_args
@@ -66,7 +67,8 @@ class ClientTest(utils.TestCase):
MOCK_REQUEST.return_value = FAKE_RESPONSE
cl = get_authed_client()
cl.post("/hi", body=[1, 2, 3])
with self.deprecations.expect_deprecations_here():
cl.post("/hi", body=[1, 2, 3])
# this may become too tightly couple later
mock_args, mock_kwargs = MOCK_REQUEST.call_args
@@ -87,7 +89,8 @@ class ClientTest(utils.TestCase):
cert="cert.pem")
cl.management_url = "https://127.0.0.1:5000"
cl.auth_token = "token"
cl.post("/hi", body=[1, 2, 3])
with self.deprecations.expect_deprecations_here():
cl.post("/hi", body=[1, 2, 3])
# this may become too tightly couple later
mock_args, mock_kwargs = MOCK_REQUEST.call_args

12
keystoneclient/tests/unit/v2_0/test_auth.py
View File

@@ -162,7 +162,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
json_body = jsonutils.loads(self.requests_mock.last_request.body)
self.assertEqual(json_body['auth']['token']['id'], fake_token)
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -233,7 +234,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
self.assertEqual(cl.auth_token, self.TEST_TOKEN)
# the token returned from the authentication will be used
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -242,7 +244,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
# then override that token and the new token shall be used
cl.auth_token = fake_token
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -251,7 +254,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
# if we clear that overridden token then we fall back to the original
del cl.auth_token
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')

12
keystoneclient/tests/unit/v3/test_auth.py
View File

@@ -229,7 +229,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
body = jsonutils.loads(self.requests_mock.last_request.body)
self.assertEqual(body['auth']['identity']['token']['id'], fake_token)
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -327,7 +328,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
self.assertEqual(cl.auth_token, self.TEST_TOKEN)
# the token returned from the authentication will be used
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -336,7 +338,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
# then override that token and the new token shall be used
cl.auth_token = fake_token
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')
@@ -345,7 +348,8 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
# if we clear that overridden token then we fall back to the original
del cl.auth_token
resp, body = cl.get(fake_url)
with self.deprecations.expect_deprecations_here():
resp, body = cl.get(fake_url)
self.assertEqual(fake_resp, body)
token = self.requests_mock.last_request.headers.get('X-Auth-Token')