Skip to content

Commit 0e9862b

Browse files
Tang ChenDean Troyer
authored andcommitted
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
1 parent 769baf3 commit 0e9862b

4 files changed

Lines changed: 42 additions & 30 deletions

File tree

openstackclient/tests/volume/v1/test_volume.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#
1515

1616
import copy
17+
import mock
1718

1819
from openstackclient.tests import fakes
1920
from openstackclient.tests.identity.v2_0 import fakes as identity_fakes
@@ -656,7 +657,8 @@ def test_volume_set_size(self):
656657
)
657658
self.assertIsNone(result)
658659

659-
def test_volume_set_size_smaller(self):
660+
@mock.patch.object(volume.LOG, 'error')
661+
def test_volume_set_size_smaller(self, mock_log_error):
660662
arglist = [
661663
'--size', '100',
662664
volume_fakes.volume_name,
@@ -672,12 +674,13 @@ def test_volume_set_size_smaller(self):
672674

673675
result = self.cmd.take_action(parsed_args)
674676

675-
self.assertEqual("New size must be greater than %s GB" %
676-
volume_fakes.volume_size,
677-
self.app.log.messages.get('error'))
677+
mock_log_error.assert_called_with("New size must be greater "
678+
"than %s GB",
679+
volume_fakes.volume_size)
678680
self.assertIsNone(result)
679681

680-
def test_volume_set_size_not_available(self):
682+
@mock.patch.object(volume.LOG, 'error')
683+
def test_volume_set_size_not_available(self, mock_log_error):
681684
self.volumes_mock.get.return_value.status = 'error'
682685
arglist = [
683686
'--size', '130',
@@ -694,9 +697,9 @@ def test_volume_set_size_not_available(self):
694697

695698
result = self.cmd.take_action(parsed_args)
696699

697-
self.assertEqual("Volume is in %s state, it must be available before "
698-
"size can be extended" % 'error',
699-
self.app.log.messages.get('error'))
700+
mock_log_error.assert_called_with("Volume is in %s state, it must be "
701+
"available before size can be "
702+
"extended", 'error')
700703
self.assertIsNone(result)
701704

702705
def test_volume_set_property(self):

openstackclient/volume/v1/volume.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"""Volume v1 Volume action implementations"""
1717

1818
import argparse
19+
import logging
1920

2021
from osc_lib.cli import parseractions
2122
from osc_lib.command import command
@@ -25,6 +26,9 @@
2526
from openstackclient.i18n import _
2627

2728

29+
LOG = logging.getLogger(__name__)
30+
31+
2832
class CreateVolume(command.ShowOne):
2933
"""Create new volume"""
3034

@@ -343,13 +347,12 @@ def take_action(self, parsed_args):
343347

344348
if parsed_args.size:
345349
if volume.status != 'available':
346-
self.app.log.error(_("Volume is in %s state, it must be "
347-
"available before size can be extended") %
348-
volume.status)
350+
LOG.error(_("Volume is in %s state, it must be available "
351+
"before size can be extended"), volume.status)
349352
return
350353
if parsed_args.size <= volume.size:
351-
self.app.log.error(_("New size must be greater than %s GB") %
352-
volume.size)
354+
LOG.error(_("New size must be greater than %s GB"),
355+
volume.size)
353356
return
354357
volume_client.volumes.extend(volume.id, parsed_args.size)
355358

openstackclient/volume/v2/volume.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"""Volume V2 Volume action implementations"""
1616

1717
import copy
18+
import logging
1819

1920
from osc_lib.cli import parseractions
2021
from osc_lib.command import command
@@ -25,6 +26,9 @@
2526
from openstackclient.identity import common as identity_common
2627

2728

29+
LOG = logging.getLogger(__name__)
30+
31+
2832
class CreateVolume(command.ShowOne):
2933
"""Create new volume"""
3034

@@ -361,13 +365,12 @@ def take_action(self, parsed_args):
361365

362366
if parsed_args.size:
363367
if volume.status != 'available':
364-
self.app.log.error(_("Volume is in %s state, it must be "
365-
"available before size can be extended") %
366-
volume.status)
368+
LOG.error(_("Volume is in %s state, it must be available "
369+
"before size can be extended"), volume.status)
367370
return
368371
if parsed_args.size <= volume.size:
369-
self.app.log.error(_("New size must be greater than %s GB") %
370-
volume.size)
372+
LOG.error(_("New size must be greater than %s GB"),
373+
volume.size)
371374
return
372375
volume_client.volumes.extend(volume.id, parsed_args.size)
373376

@@ -456,4 +459,4 @@ def take_action(self, parsed_args):
456459
volume.id, parsed_args.image_property)
457460

458461
if (not parsed_args.image_property and not parsed_args.property):
459-
self.app.log.error(_("No changes requested\n"))
462+
LOG.error(_("No changes requested"))

openstackclient/volume/v2/volume_type.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
"""Volume v2 Type action implementations"""
1616

17+
import logging
18+
1719
from osc_lib.cli import parseractions
1820
from osc_lib.command import command
1921
from osc_lib import exceptions
@@ -24,6 +26,9 @@
2426
from openstackclient.identity import common as identity_common
2527

2628

29+
LOG = logging.getLogger(__name__)
30+
31+
2732
class CreateVolumeType(command.ShowOne):
2833
"""Create new volume type"""
2934

@@ -190,16 +195,15 @@ def take_action(self, parsed_args):
190195
**kwargs
191196
)
192197
except Exception as e:
193-
self.app.log.error(_("Failed to update volume type name or"
194-
" description: %s") % str(e))
198+
LOG.error(_("Failed to update volume type name or"
199+
" description: %s"), e)
195200
result += 1
196201

197202
if parsed_args.property:
198203
try:
199204
volume_type.set_keys(parsed_args.property)
200205
except Exception as e:
201-
self.app.log.error(_("Failed to set volume type"
202-
" property: %s") % str(e))
206+
LOG.error(_("Failed to set volume type property: %s"), e)
203207
result += 1
204208

205209
if parsed_args.project:
@@ -213,13 +217,13 @@ def take_action(self, parsed_args):
213217
volume_client.volume_type_access.add_project_access(
214218
volume_type.id, project_info.id)
215219
except Exception as e:
216-
self.app.log.error(_("Failed to set volume type access to"
217-
" project: %s") % str(e))
220+
LOG.error(_("Failed to set volume type access to "
221+
"project: %s"), e)
218222
result += 1
219223

220224
if result > 0:
221225
raise exceptions.CommandError(_("Command Failed: One or more of"
222-
" the operations failed"))
226+
" the operations failed"))
223227

224228

225229
class ShowVolumeType(command.ShowOne):
@@ -284,8 +288,7 @@ def take_action(self, parsed_args):
284288
try:
285289
volume_type.unset_keys(parsed_args.property)
286290
except Exception as e:
287-
self.app.log.error(_("Failed to unset volume type property: %s"
288-
) % str(e))
291+
LOG.error(_("Failed to unset volume type property: %s"), e)
289292
result += 1
290293

291294
if parsed_args.project:
@@ -299,8 +302,8 @@ def take_action(self, parsed_args):
299302
volume_client.volume_type_access.remove_project_access(
300303
volume_type.id, project_info.id)
301304
except Exception as e:
302-
self.app.log.error(_("Failed to remove volume type access from"
303-
" project: %s") % str(e))
305+
LOG.error(_("Failed to remove volume type access from "
306+
"project: %s"), e)
304307
result += 1
305308

306309
if result > 0:

0 commit comments

Comments
 (0)