Skip to content

Commit

Permalink
Merge "Remove use of str on exceptions"
Browse files Browse the repository at this point in the history
  • Loading branch information
Jenkins authored and openstack-gerrit committed Aug 29, 2014
2 parents cf916cb + 298b032 commit 143de6e
Show file tree
Hide file tree
Showing 41 changed files with 229 additions and 59 deletions.
1 change: 1 addition & 0 deletions HACKING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Nova Specific Commandments
- [N322] Method's default argument shouldn't be mutable
- [N323] Ensure that the _() function is explicitly imported to ensure proper translations.
- [N324] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s
- [N325] str() cannot be used on an exception. Remove use or use six.text_type()

Creating Unit Tests
-------------------
Expand Down
3 changes: 2 additions & 1 deletion nova/api/openstack/compute/contrib/admin_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import os.path
import traceback

import six
import webob
from webob import exc

Expand Down Expand Up @@ -330,7 +331,7 @@ def _migrate_live(self, req, id, body):
disk_over_commit = strutils.bool_from_string(disk_over_commit,
strict=True)
except ValueError as err:
raise exc.HTTPBadRequest(explanation=str(err))
raise exc.HTTPBadRequest(explanation=six.text_type(err))

instance = common.get_instance(self.compute_api, context, id,
want_objects=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.

import six
import webob

from nova.api.openstack import extensions
Expand Down Expand Up @@ -83,7 +84,7 @@ def delete(self, req, id):
delete_info = jsonutils.loads(delete_metadata['delete_info'])
volume_id = delete_info['volume_id']
except (KeyError, ValueError) as e:
raise webob.exc.HTTPBadRequest(explanation=str(e))
raise webob.exc.HTTPBadRequest(explanation=six.text_type(e))

try:
self.compute_api.volume_snapshot_delete(context, volume_id,
Expand Down
2 changes: 1 addition & 1 deletion nova/api/openstack/compute/contrib/cells.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def sync_instances(self, req, body):
try:
deleted = strutils.bool_from_string(deleted, strict=True)
except ValueError as err:
raise exc.HTTPBadRequest(explanation=str(err))
raise exc.HTTPBadRequest(explanation=six.text_type(err))
if updated_since:
try:
timeutils.parse_isotime(updated_since)
Expand Down
3 changes: 2 additions & 1 deletion nova/api/openstack/compute/contrib/floating_ips_bulk.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import netaddr
from oslo.config import cfg
import six
import webob.exc

from nova.api.openstack import extensions
Expand Down Expand Up @@ -147,7 +148,7 @@ def _address_to_hosts(self, addresses):
else:
return net.iter_hosts()
except netaddr.AddrFormatError as exc:
raise exception.InvalidInput(reason=str(exc))
raise exception.InvalidInput(reason=six.text_type(exc))


class Floating_ips_bulk(extensions.ExtensionDescriptor):
Expand Down
2 changes: 1 addition & 1 deletion nova/api/openstack/compute/contrib/os_networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def add(self, req, body):
" with project %(project)s: %(message)s") %
{"network": network_id or "",
"project": project_id,
"message": getattr(ex, "value", str(ex))})
"message": getattr(ex, "value", ex)})
raise exc.HTTPBadRequest(explanation=msg)

return webob.Response(status_int=202)
Expand Down
5 changes: 3 additions & 2 deletions nova/api/openstack/compute/contrib/os_tenant_networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import netaddr
import netaddr.core as netexc
from oslo.config import cfg
import six
import webob
from webob import exc

Expand Down Expand Up @@ -136,7 +137,7 @@ def _rollback_quota(reservation):
self.network_api.delete(context, id)
except exception.PolicyNotAuthorized as e:
_rollback_quota(reservation)
raise exc.HTTPForbidden(explanation=str(e))
raise exc.HTTPForbidden(explanation=six.text_type(e))
except exception.NetworkInUse as e:
_rollback_quota(reservation)
raise exc.HTTPConflict(explanation=e.format_message())
Expand Down Expand Up @@ -196,7 +197,7 @@ def create(self, req, body):
if CONF.enable_network_quota:
QUOTAS.commit(context, reservation)
except exception.PolicyNotAuthorized as e:
raise exc.HTTPForbidden(explanation=str(e))
raise exc.HTTPForbidden(explanation=six.text_type(e))
except Exception:
if CONF.enable_network_quota:
QUOTAS.rollback(context, reservation)
Expand Down
2 changes: 1 addition & 1 deletion nova/api/openstack/compute/plugins/v3/servers.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def _get_servers(self, req, is_detail):
if not strutils.bool_from_string(all_tenants, True):
del search_opts['all_tenants']
except ValueError as err:
raise exception.InvalidInput(str(err))
raise exception.InvalidInput(six.text_type(err))

if 'all_tenants' in search_opts:
policy.enforce(context, 'compute:get_all_tenants',
Expand Down
2 changes: 1 addition & 1 deletion nova/api/openstack/compute/servers.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ def _get_servers(self, req, is_detail):
if not strutils.bool_from_string(all_tenants, True):
del search_opts['all_tenants']
except ValueError as err:
raise exception.InvalidInput(str(err))
raise exception.InvalidInput(six.text_type(err))

if 'all_tenants' in search_opts:
policy.enforce(context, 'compute:get_all_tenants',
Expand Down
2 changes: 1 addition & 1 deletion nova/api/openstack/xmlutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -993,4 +993,4 @@ def safe_minidom_parse_string(xml_string):
# NOTE(Vijaya Erukala): XML input such as
# <?xml version="1.0" encoding="TF-8"?>
# raises LookupError: unknown encoding: TF-8
raise exception.MalformedRequestBody(reason=str(e))
raise exception.MalformedRequestBody(reason=six.text_type(e))
75 changes: 75 additions & 0 deletions nova/hacking/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.

import ast
import re

import pep8
Expand Down Expand Up @@ -66,6 +67,42 @@
custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")


class BaseASTChecker(ast.NodeVisitor):
"""Provides a simple framework for writing AST-based checks.
Subclasses should implement visit_* methods like any other AST visitor
implementation. When they detect an error for a particular node the
method should call ``self.add_error(offending_node)``. Details about
where in the code the error occurred will be pulled from the node
object.
Subclasses should also provide a class variable named CHECK_DESC to
be used for the human readable error message.
"""

def __init__(self, tree, filename):
"""This object is created automatically by pep8.
:param tree: an AST tree
:param filename: name of the file being analyzed
(ignored by our checks)
"""
self._tree = tree
self._errors = []

def run(self):
"""Called automatically by pep8."""
self.visit(self._tree)
return self._errors

def add_error(self, node, message=None):
"""Add an error caused by a node to the list of errors for pep8."""
message = message or self.CHECK_DESC
error = (node.lineno, node.col_offset, message, self.__class__)
self._errors.append(error)


def import_no_db_in_virt(logical_line, filename):
"""Check for db calls from nova/virt
Expand Down Expand Up @@ -303,6 +340,43 @@ def use_jsonutils(logical_line, filename):
yield (pos, msg % {'fun': f[:-1]})


class CheckForStrExc(BaseASTChecker):
"""Checks for the use of str() on an exception.
This currently only handles the case where str() is used in
the scope of an exception handler. If the exception is passed
into a function, returned from an assertRaises, or used on an
exception created in the same scope, this does not catch it.
"""

CHECK_DESC = ('N325 str() cannot be used on an exception. '
'Remove or use six.text_type()')

def __init__(self, tree, filename):
super(CheckForStrExc, self).__init__(tree, filename)
self.name = []
self.already_checked = []

def visit_TryExcept(self, node):
for handler in node.handlers:
if handler.name:
self.name.append(handler.name.id)
super(CheckForStrExc, self).generic_visit(node)
self.name = self.name[:-1]
else:
super(CheckForStrExc, self).generic_visit(node)

def visit_Call(self, node):
if isinstance(node.func, ast.Name):
if node.func.id == 'str':
if node not in self.already_checked:
self.already_checked.append(node)
if isinstance(node.args[0], ast.Name):
if node.args[0].id in self.name:
self.add_error(node.args[0])
super(CheckForStrExc, self).generic_visit(node)


def factory(register):
register(import_no_db_in_virt)
register(no_db_session_in_public_api)
Expand All @@ -321,3 +395,4 @@ def factory(register):
register(no_mutable_default_args)
register(check_explicit_underscore_import)
register(use_jsonutils)
register(CheckForStrExc)
2 changes: 1 addition & 1 deletion nova/image/glance.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def call(self, context, version, method, *args, **kwargs):
LOG.exception(error_msg)
if attempt == num_attempts:
raise exception.GlanceConnectionFailed(
host=host, port=port, reason=str(e))
host=host, port=port, reason=six.text_type(e))
time.sleep(1)


Expand Down
3 changes: 2 additions & 1 deletion nova/network/floating_ips.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from oslo.config import cfg
from oslo import messaging
import six

from nova import context
from nova.db import base
Expand Down Expand Up @@ -376,7 +377,7 @@ def do_associate():
LOG.warn(_('Failed to disassociated floating '
'address: %s'), floating_address)
pass
if "Cannot find device" in str(e):
if "Cannot find device" in six.text_type(e):
try:
LOG.error(_('Interface %s not found'), interface)
except Exception:
Expand Down
4 changes: 3 additions & 1 deletion nova/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import datetime

from oslo.config import cfg
import six

from nova.compute import flavors
import nova.context
Expand Down Expand Up @@ -97,7 +98,8 @@ def send_api_fault(url, status, exception):
if not CONF.notify_api_faults:
return

payload = {'url': url, 'exception': str(exception), 'status': status}
payload = {'url': url, 'exception': six.text_type(exception),
'status': status}

rpc.get_notifier('api').error(None, 'api.fault', payload)

Expand Down
8 changes: 4 additions & 4 deletions nova/objects/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def coerce(obj, attr, value):
try:
return netaddr.IPAddress(value)
except netaddr.AddrFormatError as e:
raise ValueError(str(e))
raise ValueError(six.text_type(e))

def from_primitive(self, obj, attr, value):
return self.coerce(obj, attr, value)
Expand Down Expand Up @@ -353,7 +353,7 @@ def coerce(obj, attr, value):
try:
return netaddr.IPNetwork(value)
except netaddr.AddrFormatError as e:
raise ValueError(str(e))
raise ValueError(six.text_type(e))


class IPV4Network(IPNetwork):
Expand All @@ -362,7 +362,7 @@ def coerce(obj, attr, value):
try:
return netaddr.IPNetwork(value, version=4)
except netaddr.AddrFormatError as e:
raise ValueError(str(e))
raise ValueError(six.text_type(e))


class IPV6Network(IPNetwork):
Expand All @@ -371,7 +371,7 @@ def coerce(obj, attr, value):
try:
return netaddr.IPNetwork(value, version=6)
except netaddr.AddrFormatError as e:
raise ValueError(str(e))
raise ValueError(six.text_type(e))


class CompoundFieldType(FieldType):
Expand Down
2 changes: 1 addition & 1 deletion nova/objects/instance_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def wrapper(cls, *args, **kwargs):
exc_val = kwargs.get('exc_val')
exc_tb = kwargs.get('exc_tb')
if exc_val is not None:
kwargs['exc_val'] = str(exc_val)
kwargs['exc_val'] = six.text_type(exc_val)
if not isinstance(exc_tb, six.string_types) and exc_tb is not None:
kwargs['exc_tb'] = ''.join(traceback.format_tb(exc_tb))
# NOTE(danms): We wrap a descriptor, so use that protocol
Expand Down
3 changes: 2 additions & 1 deletion nova/pci/pci_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

import jsonschema
from oslo.config import cfg
import six

from nova import exception
from nova.openstack.common import jsonutils
Expand Down Expand Up @@ -124,7 +125,7 @@ def _get_alias_from_config():
except exception.PciInvalidAlias:
raise
except Exception as e:
raise exception.PciInvalidAlias(reason=str(e))
raise exception.PciInvalidAlias(reason=six.text_type(e))

return aliases

Expand Down
6 changes: 3 additions & 3 deletions nova/pci/pci_whitelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
# under the License.

import jsonschema
from oslo.config import cfg
import six

from nova import exception
from nova.openstack.common import jsonutils
from nova.openstack.common import log as logging
from nova.pci import pci_utils

from oslo.config import cfg

pci_opts = [cfg.MultiStrOpt('pci_passthrough_whitelist',
default=[],
help='White list of PCI devices available to VMs. '
Expand Down Expand Up @@ -77,7 +77,7 @@ def _parse_white_list_from_config(self, whitelists):
jsonschema.validate(spec, _WHITELIST_SCHEMA)
specs.extend(spec)
except Exception as e:
raise exception.PciConfigInvalidWhitelist(reason=str(e))
raise exception.PciConfigInvalidWhitelist(reason=six.text_type(e))

return specs

Expand Down
3 changes: 2 additions & 1 deletion nova/tests/api/openstack/compute/test_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

from lxml import etree
import mock
import six
import webob

from nova.api.openstack.compute import limits
Expand Down Expand Up @@ -487,7 +488,7 @@ def test_multiple_rules(self):
'(POST, /bar*, /bar.*, 5, second);'
'(Say, /derp*, /derp.*, 1, day)')
except ValueError as e:
assert False, str(e)
assert False, six.text_type(e)

# Make sure the number of returned limits are correct
self.assertEqual(len(l), 4)
Expand Down
4 changes: 3 additions & 1 deletion nova/tests/compute/test_claims.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import re
import uuid

import six

from nova.compute import claims
from nova import exception
from nova.openstack.common import jsonutils
Expand Down Expand Up @@ -110,7 +112,7 @@ def assertRaisesRegexp(self, re_obj, e, fn, *a, **kw):
fn(*a, **kw)
self.fail("Expected exception not raised")
except e as ee:
self.assertTrue(re.search(re_obj, str(ee)))
self.assertTrue(re.search(re_obj, six.text_type(ee)))

def test_memory_unlimited(self):
self._claim(memory_mb=99999999)
Expand Down

0 comments on commit 143de6e

Please sign in to comment.