Consistent way to set details for Error instances
This fixes and cleans up (making it consistent) how error instances set their details value. The base class RESTError will set the details value; all subclasses should call their parent's __init__(). Unit tests were added to test that the Error instances are initialized correctly. Change-Id: I2390fa0012f8e4e6d73cbfb188f1733dfe85e65a Closes-Bug: #1408817
This commit is contained in:
Ruby Loo
parent
b51b303403
commit
166f56da94
@ -22,10 +22,12 @@ class RESTError(Exception, encoding.Serializable):
|
||||
status_code = 500
|
||||
serializable_fields = ('type', 'code', 'message', 'details')
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
def __init__(self, details=None, *args, **kwargs):
|
||||
super(RESTError, self).__init__(*args, **kwargs)
|
||||
self.type = self.__class__.__name__
|
||||
self.code = self.status_code
|
||||
if details:
|
||||
self.details = details
|
||||
|
||||
|
||||
class InvalidContentError(RESTError):
|
||||
@ -37,7 +39,7 @@ class InvalidContentError(RESTError):
|
||||
status_code = 400
|
||||
|
||||
def __init__(self, details):
|
||||
self.details = details
|
||||
super(InvalidContentError, self).__init__(details)
|
||||
|
||||
|
||||
class NotFound(RESTError):
|
||||
@ -56,8 +58,7 @@ class CommandExecutionError(RESTError):
|
||||
message = 'Command execution failed'
|
||||
|
||||
def __init__(self, details):
|
||||
super(CommandExecutionError, self).__init__()
|
||||
self.details = details
|
||||
super(CommandExecutionError, self).__init__(details)
|
||||
|
||||
|
||||
class InvalidCommandError(InvalidContentError):
|
||||
@ -82,7 +83,6 @@ class RequestedObjectNotFoundError(NotFound):
|
||||
def __init__(self, type_descr, obj_id):
|
||||
details = '{0} with id {1} not found.'.format(type_descr, obj_id)
|
||||
super(RequestedObjectNotFoundError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class IronicAPIError(RESTError):
|
||||
@ -92,7 +92,6 @@ class IronicAPIError(RESTError):
|
||||
|
||||
def __init__(self, details):
|
||||
super(IronicAPIError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class HeartbeatError(IronicAPIError):
|
||||
@ -139,8 +138,8 @@ class ImageDownloadError(RESTError):
|
||||
message = 'Error downloading image.'
|
||||
|
||||
def __init__(self, image_id):
|
||||
super(ImageDownloadError, self).__init__()
|
||||
self.details = 'Could not download image with id {0}.'.format(image_id)
|
||||
details = 'Could not download image with id {0}.'.format(image_id)
|
||||
super(ImageDownloadError, self).__init__(details)
|
||||
|
||||
|
||||
class ImageChecksumError(RESTError):
|
||||
@ -149,9 +148,9 @@ class ImageChecksumError(RESTError):
|
||||
message = 'Error verifying image checksum.'
|
||||
|
||||
def __init__(self, image_id):
|
||||
super(ImageChecksumError, self).__init__()
|
||||
self.details = 'Image with id {0} failed to verify against checksum.'
|
||||
self.details = self.details.format(image_id)
|
||||
details = 'Image with id {0} failed to verify against checksum.'
|
||||
details = details.format(image_id)
|
||||
super(ImageChecksumError, self).__init__(details)
|
||||
|
||||
|
||||
class ImageWriteError(RESTError):
|
||||
@ -160,10 +159,10 @@ class ImageWriteError(RESTError):
|
||||
message = 'Error writing image to device.'
|
||||
|
||||
def __init__(self, device, exit_code, stdout, stderr):
|
||||
super(ImageWriteError, self).__init__()
|
||||
self.details = ('Writing image to device {0} failed with exit code '
|
||||
'{1}. stdout: {2}. stderr: {3}')
|
||||
self.details = self.details.format(device, exit_code, stdout, stderr)
|
||||
details = ('Writing image to device {0} failed with exit code '
|
||||
'{1}. stdout: {2}. stderr: {3}')
|
||||
details = details.format(device, exit_code, stdout, stderr)
|
||||
super(ImageWriteError, self).__init__(details)
|
||||
|
||||
|
||||
class ConfigDriveTooLargeError(RESTError):
|
||||
@ -174,7 +173,6 @@ class ConfigDriveTooLargeError(RESTError):
|
||||
details = ('Configdrive at {0} has size {1}, which is larger than '
|
||||
'the intended partition.').format(filename, filesize)
|
||||
super(ConfigDriveTooLargeError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class ConfigDriveWriteError(RESTError):
|
||||
@ -189,7 +187,6 @@ class ConfigDriveWriteError(RESTError):
|
||||
'{1}. stdout: {2}. stderr: {3}.')
|
||||
details = details.format(device, exit_code, stdout, stderr)
|
||||
super(ConfigDriveWriteError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class SystemRebootError(RESTError):
|
||||
@ -198,10 +195,10 @@ class SystemRebootError(RESTError):
|
||||
message = 'Error rebooting system.'
|
||||
|
||||
def __init__(self, exit_code, stdout, stderr):
|
||||
super(SystemRebootError, self).__init__()
|
||||
self.details = ('Reboot script failed with exit code {0}. stdout: '
|
||||
'{1}. stderr: {2}.')
|
||||
self.details = self.details.format(exit_code, stdout, stderr)
|
||||
details = ('Reboot script failed with exit code {0}. stdout: '
|
||||
'{1}. stderr: {2}.')
|
||||
details = details.format(exit_code, stdout, stderr)
|
||||
super(SystemRebootError, self).__init__(details)
|
||||
|
||||
|
||||
class BlockDeviceEraseError(RESTError):
|
||||
@ -211,7 +208,6 @@ class BlockDeviceEraseError(RESTError):
|
||||
|
||||
def __init__(self, details):
|
||||
super(BlockDeviceEraseError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class BlockDeviceError(RESTError):
|
||||
@ -220,7 +216,6 @@ class BlockDeviceError(RESTError):
|
||||
|
||||
def __init__(self, details):
|
||||
super(BlockDeviceError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class VirtualMediaBootError(RESTError):
|
||||
@ -231,7 +226,6 @@ class VirtualMediaBootError(RESTError):
|
||||
|
||||
def __init__(self, details):
|
||||
super(VirtualMediaBootError, self).__init__(details)
|
||||
self.details = details
|
||||
|
||||
|
||||
class ExtensionError(Exception):
|
||||
@ -267,8 +261,8 @@ class HardwareManagerMethodNotFound(RESTError):
|
||||
message = msg + '.'
|
||||
|
||||
def __init__(self, method):
|
||||
self.details = (self.msg + ': "{0}".').format(method)
|
||||
super(HardwareManagerMethodNotFound, self).__init__(self.details)
|
||||
details = (self.msg + ': "{0}".').format(method)
|
||||
super(HardwareManagerMethodNotFound, self).__init__(details)
|
||||
|
||||
|
||||
class IncompatibleHardwareMethodError(RESTError):
|
||||
@ -280,7 +274,7 @@ class IncompatibleHardwareMethodError(RESTError):
|
||||
|
||||
def __init__(self, details=None):
|
||||
if details is not None:
|
||||
self.details = details
|
||||
details = details
|
||||
else:
|
||||
self.details = self.message
|
||||
super(IncompatibleHardwareMethodError, self).__init__(self.details)
|
||||
details = self.message
|
||||
super(IncompatibleHardwareMethodError, self).__init__(details)
|
||||
|
||||
118
ironic_python_agent/tests/errors.py
Normal file
118
ironic_python_agent/tests/errors.py
Normal file
@ -0,0 +1,118 @@
|
||||
# Copyright (C) 2015 Yahoo! Inc. All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from oslotest import base as test_base
|
||||
|
||||
from ironic_python_agent import errors
|
||||
|
||||
DETAILS = 'details'
|
||||
SAME_CL_DETAILS = 'same_as_class_details'
|
||||
DIFF_CL_DETAILS = 'different_from_class_details'
|
||||
SAME_CL_MSG = 'same_as_class_message'
|
||||
SAME_DETAILS = 'same_as_DETAILS'
|
||||
|
||||
|
||||
class TestErrors(test_base.BaseTestCase):
|
||||
|
||||
def test_RESTError(self):
|
||||
e = errors.RESTError()
|
||||
d = e.serialize()
|
||||
|
||||
self.assertEqual("RESTError", e.type)
|
||||
self.assertEqual(errors.RESTError.status_code, e.code)
|
||||
self.assertEqual(errors.RESTError.message, e.message)
|
||||
self.assertEqual(errors.RESTError.details, e.details)
|
||||
|
||||
self.assertEqual("RESTError", d['type'])
|
||||
self.assertEqual(errors.RESTError.status_code, d['code'])
|
||||
self.assertEqual(errors.RESTError.message, d['message'])
|
||||
self.assertEqual(errors.RESTError.details, d['details'])
|
||||
|
||||
def test_RESTError_details(self):
|
||||
e = errors.RESTError(DETAILS)
|
||||
self.assertEqual("RESTError", e.type)
|
||||
self.assertEqual(500, e.code)
|
||||
self.assertEqual(errors.RESTError.message, e.message)
|
||||
self.assertEqual(DETAILS, e.details)
|
||||
|
||||
def _test_class(self, obj, check_details):
|
||||
"""Test that the object is created correctly
|
||||
|
||||
:param obj: object to be tested
|
||||
:param check_details: how to check the object's details value.
|
||||
One of SAME_CL_DETAILS, DIFF_CL_DETAILS,
|
||||
SAME_CL_MSG, SAME_DETAILS
|
||||
|
||||
"""
|
||||
obj_info = "object info = %s" % obj.__dict__
|
||||
cls = obj.__class__
|
||||
d = obj.serialize()
|
||||
|
||||
self.assertEqual(cls.__name__, obj.type, obj_info)
|
||||
self.assertEqual(cls.status_code, obj.code, obj_info)
|
||||
self.assertEqual(cls.message, obj.message, obj_info)
|
||||
|
||||
self.assertEqual(cls.__name__, d['type'], obj_info)
|
||||
self.assertEqual(cls.status_code, d['code'], obj_info)
|
||||
self.assertEqual(cls.message, d['message'], obj_info)
|
||||
self.assertEqual(obj.details, d['details'], obj_info)
|
||||
|
||||
if check_details == SAME_CL_DETAILS:
|
||||
self.assertEqual(cls.details, obj.details, obj_info)
|
||||
elif check_details == DIFF_CL_DETAILS:
|
||||
self.assertNotEqual(cls.details, obj.details, obj_info)
|
||||
elif check_details == SAME_CL_MSG:
|
||||
self.assertEqual(cls.message, obj.details, obj_info)
|
||||
elif check_details == SAME_DETAILS:
|
||||
self.assertEqual(DETAILS, obj.details, obj_info)
|
||||
else:
|
||||
self.fail("unexpected value for check_details: %(chk)s, %(info)s" %
|
||||
{'info': obj_info, 'chk': check_details})
|
||||
|
||||
def test_error_classes(self):
|
||||
cases = [(errors.InvalidContentError(DETAILS), SAME_DETAILS),
|
||||
(errors.NotFound(), SAME_CL_DETAILS),
|
||||
(errors.CommandExecutionError(DETAILS), SAME_DETAILS),
|
||||
(errors.InvalidCommandError(DETAILS), SAME_DETAILS),
|
||||
(errors.InvalidCommandParamsError(DETAILS), SAME_DETAILS),
|
||||
(errors.RequestedObjectNotFoundError('type_descr', 'obj_id'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.IronicAPIError(DETAILS), SAME_DETAILS),
|
||||
(errors.HeartbeatError(DETAILS), SAME_DETAILS),
|
||||
(errors.LookupNodeError(DETAILS), SAME_DETAILS),
|
||||
(errors.LookupAgentIPError(DETAILS), SAME_DETAILS),
|
||||
(errors.LookupAgentInterfaceError(DETAILS), SAME_DETAILS),
|
||||
(errors.ImageDownloadError('image_id'), DIFF_CL_DETAILS),
|
||||
(errors.ImageChecksumError('image_id'), DIFF_CL_DETAILS),
|
||||
(errors.ImageWriteError('device', 'exit_code', 'stdout',
|
||||
'stderr'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.ConfigDriveTooLargeError('filename', 'filesize'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.ConfigDriveWriteError('device', 'exit_code', 'stdout',
|
||||
'stderr'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.SystemRebootError('exit_code', 'stdout', 'stderr'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.BlockDeviceEraseError(DETAILS), SAME_DETAILS),
|
||||
(errors.BlockDeviceError(DETAILS), SAME_DETAILS),
|
||||
(errors.VirtualMediaBootError(DETAILS), SAME_DETAILS),
|
||||
(errors.HardwareManagerMethodNotFound('method'),
|
||||
DIFF_CL_DETAILS),
|
||||
(errors.IncompatibleHardwareMethodError(), SAME_CL_MSG),
|
||||
(errors.IncompatibleHardwareMethodError(DETAILS),
|
||||
SAME_DETAILS),
|
||||
]
|
||||
for (obj, check_details) in cases:
|
||||
self._test_class(obj, check_details)
|
||||
Loading…
x
Reference in New Issue
Block a user