From adfa4c71dfa96b5be2d2d8093849fe05bd1421ef Mon Sep 17 00:00:00 2001 From: poojajadhav Date: Fri, 27 Jan 2017 19:19:17 +0530 Subject: [PATCH] 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 --- cinder/tests/unit/test_exception.py | 28 ++++++++++------- .../drivers/coprhd/helpers/authentication.py | 24 ++++++++------ .../drivers/coprhd/helpers/commoncoprhdapi.py | 17 +++++----- cinder/volume/drivers/coprhd/scaleio.py | 6 ++-- cinder/volume/drivers/datera/datera_iscsi.py | 14 +++++---- .../drivers/dell_emc/sc/storagecenter_api.py | 11 ++++--- .../volume/drivers/dell_emc/scaleio/driver.py | 31 ++++++++++--------- cinder/volume/drivers/dell_emc/vmax/https.py | 2 +- cinder/volume/drivers/dell_emc/xtremio.py | 6 ++-- 9 files changed, 79 insertions(+), 60 deletions(-) diff --git a/cinder/tests/unit/test_exception.py b/cinder/tests/unit/test_exception.py index 5de24729f67..909618a862b 100644 --- a/cinder/tests/unit/test_exception.py +++ b/cinder/tests/unit/test_exception.py @@ -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'}): diff --git a/cinder/volume/drivers/coprhd/helpers/authentication.py b/cinder/volume/drivers/coprhd/helpers/authentication.py index 4383b1b3e76..c0d9f7c1b6f 100644 --- a/cinder/volume/drivers/coprhd/helpers/authentication.py +++ b/cinder/volume/drivers/coprhd/helpers/authentication.py @@ -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") diff --git a/cinder/volume/drivers/coprhd/helpers/commoncoprhdapi.py b/cinder/volume/drivers/coprhd/helpers/commoncoprhdapi.py index 8f61e12a1ae..1a04ef536f7 100644 --- a/cinder/volume/drivers/coprhd/helpers/commoncoprhdapi.py +++ b/cinder/volume/drivers/coprhd/helpers/commoncoprhdapi.py @@ -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 = "" diff --git a/cinder/volume/drivers/coprhd/scaleio.py b/cinder/volume/drivers/coprhd/scaleio.py index ef65598c223..81e01b38bb7 100644 --- a/cinder/volume/drivers/coprhd/scaleio.py +++ b/cinder/volume/drivers/coprhd/scaleio.py @@ -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") diff --git a/cinder/volume/drivers/datera/datera_iscsi.py b/cinder/volume/drivers/datera/datera_iscsi.py index 0b29e375279..3f441526362 100644 --- a/cinder/volume/drivers/datera/datera_iscsi.py +++ b/cinder/volume/drivers/datera/datera_iscsi.py @@ -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) diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_api.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_api.py index bc8c7418121..62bb9c26ecb 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_api.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_api.py @@ -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 diff --git a/cinder/volume/drivers/dell_emc/scaleio/driver.py b/cinder/volume/drivers/dell_emc/scaleio/driver.py index a6789fbe10a..222399b327a 100644 --- a/cinder/volume/drivers/dell_emc/scaleio/driver.py +++ b/cinder/volume/drivers/dell_emc/scaleio/driver.py @@ -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']}) diff --git a/cinder/volume/drivers/dell_emc/vmax/https.py b/cinder/volume/drivers/dell_emc/vmax/https.py index 64ddbffa1b0..bd04d51a693 100644 --- a/cinder/volume/drivers/dell_emc/vmax/https.py +++ b/cinder/volume/drivers/dell_emc/vmax/https.py @@ -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: diff --git a/cinder/volume/drivers/dell_emc/xtremio.py b/cinder/volume/drivers/dell_emc/xtremio.py index 80b9696a203..4e1ee96a431 100644 --- a/cinder/volume/drivers/dell_emc/xtremio.py +++ b/cinder/volume/drivers/dell_emc/xtremio.py @@ -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):