]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Logging not using oslo.i18n guidelines (openstack)
authorSean McGinnis <sean_mcginnis@dell.com>
Tue, 12 May 2015 13:52:52 +0000 (08:52 -0500)
committerSean McGinnis <sean_mcginnis@dell.com>
Tue, 12 May 2015 13:52:52 +0000 (08:52 -0500)
Multi-patch set for easier chunks. This one addresses
the openstack cinder directory. That directory is synced
from oslo, so no changes made. Translation markers are
being used, so this just removes the hacking check
exclusion of that directory.

Some cleanup of a couple files are also included in this
patch for other directories that had been previously
covered.

There have been quite a few instances found where the
i18n guidelines are not being followed. I believe this
has helped lead to some of the confusion around how to
correctly do this. Other developers see this code and
assume it is an example of the correct usage.

This patch attempts to clean up most of those violations
in the existing codebase to hopefully help avoid some of
that confusion in reviews.

Some issues address:
* Correct log translation markers for different log levels
* Passing format values as arguments to call, not preformatting
* Not forcing translation via six.text_type and others

Guidelines can be found here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html

Hacking checks will not be able to identify all violations of
the guidelines, but it could be useful for catching obvious
one such as LOG.info("No markers!").

Change-Id: If58a29c179e79e0b99e9da1d9a2ff1bc9c7b09e1
Closes-bug: 1433216
Closes-bug: 1431256

cinder/hacking/checks.py
cinder/volume/drivers/eqlx.py
cinder/volume/drivers/netapp/eseries/host_mapper.py

index a860c7ce92c61a60f59481ad45275cbcfa13030c..c0930c17c69ca0c2ececa4d90c80047483418b87 100644 (file)
@@ -211,14 +211,6 @@ def check_assert_called_once(logical_line, filename):
 
 
 def validate_log_translations(logical_line, filename):
-    # TODO(smcginnis): The following is temporary as a series
-    # of patches are done to address these issues. It should be
-    # removed completely when bug 1433216 is closed.
-    ignore_dirs = ["cinder/openstack"]
-    for directory in ignore_dirs:
-        if directory in filename:
-            return
-
     # Translations are not required in the test directory.
     # This will not catch all instances of violations, just direct
     # misuse of the form LOG.info('Message').
index 36c2b1afdcbe712dd66f6e2b6630935364da4ee6..3a5843ea51d1273c3e27d77cacbb61d32d9836cc 100644 (file)
@@ -245,8 +245,8 @@ class DellEQLSanISCSIDriver(san.SanISCSIDriver):
                         return self._ssh_execute(
                             ssh, command,
                             timeout=self.configuration.eqlx_cli_timeout)
-                    except Exception as e:
-                        LOG.exception(e)
+                    except Exception:
+                        LOG.exception(_LE('Error running command.'))
                         greenthread.sleep(random.randint(20, 500) / 100.0)
                 msg = (_("SSH Command failed after '%(total_attempts)r' "
                          "attempts : '%(command)s'") %
index bb66c3c91b6e6bd94ea09b8a1bba9ea60fe25096..b6170c30a03b7142c50a69c5faf68c3d856221c0 100644 (file)
@@ -33,7 +33,7 @@ LOG = logging.getLogger(__name__)
 def map_volume_to_single_host(client, volume, eseries_vol, host,
                               vol_map):
     """Maps the e-series volume to host with initiator."""
-    LOG.debug("Attempting to map volume %s to single host." % volume['id'])
+    LOG.debug("Attempting to map volume %s to single host.", volume['id'])
 
     # If volume is not mapped on the backend, map directly to host
     if not vol_map:
@@ -56,8 +56,8 @@ def map_volume_to_single_host(client, volume, eseries_vol, host,
 
     # Volume is mapped to the multiattach host group
     if vol_map.get('mapRef') == multiattach_cluster_ref:
-        LOG.debug("Volume %s is mapped to multiattach host group."
-                  volume['id'])
+        LOG.debug("Volume %s is mapped to multiattach host group.",
+                  volume['id'])
 
         # If volume is not currently attached according to Cinder, it is
         # safe to delete the mapping
@@ -84,7 +84,7 @@ def map_volume_to_multiple_hosts(client, volume, eseries_vol, target_host,
                                  mapping):
     """Maps the e-series volume to multiattach host group."""
 
-    LOG.debug("Attempting to map volume %s to multiple hosts." % volume['id'])
+    LOG.debug("Attempting to map volume %s to multiple hosts.", volume['id'])
 
     # If volume is already mapped to desired host, return the mapping
     if mapping['mapRef'] == target_host['hostRef']: