Enable flake8-logging-format extension

The flake8-logging-format extension includes several checks for things
we've had to try to catch in code reviews until now. This enables the
extension and fixes the few cases where things had slipped through code
review.

G200: Logging statements should not include the exception in logged string
is disabled since that triggers a lot more issues, some of which may be
acceptable. That can be left as a follow up exercise if we want to clean
those up and enable all checks.

Change-Id: I1dedc0b31f78f518c2ab5dee5ed7abda1c1d9296
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>

cinder/api/extensions.py

@@ -281,7 +281,7 @@ def load_standard_extensions(ext_mgr, logger, path, package, ext_list=None):
                          (package, relpkg, root, classname))
 
             if ext_list is not None and classname not in ext_list:
-                logger.debug("Skipping extension: %s" % classpath)
+                logger.debug("Skipping extension: %s", classpath)
                 continue
 
             try:

cinder/api/openstack/wsgi.py

@@ -600,13 +600,11 @@ class ResourceExceptionHandler(object):
             raise Fault(exception.ConvertedException(
                 code=ex_value.code, explanation=six.text_type(ex_value)))
         elif isinstance(ex_value, TypeError):
-            exc_info = (ex_type, ex_value, ex_traceback)
+            LOG.exception('Exception handling resource:')
-            LOG.error('Exception handling resource: %s',
-                      ex_value, exc_info=exc_info)
             raise Fault(webob.exc.HTTPBadRequest())
         elif isinstance(ex_value, Fault):
             LOG.info("Fault thrown: %s", ex_value)
-            raise ex_value
+            raise
         elif isinstance(ex_value, webob.exc.HTTPException):
             LOG.info("HTTP exception thrown: %s", ex_value)
             raise Fault(ex_value)

cinder/privsep/hscli.py

@@ -37,9 +37,8 @@ def hsexecute(cmdarg_json):
         (cmd_out, cmd_err) = putils.execute("hscli", cmdarg_json)
     except (putils.UnknownArgumentError, putils.ProcessExecutionError,
             OSError):
-        LOG.error("Exception in running the command for %s",
+        LOG.exception("Exception in running the command for %s",
-                  cmdarg_json,
+                      cmdarg_json)
-                  exc_info=True)
         raise exception.UnableToExecuteHyperScaleCmd(command=cmdarg_json)
 
     return (cmd_out, cmd_err)

cinder/tests/hacking/checks.py

@@ -416,12 +416,6 @@ def check_timeutils_strtime(logical_line):
         yield(0, msg)
 
 
-def no_log_warn(logical_line):
-    msg = "C307: LOG.warn is deprecated, please use LOG.warning!"
-    if "LOG.warn(" in logical_line:
-        yield (0, msg)
-
-
 def dict_constructor_with_list_copy(logical_line):
     msg = ("N336: Must use a dict comprehension instead of a dict constructor "
            "with a sequence of key-value pairs.")
@@ -470,7 +464,6 @@ def factory(register):
     register(check_unicode_usage)
     register(check_no_print_statements)
     register(check_no_log_audit)
-    register(no_log_warn)
     register(dict_constructor_with_list_copy)
     register(no_test_log)
     register(validate_assertTrue)

cinder/volume/drivers/linstordrv.py

@@ -1036,7 +1036,7 @@ class LinstorIscsiDriver(LinstorBaseDriver):
                 db=self.db,
                 executor=self._execute)
 
-        LOG.info('START: LINSTOR DRBD driver {}'.format(self.helper_name))
+        LOG.info('START: LINSTOR DRBD driver %s', self.helper_name)
 
     def get_volume_stats(self, refresh=False):
         data = self._get_volume_stats()

cinder/volume/drivers/stx/client.py

@@ -392,8 +392,8 @@ class STXClient(object):
                 lun = obj.findtext("PROPERTY[@name='lun']")
                 iid = obj.findtext("PROPERTY[@name='identifier']")
                 if iid in ids:
-                    LOG.debug("volume '{}' is already mapped to {} at lun {}".
+                    LOG.debug("volume '%s' is already mapped to %s at lun %s",
-                              format(volume_name, iid, lun))
+                              volume_name, iid, lun)
                     return int(lun)
         except Exception:
             LOG.exception("failed to look up mappings for volume '%s'",

cinder/volume/flows/api/create_volume.py

@@ -799,7 +799,7 @@ class VolumeCastTask(flow_utils.CinderTask):
         exc_info = False
         if all(flow_failures[-1].exc_info):
             exc_info = flow_failures[-1].exc_info
-        LOG.error('Unexpected build error:', exc_info=exc_info)
+        LOG.error('Unexpected build error:', exc_info=exc_info)  # noqa
 
 
 def get_flow(db_api, image_service_api, availability_zones, create_what,

cinder/volume/flows/api/manage_existing.py

@@ -126,7 +126,7 @@ class ManageCastTask(flow_utils.CinderTask):
         exc_info = False
         if all(flow_failures[-1].exc_info):
             exc_info = flow_failures[-1].exc_info
-        LOG.error('Unexpected build error:', exc_info=exc_info)
+        LOG.error('Unexpected build error:', exc_info=exc_info)  # noqa
 
 
 def get_flow(scheduler_rpcapi, db_api, create_what):

lower-constraints.txt

@@ -33,6 +33,7 @@ fasteners==0.14.1
 fixtures==3.0.0
 flake8==2.5.5
 flake8-import-order==0.18.1
+flake8-logging-format==0.6.0
 future==0.16.0
 futurist==1.6.0
 gitdb2==2.0.3

test-requirements.txt

@@ -5,6 +5,7 @@
 # Install bounded pep8/pyflakes first, then let flake8 install
 hacking>=2.0.0 # Apache-2.0
 flake8-import-order # LGPLv3
+flake8-logging-format>=0.6.0 # Apache-2.0
 
 coverage!=4.4,>=4.0 # Apache-2.0
 ddt>=1.2.1 # MIT

tox.ini

@@ -188,7 +188,9 @@ usedevelop = False
 #  tools from getting in our way with regards to this.
 # H101 include name with TODO
 #  reason: no real benefit
-ignore = E251,E402,W503,W504,H101
+# G200 Logging statements should not include the exception
+#  reason: Many existing cases of this that may be legitimate
+ignore = E251,E402,W503,W504,H101,G200
 # H904 Delay string interpolations at logging calls.
 enable-extensions = H106,H203,H904
 exclude = .git,.venv,.tox,dist,tools,doc/ext,*egg,build