[Bug-Fix]: Remove ability to put incorrect kwargs for an exception

Also, this patch simplify `format_message` method of base
Rally exception class

Change-Id: Ia6ccd68bb7740ea43402e633017684d4df8c07fe
Closes-Bug: #1491388
This commit is contained in:
Andrey Kurilin 2015-09-02 16:02:32 +03:00
parent 7041971c41
commit aa191d6b66
10 changed files with 18 additions and 55 deletions

View File

@ -47,8 +47,9 @@
# (Optional) Enables or disables syslog rfc5424 format for logging. If # (Optional) Enables or disables syslog rfc5424 format for logging. If
# enabled, prefixes the MSG part of the syslog message with APP-NAME # enabled, prefixes the MSG part of the syslog message with APP-NAME
# (RFC5424). The format without the APP-NAME is deprecated in K, and # (RFC5424). The format without the APP-NAME is deprecated in Kilo,
# will be removed in M, along with this option. (boolean value) # and will be removed in Mitaka, along with this option. (boolean
# value)
# This option is deprecated for removal. # This option is deprecated for removal.
# Its value may be silently ignored in the future. # Its value may be silently ignored in the future.
#use_syslog_rfc_format = true #use_syslog_rfc_format = true
@ -98,9 +99,6 @@
# quiet. (boolean value) # quiet. (boolean value)
#rally_debug = false #rally_debug = false
# make exception message format errors fatal (boolean value)
#fatal_exception_format_errors = false
# HTTP timeout for any of OpenStack service in seconds (floating point # HTTP timeout for any of OpenStack service in seconds (floating point
# value) # value)
#openstack_client_http_timeout = 180.0 #openstack_client_http_timeout = 180.0

View File

@ -205,7 +205,7 @@ class Connection(object):
count = (self.model_query(models.Deployment, session=session). count = (self.model_query(models.Deployment, session=session).
filter_by(uuid=uuid).delete(synchronize_session=False)) filter_by(uuid=uuid).delete(synchronize_session=False))
if not count: if not count:
raise exceptions.DeploymentNotFound(uuid=uuid) raise exceptions.DeploymentNotFound(deployment=uuid)
def deployment_get(self, deployment): def deployment_get(self, deployment):
return self._deployment_get(deployment) return self._deployment_get(deployment)

View File

@ -15,7 +15,6 @@
import itertools import itertools
from rally.common import log from rally.common import log
from rally import exceptions
from rally import osclients from rally import osclients
from rally.plugins.openstack.context.cleanup import base as cleanup_base from rally.plugins.openstack.context.cleanup import base as cleanup_base
from rally.plugins.openstack.context.keystone import users from rally.plugins.openstack.context.keystone import users
@ -33,7 +32,6 @@ def list_opts():
return [ return [
("DEFAULT", ("DEFAULT",
itertools.chain(log.DEBUG_OPTS, itertools.chain(log.DEBUG_OPTS,
exceptions.EXC_LOG_OPTS,
osclients.OSCLIENTS_OPTS)), osclients.OSCLIENTS_OPTS)),
("benchmark", ("benchmark",
itertools.chain(cinder_utils.CINDER_BENCHMARK_OPTS, itertools.chain(cinder_utils.CINDER_BENCHMARK_OPTS,

View File

@ -96,12 +96,13 @@ class Engine(plugin.Plugin):
try: try:
engine_cls = Engine.get(name) engine_cls = Engine.get(name)
return engine_cls(deployment) return engine_cls(deployment)
except exceptions.PluginNotFound: except exceptions.PluginNotFound as e:
LOG.error(_("Deployment %(uuid)s: Deploy engine for %(name)s " LOG.error(_("Deployment %(uuid)s: Deploy engine for %(name)s "
"does not exist.") % "does not exist.") %
{"uuid": deployment["uuid"], "name": name}) {"uuid": deployment["uuid"], "name": name})
deployment.update_status(consts.DeployStatus.DEPLOY_FAILED) deployment.update_status(consts.DeployStatus.DEPLOY_FAILED)
raise exceptions.PluginNotFound(name=name) raise exceptions.PluginNotFound(
namespace=e.kwargs.get("namespace"), name=name)
@abc.abstractmethod @abc.abstractmethod
def deploy(self): def deploy(self):

View File

@ -13,10 +13,6 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import sys
from oslo_config import cfg
import six import six
from rally.common.i18n import _ from rally.common.i18n import _
@ -24,15 +20,6 @@ from rally.common import log as logging
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
EXC_LOG_OPTS = [
cfg.BoolOpt("fatal_exception_format_errors",
default=False,
help="make exception message format errors fatal"),
]
CONF = cfg.CONF
CONF.register_opts(EXC_LOG_OPTS)
class RallyException(Exception): class RallyException(Exception):
"""Base Rally Exception """Base Rally Exception
@ -47,37 +34,13 @@ class RallyException(Exception):
def __init__(self, message=None, **kwargs): def __init__(self, message=None, **kwargs):
self.kwargs = kwargs self.kwargs = kwargs
if "code" not in self.kwargs:
try:
self.kwargs["code"] = self.code
except AttributeError:
pass
if "%(message)s" in self.msg_fmt: if "%(message)s" in self.msg_fmt:
kwargs.update({"message": message}) kwargs.update({"message": message})
try: super(RallyException, self).__init__(self.msg_fmt % kwargs)
message = self.msg_fmt % kwargs
except KeyError:
exc_info = sys.exc_info()
# kwargs doesn't match a variable in the message
# log the issue and the kwargs
msg = "kwargs don't match in string format operation: %s"
LOG.debug(msg % kwargs, exc_info=exc_info)
if CONF.fatal_exception_format_errors:
raise exc_info[0].with_traceback(exc_info[2])
else:
# at least get the core message out if something happened
message = message or self.msg_fmt
super(RallyException, self).__init__(message)
def format_message(self): def format_message(self):
if self.__class__.__name__.endswith("_Remote"): return six.text_type(self)
return self.args[0]
else:
return six.text_type(self)
class ImmutableException(RallyException): class ImmutableException(RallyException):

View File

@ -319,7 +319,7 @@ class DeploymentCommandsTestCase(test.TestCase):
def test_deployment_not_found(self, mock_deployment_get): def test_deployment_not_found(self, mock_deployment_get):
deployment_id = "e87e4dca-b515-4477-888d-5f6103f13b42" deployment_id = "e87e4dca-b515-4477-888d-5f6103f13b42"
mock_deployment_get.side_effect = exceptions.DeploymentNotFound( mock_deployment_get.side_effect = exceptions.DeploymentNotFound(
uuid=deployment_id) deployment=deployment_id)
self.assertEqual(1, self.deployment.use(deployment_id)) self.assertEqual(1, self.deployment.use(deployment_id))
@mock.patch("rally.osclients.Clients.verified_keystone") @mock.patch("rally.osclients.Clients.verified_keystone")
@ -354,7 +354,8 @@ class DeploymentCommandsTestCase(test.TestCase):
def test_deployment_check_not_exist(self, mock_deployment_get, def test_deployment_check_not_exist(self, mock_deployment_get,
mock_clients_verified_keystone): mock_clients_verified_keystone):
deployment_id = "e87e4dca-b515-4477-888d-5f6103f13b42" deployment_id = "e87e4dca-b515-4477-888d-5f6103f13b42"
mock_deployment_get.side_effect = exceptions.DeploymentNotFound() mock_deployment_get.side_effect = exceptions.DeploymentNotFound(
deployment=deployment_id)
mock_clients_verified_keystone.services.list.return_value = [] mock_clients_verified_keystone.services.list.return_value = []
self.assertEqual(self.deployment.check(deployment_id), 1) self.assertEqual(self.deployment.check(deployment_id), 1)

View File

@ -46,8 +46,10 @@ class InfoCommandsTestCase(test.TestCase):
mock_scenario_get.assert_called_once_with(query) mock_scenario_get.assert_called_once_with(query)
self.assertIsNone(status) self.assertIsNone(status)
@mock.patch(SCENARIO + ".get", side_effect=exceptions.PluginNotFound) @mock.patch(SCENARIO + ".get")
def test_find_failure_status(self, mock_scenario_get): def test_find_failure_status(self, mock_scenario_get):
mock_scenario_get.side_effect = exceptions.PluginNotFound(
namespace="any", name="any")
query = "Dummy.non_existing" query = "Dummy.non_existing"
status = self.info.find(query) status = self.info.find(query)
mock_scenario_get.assert_called_once_with(query) mock_scenario_get.assert_called_once_with(query)

View File

@ -201,7 +201,7 @@ class CliUtilsTestCase(test.TestCase):
self.assertEqual(ret, 1) self.assertEqual(ret, 1)
@mock.patch("rally.common.db.task_get", @mock.patch("rally.common.db.task_get",
side_effect=exceptions.TaskNotFound(FAKE_TASK_UUID)) side_effect=exceptions.TaskNotFound(uuid=FAKE_TASK_UUID))
def test_run_task_not_found(self, mock_task_get): def test_run_task_not_found(self, mock_task_get):
ret = cliutils.run(["rally", "task", "status", "%s" % FAKE_TASK_UUID], ret = cliutils.run(["rally", "task", "status", "%s" % FAKE_TASK_UUID],
self.categories) self.categories)

View File

@ -141,7 +141,7 @@ class NovaNetworkWrapperTestCase(test.TestCase):
def get_fip(*args, **kwargs): def get_fip(*args, **kwargs):
for i in fip_found: for i in fip_found:
return "fip_id" return "fip_id"
raise exceptions.GetResourceNotFound raise exceptions.GetResourceNotFound(resource="")
wrap._get_floating_ip = mock.Mock(side_effect=get_fip) wrap._get_floating_ip = mock.Mock(side_effect=get_fip)
wrap.delete_floating_ip("fip_id") wrap.delete_floating_ip("fip_id")

View File

@ -28,7 +28,7 @@ class BenchmarkUtilsTestCase(test.TestCase):
def test_wait_for_delete(self): def test_wait_for_delete(self):
def update_resource(self): def update_resource(self):
raise exceptions.GetResourceNotFound() raise exceptions.GetResourceNotFound(resource=None)
resource = mock.MagicMock() resource = mock.MagicMock()
utils.wait_for_delete(resource, update_resource=update_resource) utils.wait_for_delete(resource, update_resource=update_resource)