openstack
/
cinder
Code Issues Proposed changes

Extracted HTTP response codes to constants 

There are several places in the source code where HTTP response
codes are used as numeric values.

Status codes 200, 202, 300, 400, 401, 403, 404, 405, 500 and 503 under
tests/unit/volume/drivers are replaced with symbolic constants from
six.moves.http_client thus improves code readability. More patches will
be submitted to address other status codes.

Partial-Bug: #1520159
Change-Id: Id5034fb1cac935ce235d5605a2ae778f44d20fa1
This commit is contained in:
poojajadhav 2017-01-27 19:19:17 +05:30
parent ac989f682b
commit adfa4c71df
9 changed files with 79 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
cinder/tests/unit/test_exception.py
View File

@ -20,6 +20,7 @@ from cinder import test
import mock
import six
from six.moves import http_client
import webob.util
@ -53,7 +54,7 @@ class CinderExceptionTestCase(test.TestCase):
class FakeCinderException(exception.CinderException):
message = "default message: %(code)s"
exc = FakeCinderException(code=500)
exc = FakeCinderException(code=int(http_client.INTERNAL_SERVER_ERROR))
self.assertEqual('default message: 500', six.text_type(exc))
def test_error_msg_exception_with_kwargs(self):
@ -63,23 +64,23 @@ class CinderExceptionTestCase(test.TestCase):
class FakeCinderException(exception.CinderException):
message = "default message: %(misspelled_code)s"
exc = FakeCinderException(code=500)
exc = FakeCinderException(code=http_client.INTERNAL_SERVER_ERROR)
self.assertEqual('default message: %(misspelled_code)s',
six.text_type(exc))
def test_default_error_code(self):
class FakeCinderException(exception.CinderException):
code = 404
code = http_client.NOT_FOUND
exc = FakeCinderException()
self.assertEqual(404, exc.kwargs['code'])
self.assertEqual(http_client.NOT_FOUND, exc.kwargs['code'])
def test_error_code_from_kwarg(self):
class FakeCinderException(exception.CinderException):
code = 500
code = http_client.INTERNAL_SERVER_ERROR
exc = FakeCinderException(code=404)
self.assertEqual(404, exc.kwargs['code'])
exc = FakeCinderException(code=http_client.NOT_FOUND)
self.assertEqual(http_client.NOT_FOUND, exc.kwargs['code'])
def test_error_msg_is_exception_to_string(self):
msg = 'test message'
@ -104,7 +105,8 @@ class CinderExceptionTestCase(test.TestCase):
class FakeCinderException(exception.CinderException):
message = 'Error %(code)d: %(message)s'
exc = FakeCinderException(message='message', code=404)
exc = FakeCinderException(message='message',
code=http_client.NOT_FOUND)
self.assertEqual('Error 404: message', six.text_type(exc))
def test_message_is_exception_in_format_string(self):
@ -121,15 +123,17 @@ class CinderConvertedExceptionTestCase(test.TestCase):
def test_default_args(self):
exc = exception.ConvertedException()
self.assertNotEqual('', exc.title)
self.assertEqual(500, exc.code)
self.assertEqual(http_client.INTERNAL_SERVER_ERROR, exc.code)
self.assertEqual('', exc.explanation)
def test_standard_status_code(self):
with mock.patch.dict(webob.util.status_reasons, {200: 'reason'}):
exc = exception.ConvertedException(code=200)
with mock.patch.dict(webob.util.status_reasons,
{http_client.OK: 'reason'}):
exc = exception.ConvertedException(code=int(http_client.OK))
self.assertEqual('reason', exc.title)
@mock.patch.dict(webob.util.status_reasons, {500: 'reason'})
@mock.patch.dict(webob.util.status_reasons, {
http_client.INTERNAL_SERVER_ERROR: 'reason'})
def test_generic_status_code(self):
with mock.patch.dict(webob.util.status_generic_reasons,
{5: 'generic_reason'}):

24
cinder/volume/drivers/coprhd/helpers/authentication.py
View File

@ -23,6 +23,7 @@ import socket
import requests
from requests import exceptions
import six
from six.moves import http_client
from cinder.i18n import _
from cinder.volume.drivers.coprhd.helpers import commoncoprhdapi as common
@ -82,7 +83,7 @@ class Authentication(common.CoprHDResource):
cookies=cookiejar, allow_redirects=False,
timeout=common.TIMEOUT_SEC)
if (login_response.status_code !=
requests.codes['unauthorized']):
http_client.UNAUTHORIZED):
raise common.CoprHdError(
common.CoprHdError.HTTP_ERR, (_("The"
" authentication"
@ -124,7 +125,7 @@ class Authentication(common.CoprHDResource):
location, headers=new_headers, verify=False,
cookies=cookiejar, allow_redirects=False,
timeout=common.TIMEOUT_SEC)
if login_response.status_code != requests.codes['ok']:
if login_response.status_code != http_client.OK:
raise common.CoprHdError(
common.CoprHdError.HTTP_ERR, (_(
"Login failure code: "
@ -138,7 +139,7 @@ class Authentication(common.CoprHDResource):
cookies=cookiejar, allow_redirects=False)
if(login_response.status_code ==
requests.codes['unauthorized']):
http_client.UNAUTHORIZED):
# Now provide the credentials
login_response = requests.get(
url, headers=self.HEADERS, auth=(username, password),
@ -162,24 +163,27 @@ class Authentication(common.CoprHDResource):
(_("The token is not generated by authentication service."
" %s") % details_str))
if login_response.status_code != requests.codes['ok']:
if login_response.status_code != http_client.OK:
error_msg = None
if login_response.status_code == 401:
if login_response.status_code == http_client.UNAUTHORIZED:
error_msg = _("Access forbidden: Authentication required")
elif login_response.status_code == 403:
elif login_response.status_code == http_client.FORBIDDEN:
error_msg = _("Access forbidden: You don't have"
" sufficient privileges to perform"
" this operation")
elif login_response.status_code == 500:
elif (login_response.status_code ==
http_client.INTERNAL_SERVER_ERROR):
error_msg = _("Bourne internal server error")
elif login_response.status_code == 404:
elif login_response.status_code == http_client.NOT_FOUND:
error_msg = _(
"Requested resource is currently unavailable")
elif login_response.status_code == 405:
elif (login_response.status_code ==
http_client.METHOD_NOT_ALLOWED):
error_msg = (_("GET method is not supported by resource:"
" %s"),
url)
elif login_response.status_code == 503:
elif (login_response.status_code ==
http_client.SERVICE_UNAVAILABLE):
error_msg = _("Service temporarily unavailable:"
" The server is temporarily unable"
" to service your request")

17
cinder/volume/drivers/coprhd/helpers/commoncoprhdapi.py
View File

@ -28,6 +28,7 @@ from oslo_utils import units
import requests
from requests import exceptions
import six
from six.moves import http_client
from cinder import exception
from cinder.i18n import _
@ -137,21 +138,21 @@ def service_json_request(ip_addr, port, http_method, uri, body,
(_("Unknown/Unsupported HTTP method: %s") %
http_method))
if (response.status_code == requests.codes['ok'] or
response.status_code == 202):
if (response.status_code == http_client.OK or
response.status_code == http_client.ACCEPTED):
return (response.text, response.headers)
error_msg = None
if response.status_code == 500:
if response.status_code == http_client.INTERNAL_SERVER_ERROR:
response_text = json_decode(response.text)
error_details = ""
if 'details' in response_text:
error_details = response_text['details']
error_msg = (_("CoprHD internal server error. Error details: %s"),
error_details)
elif response.status_code == 401:
elif response.status_code == http_client.UNAUTHORIZED:
error_msg = _("Access forbidden: Authentication required")
elif response.status_code == 403:
elif response.status_code == http_client.FORBIDDEN:
error_msg = ""
error_details = ""
error_description = ""
@ -177,11 +178,11 @@ def service_json_request(ip_addr, port, http_method, uri, body,
" sufficient privileges to perform this"
" operation")
elif response.status_code == 404:
elif response.status_code == http_client.NOT_FOUND:
error_msg = "Requested resource not found"
elif response.status_code == 405:
elif response.status_code == http_client.METHOD_NOT_ALLOWED:
error_msg = six.text_type(response.text)
elif response.status_code == 503:
elif response.status_code == http_client.SERVICE_UNAVAILABLE:
error_msg = ""
error_details = ""
error_description = ""

6
cinder/volume/drivers/coprhd/scaleio.py
View File

@ -20,6 +20,7 @@ from oslo_config import cfg
from oslo_log import log as logging
import requests
import six
from six.moves import http_client
from six.moves import urllib
from cinder import exception
@ -284,7 +285,7 @@ class EMCCoprHDScaleIODriver(driver.VolumeDriver):
msg = (_("Client with ip %s wasn't found ") % sdc_ip)
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
if r.status_code != 200 and "errorCode" in sdc_id:
if r.status_code != http_client.OK and "errorCode" in sdc_id:
msg = (_("Error getting sdc id from ip %(sdc_ip)s:"
" %(sdc_id_message)s") % {'sdc_ip': sdc_ip,
'sdc_id_message': sdc_id[
@ -297,7 +298,8 @@ class EMCCoprHDScaleIODriver(driver.VolumeDriver):
def _check_response(self, response, request,
server_ip, server_port,
server_username, server_password):
if response.status_code == 401 or response.status_code == 403:
if (response.status_code == http_client.UNAUTHORIZED) or (
response.status_code == http_client.FORBIDDEN):
LOG.info(
"Token is invalid, going to re-login and get a new one")

14
cinder/volume/drivers/datera/datera_iscsi.py
View File

@ -22,6 +22,7 @@ from oslo_config import cfg
from oslo_log import log as logging
import requests
import six
from six.moves import http_client
from cinder import context
from cinder import exception
@ -628,7 +629,7 @@ class DateraDriver(san.SanISCSIDriver, api2.DateraApi, api21.DateraApi):
cert_data,
sensitive=False,
conflict_ok=False):
if (response.status_code == 400 and
if (response.status_code == http_client.BAD_REQUEST and
connection_string.endswith("api_versions")):
# Raise the exception, but don't log any error. We'll just fall
# back to the old style of determining API version. We make this
@ -641,14 +642,15 @@ class DateraDriver(san.SanISCSIDriver, api2.DateraApi, api21.DateraApi):
response.url,
payload,
vars(response))
if response.status_code == 404:
if response.status_code == http_client.NOT_FOUND:
raise exception.NotFound(response.json()['message'])
elif response.status_code in [403, 401]:
elif response.status_code in [http_client.FORBIDDEN,
http_client.UNAUTHORIZED]:
raise exception.NotAuthorized()
elif response.status_code == 409 and conflict_ok:
elif response.status_code == http_client.CONFLICT and conflict_ok:
# Don't raise, because we're expecting a conflict
pass
elif response.status_code == 503:
elif response.status_code == http_client.SERVICE_UNAVAILABLE:
current_retry = 0
while current_retry <= self.retry_attempts:
LOG.debug("Datera 503 response, trying request again")
@ -660,7 +662,7 @@ class DateraDriver(san.SanISCSIDriver, api2.DateraApi, api21.DateraApi):
cert_data)
if resp.ok:
return response.json()
elif resp.status_code != 503:
elif resp.status_code != http_client.SERVICE_UNAVAILABLE:
self._raise_response(resp)
else:
self._raise_response(response)

11
cinder/volume/drivers/dell_emc/sc/storagecenter_api.py
View File

@ -22,6 +22,7 @@ from oslo_utils import excutils
import requests
from simplejson import scanner
import six
from six.moves import http_client
import uuid
from cinder import exception
@ -208,7 +209,7 @@ class HttpClient(object):
# If we made an async call and it was accepted
# we wait for our response.
if async:
if rest_response.status_code == 202:
if rest_response.status_code == http_client.ACCEPTED:
asyncTask = rest_response.json()
return self._wait_for_async_complete(asyncTask)
else:
@ -231,8 +232,9 @@ class HttpClient(object):
headers=self.header,
verify=self.verify)
if rest_response and rest_response.status_code == 400 and (
'Unhandled Exception' in rest_response.text):
if (rest_response and rest_response.status_code == (
http_client.BAD_REQUEST)) and (
'Unhandled Exception' in rest_response.text):
raise exception.DellDriverRetryableException()
return rest_response
@ -467,7 +469,8 @@ class SCApi(object):
:returns: ``True`` if success, ``False`` otherwise.
"""
if rest_response is not None:
if 200 <= rest_response.status_code < 300:
if http_client.OK <= rest_response.status_code < (
http_client.MULTIPLE_CHOICES):
# API call was a normal success
return True

31
cinder/volume/drivers/dell_emc/scaleio/driver.py
View File

@ -29,6 +29,7 @@ from oslo_utils import units
import re
import requests
import six
from six.moves import http_client
from six.moves import urllib
from cinder import context
@ -103,7 +104,6 @@ QOS_IOPS_PER_GB = 'maxIOPSperGB'
QOS_BANDWIDTH_PER_GB = 'maxBWSperGB'
BLOCK_SIZE = 8
OK_STATUS_CODE = 200
VOLUME_NOT_FOUND_ERROR = 79
# This code belongs to older versions of ScaleIO
OLD_VOLUME_NOT_FOUND_ERROR = 78
@ -410,7 +410,7 @@ class ScaleIODriver(driver.VolumeDriver):
% self.protection_domain_name)
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
if r.status_code != OK_STATUS_CODE and "errorCode" in domain_id:
if r.status_code != http_client.OK and "errorCode" in domain_id:
msg = (_("Error getting domain id from name %(name)s: %(id)s.")
% {'name': self.protection_domain_name,
'id': domain_id['message']})
@ -439,7 +439,7 @@ class ScaleIODriver(driver.VolumeDriver):
'domain_id': domain_id})
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
if r.status_code != OK_STATUS_CODE and "errorCode" in pool_id:
if r.status_code != http_client.OK and "errorCode" in pool_id:
msg = (_("Error getting pool id from name %(pool_name)s: "
"%(err_msg)s.")
% {'pool_name': pool_name,
@ -471,7 +471,7 @@ class ScaleIODriver(driver.VolumeDriver):
LOG.info("Add volume response: %s", response)
if r.status_code != OK_STATUS_CODE and "errorCode" in response:
if r.status_code != http_client.OK and "errorCode" in response:
msg = (_("Error creating volume: %s.") % response['message'])
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
@ -511,7 +511,7 @@ class ScaleIODriver(driver.VolumeDriver):
"/api/instances/System/action/snapshotVolumes") % req_vars
r, response = self._execute_scaleio_post_request(params, request)
LOG.info("Snapshot volume response: %s.", response)
if r.status_code != OK_STATUS_CODE and "errorCode" in response:
if r.status_code != http_client.OK and "errorCode" in response:
msg = (_("Failed creating snapshot for volume %(volname)s: "
"%(response)s.") %
{'volname': vol_id,
@ -540,7 +540,8 @@ class ScaleIODriver(driver.VolumeDriver):
def _check_response(self, response, request, is_get_request=True,
params=None):
if response.status_code == 401 or response.status_code == 403:
if (response.status_code == http_client.UNAUTHORIZED or
response.status_code == http_client.FORBIDDEN):
LOG.info("Token is invalid, going to re-login and get "
"a new one.")
login_request = (
@ -580,7 +581,7 @@ class ScaleIODriver(driver.VolumeDriver):
":" + self.server_port + "/api/version")
r, unused = self._execute_scaleio_get_request(request)
if r.status_code == OK_STATUS_CODE:
if r.status_code == http_client.OK:
self.server_api_version = r.text.replace('\"', '')
LOG.info("REST API Version: %(api_version)s",
{'api_version': self.server_api_version})
@ -663,7 +664,7 @@ class ScaleIODriver(driver.VolumeDriver):
params = {'sizeInGB': six.text_type(volume_new_size)}
r, response = self._execute_scaleio_post_request(params, request)
if r.status_code != OK_STATUS_CODE:
if r.status_code != http_client.OK:
response = r.json()
msg = (_("Error extending volume %(vol)s: %(err)s.")
% {'vol': vol_id,
@ -727,7 +728,7 @@ class ScaleIODriver(driver.VolumeDriver):
"/action/removeVolume") % req_vars
r, response = self._execute_scaleio_post_request(params, request)
if r.status_code != OK_STATUS_CODE:
if r.status_code != http_client.OK:
error_code = response['errorCode']
if error_code == VOLUME_NOT_FOUND_ERROR:
LOG.warning("Ignoring error in delete volume %s:"
@ -876,7 +877,7 @@ class ScaleIODriver(driver.VolumeDriver):
% self.protection_domain_name)
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
if r.status_code != OK_STATUS_CODE and "errorCode" in domain_id:
if r.status_code != http_client.OK and "errorCode" in domain_id:
msg = (_("Error getting domain id from name %(name)s: "
"%(err)s.")
% {'name': self.protection_domain_name,
@ -903,7 +904,7 @@ class ScaleIODriver(driver.VolumeDriver):
'domain': domain_id})
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
if r.status_code != OK_STATUS_CODE and "errorCode" in pool_id:
if r.status_code != http_client.OK and "errorCode" in pool_id:
msg = (_("Error getting pool id from name %(pool)s: "
"%(err)s.")
% {'pool': pool_name,
@ -1132,7 +1133,7 @@ class ScaleIODriver(driver.VolumeDriver):
params = {'newName': new_name}
r, response = self._execute_scaleio_post_request(params, request)
if r.status_code != OK_STATUS_CODE:
if r.status_code != http_client.OK:
error_code = response['errorCode']
if ((error_code == VOLUME_NOT_FOUND_ERROR or
error_code == OLD_VOLUME_NOT_FOUND_ERROR or
@ -1247,7 +1248,7 @@ class ScaleIODriver(driver.VolumeDriver):
@staticmethod
def _manage_existing_check_legal_response(response, existing_ref):
if response.status_code != OK_STATUS_CODE:
if response.status_code != http_client.OK:
reason = (_("Error managing volume: %s.") % response.json()[
'message'])
raise exception.ManageExistingInvalidReference(
@ -1339,7 +1340,7 @@ class ScaleIODriver(driver.VolumeDriver):
snapshot_defs = list(map(get_scaleio_snapshot_params, snapshots))
r, response = self._snapshot_volume_group(snapshot_defs)
LOG.info("Snapshot volume response: %s.", response)
if r.status_code != OK_STATUS_CODE and "errorCode" in response:
if r.status_code != http_client.OK and "errorCode" in response:
msg = (_("Failed creating snapshot for group: "
"%(response)s.") %
{'response': response['message']})
@ -1425,7 +1426,7 @@ class ScaleIODriver(driver.VolumeDriver):
volumes)
r, response = self._snapshot_volume_group(list(snapshot_defs))
LOG.info("Snapshot volume response: %s.", response)
if r.status_code != OK_STATUS_CODE and "errorCode" in response:
if r.status_code != http_client.OK and "errorCode" in response:
msg = (_("Failed creating snapshot for group: "
"%(response)s.") %
{'response': response['message']})

2
cinder/volume/drivers/dell_emc/vmax/https.py
View File

@ -326,7 +326,7 @@ def wbem_request(url, data, creds, headers=None, debug=0, x509=None,
body = response.read()
h.close()
if response.status != 200:
if response.status != http_client.OK:
raise pywbem.cim_http.Error('HTTP error')
except http_client.BadStatusLine as arg:

6
cinder/volume/drivers/dell_emc/xtremio.py
View File

@ -41,6 +41,7 @@ from oslo_log import log as logging
from oslo_utils import strutils
from oslo_utils import units
import six
from six.moves import http_client
from cinder import context
from cinder import exception
@ -143,7 +144,8 @@ class XtremIOClient(object):
msg = (_('Exception: %s') % six.text_type(exc))
raise exception.VolumeDriverException(message=msg)
if 200 <= response.status_code < 300:
if (http_client.OK <= response.status_code <
http_client.MULTIPLE_CHOICES):
if method in ('GET', 'POST'):
return response.json()
else:
@ -153,7 +155,7 @@ class XtremIOClient(object):
return _do_req(object_type, method, data, name, idx, ver)
def handle_errors(self, response, key, object_type):
if response.status_code == 400:
if response.status_code == http_client.BAD_REQUEST:
error = response.json()
err_msg = error.get('message')
if err_msg.endswith(OBJ_NOT_FOUND_ERR):