openstack/python-openstackclient
Code Issues Proposed changes

Standardize logger usage in volume

Browse Source 
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
This commit is contained in:
Tang Chen 2016-06-07 14:29:41 +08:00 committed by Dean Troyer
parent 769baf329e
commit 0e9862be7a
4 changed files with 42 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
openstackclient/tests/volume/v1/test_volume.py
View File

@ -14,6 +14,7 @@
# #
import copy import copy
import mock
from openstackclient.tests import fakes from openstackclient.tests import fakes
from openstackclient.tests.identity.v2_0 import fakes as identity_fakes from openstackclient.tests.identity.v2_0 import fakes as identity_fakes
@ -656,7 +657,8 @@ class TestVolumeSet(TestVolume):
) )
self.assertIsNone(result) 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 = [ arglist = [
'--size', '100', '--size', '100',
volume_fakes.volume_name, volume_fakes.volume_name,
@ -672,12 +674,13 @@ class TestVolumeSet(TestVolume):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
self.assertEqual("New size must be greater than %s GB" % mock_log_error.assert_called_with("New size must be greater "
volume_fakes.volume_size, "than %s GB",
self.app.log.messages.get('error')) volume_fakes.volume_size)
self.assertIsNone(result) 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' self.volumes_mock.get.return_value.status = 'error'
arglist = [ arglist = [
'--size', '130', '--size', '130',
@ -694,9 +697,9 @@ class TestVolumeSet(TestVolume):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
self.assertEqual("Volume is in %s state, it must be available before " mock_log_error.assert_called_with("Volume is in %s state, it must be "
"size can be extended" % 'error', "available before size can be "
self.app.log.messages.get('error')) "extended", 'error')
self.assertIsNone(result) self.assertIsNone(result)
def test_volume_set_property(self): def test_volume_set_property(self):

11
openstackclient/volume/v1/volume.py
View File

@ -16,6 +16,7 @@
"""Volume v1 Volume action implementations""" """Volume v1 Volume action implementations"""
import argparse import argparse
import logging
from osc_lib.cli import parseractions from osc_lib.cli import parseractions
from osc_lib.command import command from osc_lib.command import command
@ -25,6 +26,9 @@ import six
from openstackclient.i18n import _ from openstackclient.i18n import _
LOG = logging.getLogger(__name__)
class CreateVolume(command.ShowOne): class CreateVolume(command.ShowOne):
"""Create new volume""" """Create new volume"""
@ -343,12 +347,11 @@ class SetVolume(command.Command):
if parsed_args.size: if parsed_args.size:
if volume.status != 'available': if volume.status != 'available':
self.app.log.error(_("Volume is in %s state, it must be " LOG.error(_("Volume is in %s state, it must be available "
"available before size can be extended") % "before size can be extended"), volume.status)
volume.status)
return return
if parsed_args.size <= volume.size: if parsed_args.size <= volume.size:
self.app.log.error(_("New size must be greater than %s GB") % LOG.error(_("New size must be greater than %s GB"),
volume.size) volume.size)
return return
volume_client.volumes.extend(volume.id, parsed_args.size) volume_client.volumes.extend(volume.id, parsed_args.size)

13
openstackclient/volume/v2/volume.py
View File

@ -15,6 +15,7 @@
"""Volume V2 Volume action implementations""" """Volume V2 Volume action implementations"""
import copy import copy
import logging
from osc_lib.cli import parseractions from osc_lib.cli import parseractions
from osc_lib.command import command from osc_lib.command import command
@ -25,6 +26,9 @@ from openstackclient.i18n import _
from openstackclient.identity import common as identity_common from openstackclient.identity import common as identity_common
LOG = logging.getLogger(__name__)
class CreateVolume(command.ShowOne): class CreateVolume(command.ShowOne):
"""Create new volume""" """Create new volume"""
@ -361,12 +365,11 @@ class SetVolume(command.Command):
if parsed_args.size: if parsed_args.size:
if volume.status != 'available': if volume.status != 'available':
self.app.log.error(_("Volume is in %s state, it must be " LOG.error(_("Volume is in %s state, it must be available "
"available before size can be extended") % "before size can be extended"), volume.status)
volume.status)
return return
if parsed_args.size <= volume.size: if parsed_args.size <= volume.size:
self.app.log.error(_("New size must be greater than %s GB") % LOG.error(_("New size must be greater than %s GB"),
volume.size) volume.size)
return return
volume_client.volumes.extend(volume.id, parsed_args.size) volume_client.volumes.extend(volume.id, parsed_args.size)
@ -456,4 +459,4 @@ class UnsetVolume(command.Command):
volume.id, parsed_args.image_property) volume.id, parsed_args.image_property)
if (not parsed_args.image_property and not parsed_args.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"))

23
openstackclient/volume/v2/volume_type.py
View File

@ -14,6 +14,8 @@
"""Volume v2 Type action implementations""" """Volume v2 Type action implementations"""
import logging
from osc_lib.cli import parseractions from osc_lib.cli import parseractions
from osc_lib.command import command from osc_lib.command import command
from osc_lib import exceptions from osc_lib import exceptions
@ -24,6 +26,9 @@ from openstackclient.i18n import _
from openstackclient.identity import common as identity_common from openstackclient.identity import common as identity_common
LOG = logging.getLogger(__name__)
class CreateVolumeType(command.ShowOne): class CreateVolumeType(command.ShowOne):
"""Create new volume type""" """Create new volume type"""
@ -190,16 +195,15 @@ class SetVolumeType(command.Command):
**kwargs **kwargs
) )
except Exception as e: except Exception as e:
self.app.log.error(_("Failed to update volume type name or" LOG.error(_("Failed to update volume type name or"
" description: %s") % str(e)) " description: %s"), e)
result += 1 result += 1
if parsed_args.property: if parsed_args.property:
try: try:
volume_type.set_keys(parsed_args.property) volume_type.set_keys(parsed_args.property)
except Exception as e: except Exception as e:
self.app.log.error(_("Failed to set volume type" LOG.error(_("Failed to set volume type property: %s"), e)
" property: %s") % str(e))
result += 1 result += 1
if parsed_args.project: if parsed_args.project:
@ -213,8 +217,8 @@ class SetVolumeType(command.Command):
volume_client.volume_type_access.add_project_access( volume_client.volume_type_access.add_project_access(
volume_type.id, project_info.id) volume_type.id, project_info.id)
except Exception as e: except Exception as e:
self.app.log.error(_("Failed to set volume type access to" LOG.error(_("Failed to set volume type access to "
" project: %s") % str(e)) "project: %s"), e)
result += 1 result += 1
if result > 0: if result > 0:
@ -284,8 +288,7 @@ class UnsetVolumeType(command.Command):
try: try:
volume_type.unset_keys(parsed_args.property) volume_type.unset_keys(parsed_args.property)
except Exception as e: except Exception as e:
self.app.log.error(_("Failed to unset volume type property: %s" LOG.error(_("Failed to unset volume type property: %s"), e)
) % str(e))
result += 1 result += 1
if parsed_args.project: if parsed_args.project:
@ -299,8 +302,8 @@ class UnsetVolumeType(command.Command):
volume_client.volume_type_access.remove_project_access( volume_client.volume_type_access.remove_project_access(
volume_type.id, project_info.id) volume_type.id, project_info.id)
except Exception as e: except Exception as e:
self.app.log.error(_("Failed to remove volume type access from" LOG.error(_("Failed to remove volume type access from "
" project: %s") % str(e)) "project: %s"), e)
result += 1 result += 1
if result > 0: if result > 0: