From 601201d120b6e3996f616323186fa16dd9c1378c Mon Sep 17 00:00:00 2001 From: Jim Rollenhagen Date: Wed, 3 Jun 2015 16:56:50 -0700 Subject: [PATCH] Update hacking and fix hacking violations This does a few things: * Update hacking to the version in global-requirements. Old hacking was installing a version of pbr that was breaking other packages. * Fix all the hacking/pep8 rules that updating hacking raised. * Do some general docstring cleanup, while already in there cleaning up a bunch of docstrings due to H405 violations. Change-Id: I1fc1e59d4c3d7b14631f8b576e3f3854bc452188 Closes-Bug: #1461717 --- ironic_python_agent/backoff.py | 6 ++-- ironic_python_agent/encoding.py | 5 +-- ironic_python_agent/errors.py | 38 ++++++++------------- ironic_python_agent/extensions/base.py | 6 ++-- ironic_python_agent/ironic_api_client.py | 5 +-- ironic_python_agent/tests/hardware.py | 6 +++- ironic_python_agent/tests/multi_hardware.py | 20 +++++------ ironic_python_agent/utils.py | 3 +- test-requirements.txt | 2 +- 9 files changed, 44 insertions(+), 47 deletions(-) diff --git a/ironic_python_agent/backoff.py b/ironic_python_agent/backoff.py index 5469c9817..0dcc20645 100644 --- a/ironic_python_agent/backoff.py +++ b/ironic_python_agent/backoff.py @@ -24,7 +24,7 @@ from ironic_python_agent.openstack.common import loopingcall LOG = log.getLogger(__name__) -#TODO(JoshNang) move to oslo, i18n +# TODO(JoshNang) move to oslo, i18n class LoopingCallTimeOut(Exception): """Exception for a timed out LoopingCall. @@ -35,7 +35,9 @@ class LoopingCallTimeOut(Exception): class BackOffLoopingCall(loopingcall.LoopingCallBase): - """The passed in function should return True (no error, return to + """Run a method in a loop with backoff on error. + + The passed in function should return True (no error, return to initial_interval), False (error, start backing off), or raise LoopingCallDone(retvalue=None) (quit looping, return retvalue if set). diff --git a/ironic_python_agent/encoding.py b/ironic_python_agent/encoding.py index bd521d3e4..9022afd58 100644 --- a/ironic_python_agent/encoding.py +++ b/ironic_python_agent/encoding.py @@ -42,8 +42,9 @@ class RESTJSONEncoder(json.JSONEncoder): return super(RESTJSONEncoder, self).encode(o) + delimiter def default(self, o): - """Turn an object into a serializable object. In particular, by - calling :meth:`.Serializable.serialize`. + """Turn an object into a serializable object. + + In particular, by calling :meth:`.Serializable.serialize` on `o`. """ if isinstance(o, Serializable): return o.serialize() diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py index 5ddbf0a18..7d713445a 100644 --- a/ironic_python_agent/errors.py +++ b/ironic_python_agent/errors.py @@ -31,8 +31,9 @@ class RESTError(Exception, encoding.Serializable): class InvalidContentError(RESTError): - """Error which occurs when a user supplies invalid content, either - because that content cannot be parsed according to the advertised + """Error which occurs when a user supplies invalid content. + + Either because that content cannot be parsed according to the advertised `Content-Type`, or due to a content validation error. """ message = 'Invalid request body' @@ -43,10 +44,7 @@ class InvalidContentError(RESTError): class NotFound(RESTError): - """Error which occurs when a user supplies invalid content, either - because that content cannot be parsed according to the advertised - `Content-Type`, or due to a content validation error. - """ + """Error which occurs if a non-existent API endpoint is called.""" message = 'Not found' status_code = 404 details = 'The requested URL was not found.' @@ -104,9 +102,7 @@ class HeartbeatError(IronicAPIError): class LookupNodeError(IronicAPIError): - """Error raised when the node configuration lookup to the Ironic API - fails. - """ + """Error raised when the node lookup to the Ironic API fails.""" message = 'Error getting configuration from Ironic.' @@ -167,6 +163,7 @@ class ImageWriteError(RESTError): class ConfigDriveTooLargeError(RESTError): """Error raised when a configdrive is larger than the partition.""" + message = 'Configdrive is too large for intended partition.' def __init__(self, filename, filesize): @@ -176,9 +173,7 @@ class ConfigDriveTooLargeError(RESTError): class ConfigDriveWriteError(RESTError): - """Error raised when a configdrive directory cannot be written to a - device. - """ + """Error raised when a configdrive cannot be written to a device.""" message = 'Error writing configdrive to device.' @@ -211,7 +206,8 @@ class BlockDeviceEraseError(RESTError): class BlockDeviceError(RESTError): - """Error raised when a block devices causes an unknown error""" + """Error raised when a block devices causes an unknown error.""" + message = 'Block device caused unknown error' def __init__(self, details): @@ -219,10 +215,9 @@ class BlockDeviceError(RESTError): class VirtualMediaBootError(RESTError): - """Error raised when booting ironic-python-client from virtual media - fails. - """ - message = 'Booting ironic-python-client from virtual media failed.' + """Error raised when virtual media device cannot be found for config.""" + + message = 'Configuring agent from virtual media failed.' def __init__(self, details): super(VirtualMediaBootError, self).__init__(details) @@ -270,9 +265,7 @@ class HardwareManagerMethodNotFound(RESTError): class IncompatibleHardwareMethodError(RESTError): - """Error raised when HardwareManager method is incompatible with node - hardware. - """ + """Error raised when HardwareManager method incompatible with hardware.""" message = 'HardwareManager method is not compatible with hardware.' @@ -306,6 +299,7 @@ class CleanVersionMismatch(RESTError): class CleaningError(RESTError): """Error raised when a cleaning step fails.""" + message = 'Clean step failed.' def __init__(self, details=None): @@ -329,9 +323,7 @@ class ISCSIError(RESTError): class DeviceNotFound(NotFound): - """Error raised when the disk or partition to deploy the image onto is - not found. - """ + """Error raised when the device to deploy the image onto is not found.""" message = ('Error finding the disk or partition device to deploy ' 'the image onto.') diff --git a/ironic_python_agent/extensions/base.py b/ironic_python_agent/extensions/base.py index 9d2eb4aca..2d8cbc068 100644 --- a/ironic_python_agent/extensions/base.py +++ b/ironic_python_agent/extensions/base.py @@ -249,6 +249,7 @@ class ExecuteCommandMixin(object): def async_command(command_name, validator=None): """Will run the command in an AsyncCommandResult in its own thread. + command_name is set based on the func name and command_params will be whatever args/kwargs you pass into the decorated command. Return values of type `str` or `unicode` are prefixed with the @@ -277,8 +278,9 @@ def async_command(command_name, validator=None): def sync_command(command_name, validator=None): - """Decorate a method in order to wrap up its return value in a - SyncCommandResult. For consistency with @async_command() can also accept a + """Decorate a method to wrap its return value in a SyncCommandResult. + + For consistency with @async_command() can also accept a validator which will be used to validate input, although a synchronous command can also choose to implement validation inline. """ diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index 97a7ce922..b8de2c006 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -91,8 +91,9 @@ class APIClient(object): return node_content def _do_lookup(self, hardware_info): - """The actual call to lookup a node. Should be called inside - loopingcall.BackOffLoopingCall. + """The actual call to lookup a node. + + Should be called as a `loopingcall.BackOffLoopingCall`. """ path = '/{api_version}/drivers/{driver}/vendor_passthru/lookup'.format( api_version=self.api_version, diff --git a/ironic_python_agent/tests/hardware.py b/ironic_python_agent/tests/hardware.py index 48f391d1a..fd3950c58 100644 --- a/ironic_python_agent/tests/hardware.py +++ b/ironic_python_agent/tests/hardware.py @@ -113,7 +113,11 @@ HDPARM_INFO_TEMPLATE = ( '\t24min for SECURITY ERASE UNIT. 24min for ENHANCED SECURITY ' 'ERASE UNIT.\n' 'Checksum: correct\n' -) +) # noqa +# NOTE(jroll) noqa here is to dodge E131 (indent rules). Since this is a +# massive multi-line string (with specific whitespace formatting), it's easier +# for a human to parse it with indentations on line continuations. The other +# option would be to ignore the 79-character limit here. Ew. BLK_DEVICE_TEMPLATE = ( 'KNAME="sda" MODEL="TinyUSB Drive" SIZE="3116853504" ' diff --git a/ironic_python_agent/tests/multi_hardware.py b/ironic_python_agent/tests/multi_hardware.py index be0dfe0e1..ba5e62597 100644 --- a/ironic_python_agent/tests/multi_hardware.py +++ b/ironic_python_agent/tests/multi_hardware.py @@ -140,18 +140,16 @@ class TestMultipleHardwareManagerLoading(test_base.BaseTestCase): self.assertEqual(1, self.generic_hwm.obj._call_counts['generic_only']) def test_both_succeed(self): - """In the case where both managers will work; only the most specific - manager should have it's function called. - """ + # In the case where both managers will work; only the most specific + # manager should have it's function called. hardware.dispatch_to_managers('both_succeed') self.assertEqual(1, self.mainline_hwm.obj._call_counts['both_succeed']) self.assertEqual(0, self.generic_hwm.obj._call_counts['both_succeed']) def test_mainline_fails(self): - """Ensure that if the mainline manager is unable to run the method - that we properly fall back to generic. - """ + # Ensure that if the mainline manager is unable to run the method + # that we properly fall back to generic. hardware.dispatch_to_managers('mainline_fail') self.assertEqual( @@ -184,9 +182,8 @@ class TestMultipleHardwareManagerLoading(test_base.BaseTestCase): 'FakeMainlineHardwareManager': None}, results) def test_dispatch_to_all_managers_both_succeed(self): - """In the case where both managers will work; only the most specific - manager should have it's function called. - """ + # In the case where both managers will work; only the most specific + # manager should have it's function called. results = hardware.dispatch_to_all_managers('both_succeed') self.assertEqual({'FakeGenericHardwareManager': 'generic_both', @@ -196,9 +193,8 @@ class TestMultipleHardwareManagerLoading(test_base.BaseTestCase): self.assertEqual(1, self.generic_hwm.obj._call_counts['both_succeed']) def test_dispatch_to_all_managers_mainline_fails(self): - """Ensure that if the mainline manager is unable to run the method - that we properly fall back to generic. - """ + # Ensure that if the mainline manager is unable to run the method + # that we properly fall back to generic. hardware.dispatch_to_all_managers('mainline_fail') self.assertEqual( diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index e8ed6e872..4e7367b18 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -105,8 +105,7 @@ def _get_vmedia_device(): def _get_vmedia_params(): - """This method returns the parameters passed to the agent through virtual - media floppy. + """This method returns the parameters passed through virtual media floppy. :returns: a partial dict of potential agent configuration parameters :raises: VirtualMediaBootError when it cannot find the virtual media device diff --git a/test-requirements.txt b/test-requirements.txt index 306baaa6b..6d105922d 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=0.8.0,<0.9 +hacking>=0.10.0,<0.11 coverage>=3.6 discover mock>=1.0