From 0e9862be7af9a88c9c0e6a9ef4e11e48a1192727 Mon Sep 17 00:00:00 2001 From: Tang Chen Date: Tue, 7 Jun 2016 14:29:41 +0800 Subject: [PATCH] Standardize logger usage in volume self.app.log is the logger in class OpenStackShell, which should be used to record logs that have nothing to do with any specific command. So, use the file logger instead. This patch also fixes some usage that doesn't follow rules in: http://docs.openstack.org/developer/oslo.i18n/guidelines.html 1. add variables to logger as an argument 2. do not wrap variables with str() Change-Id: I248861a38a4de0412a080046aa7a6f6473c3e082 Implements: blueprint log-usage --- .../tests/volume/v1/test_volume.py | 19 ++++++++------ openstackclient/volume/v1/volume.py | 13 ++++++---- openstackclient/volume/v2/volume.py | 15 ++++++----- openstackclient/volume/v2/volume_type.py | 25 +++++++++++-------- 4 files changed, 42 insertions(+), 30 deletions(-) diff --git a/openstackclient/tests/volume/v1/test_volume.py b/openstackclient/tests/volume/v1/test_volume.py index e4f51bb53..380bc632d 100644 --- a/openstackclient/tests/volume/v1/test_volume.py +++ b/openstackclient/tests/volume/v1/test_volume.py @@ -14,6 +14,7 @@ # import copy +import mock from openstackclient.tests import fakes from openstackclient.tests.identity.v2_0 import fakes as identity_fakes @@ -656,7 +657,8 @@ class TestVolumeSet(TestVolume): ) self.assertIsNone(result) - def test_volume_set_size_smaller(self): + @mock.patch.object(volume.LOG, 'error') + def test_volume_set_size_smaller(self, mock_log_error): arglist = [ '--size', '100', volume_fakes.volume_name, @@ -672,12 +674,13 @@ class TestVolumeSet(TestVolume): result = self.cmd.take_action(parsed_args) - self.assertEqual("New size must be greater than %s GB" % - volume_fakes.volume_size, - self.app.log.messages.get('error')) + mock_log_error.assert_called_with("New size must be greater " + "than %s GB", + volume_fakes.volume_size) self.assertIsNone(result) - def test_volume_set_size_not_available(self): + @mock.patch.object(volume.LOG, 'error') + def test_volume_set_size_not_available(self, mock_log_error): self.volumes_mock.get.return_value.status = 'error' arglist = [ '--size', '130', @@ -694,9 +697,9 @@ class TestVolumeSet(TestVolume): result = self.cmd.take_action(parsed_args) - self.assertEqual("Volume is in %s state, it must be available before " - "size can be extended" % 'error', - self.app.log.messages.get('error')) + mock_log_error.assert_called_with("Volume is in %s state, it must be " + "available before size can be " + "extended", 'error') self.assertIsNone(result) def test_volume_set_property(self): diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py index 7a3bfb971..e11aa1a71 100644 --- a/openstackclient/volume/v1/volume.py +++ b/openstackclient/volume/v1/volume.py @@ -16,6 +16,7 @@ """Volume v1 Volume action implementations""" import argparse +import logging from osc_lib.cli import parseractions from osc_lib.command import command @@ -25,6 +26,9 @@ import six from openstackclient.i18n import _ +LOG = logging.getLogger(__name__) + + class CreateVolume(command.ShowOne): """Create new volume""" @@ -343,13 +347,12 @@ class SetVolume(command.Command): if parsed_args.size: if volume.status != 'available': - self.app.log.error(_("Volume is in %s state, it must be " - "available before size can be extended") % - volume.status) + LOG.error(_("Volume is in %s state, it must be available " + "before size can be extended"), volume.status) return if parsed_args.size <= volume.size: - self.app.log.error(_("New size must be greater than %s GB") % - volume.size) + LOG.error(_("New size must be greater than %s GB"), + volume.size) return volume_client.volumes.extend(volume.id, parsed_args.size) diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index b1a3af2ef..e54395fa0 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -15,6 +15,7 @@ """Volume V2 Volume action implementations""" import copy +import logging from osc_lib.cli import parseractions from osc_lib.command import command @@ -25,6 +26,9 @@ from openstackclient.i18n import _ from openstackclient.identity import common as identity_common +LOG = logging.getLogger(__name__) + + class CreateVolume(command.ShowOne): """Create new volume""" @@ -361,13 +365,12 @@ class SetVolume(command.Command): if parsed_args.size: if volume.status != 'available': - self.app.log.error(_("Volume is in %s state, it must be " - "available before size can be extended") % - volume.status) + LOG.error(_("Volume is in %s state, it must be available " + "before size can be extended"), volume.status) return if parsed_args.size <= volume.size: - self.app.log.error(_("New size must be greater than %s GB") % - volume.size) + LOG.error(_("New size must be greater than %s GB"), + volume.size) return volume_client.volumes.extend(volume.id, parsed_args.size) @@ -456,4 +459,4 @@ class UnsetVolume(command.Command): volume.id, parsed_args.image_property) if (not parsed_args.image_property and not parsed_args.property): - self.app.log.error(_("No changes requested\n")) + LOG.error(_("No changes requested")) diff --git a/openstackclient/volume/v2/volume_type.py b/openstackclient/volume/v2/volume_type.py index dc6b4f9b2..5e9faa1de 100644 --- a/openstackclient/volume/v2/volume_type.py +++ b/openstackclient/volume/v2/volume_type.py @@ -14,6 +14,8 @@ """Volume v2 Type action implementations""" +import logging + from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions @@ -24,6 +26,9 @@ from openstackclient.i18n import _ from openstackclient.identity import common as identity_common +LOG = logging.getLogger(__name__) + + class CreateVolumeType(command.ShowOne): """Create new volume type""" @@ -190,16 +195,15 @@ class SetVolumeType(command.Command): **kwargs ) except Exception as e: - self.app.log.error(_("Failed to update volume type name or" - " description: %s") % str(e)) + LOG.error(_("Failed to update volume type name or" + " description: %s"), e) result += 1 if parsed_args.property: try: volume_type.set_keys(parsed_args.property) except Exception as e: - self.app.log.error(_("Failed to set volume type" - " property: %s") % str(e)) + LOG.error(_("Failed to set volume type property: %s"), e) result += 1 if parsed_args.project: @@ -213,13 +217,13 @@ class SetVolumeType(command.Command): volume_client.volume_type_access.add_project_access( volume_type.id, project_info.id) except Exception as e: - self.app.log.error(_("Failed to set volume type access to" - " project: %s") % str(e)) + LOG.error(_("Failed to set volume type access to " + "project: %s"), e) result += 1 if result > 0: raise exceptions.CommandError(_("Command Failed: One or more of" - " the operations failed")) + " the operations failed")) class ShowVolumeType(command.ShowOne): @@ -284,8 +288,7 @@ class UnsetVolumeType(command.Command): try: volume_type.unset_keys(parsed_args.property) except Exception as e: - self.app.log.error(_("Failed to unset volume type property: %s" - ) % str(e)) + LOG.error(_("Failed to unset volume type property: %s"), e) result += 1 if parsed_args.project: @@ -299,8 +302,8 @@ class UnsetVolumeType(command.Command): volume_client.volume_type_access.remove_project_access( volume_type.id, project_info.id) except Exception as e: - self.app.log.error(_("Failed to remove volume type access from" - " project: %s") % str(e)) + LOG.error(_("Failed to remove volume type access from " + "project: %s"), e) result += 1 if result > 0: