Fix some LOG invocations and messages

LOG.xxx("Hello %s" % xyz) should be LOG.xxx("Hello %s", xyz).
This allows the logging package to skip creating the
formatted log message if the message is not going to be emitted because
of the current log level.

The change touches error, warning and info LOG messages.

Adjust test suite for the above change since LOG now uses two parameters
instead of one.

Fix grammar, add missing "." to log messages in some places.

Change-Id: I86404c34502f07ed6dbda1c82b13db888c60f074

manila/api/extensions.py

@@ -287,7 +287,7 @@ class ExtensionManager(object):
                 self.load_extension(ext_factory)
             except Exception as exc:
                 LOG.warn(_('Failed to load extension %(ext_factory)s: '
-                           '%(exc)s') %
+                           '%(exc)s'),
                          {"ext_factory": ext_factory, "exc": exc})
 
 

manila/api/middleware/fault.py

@@ -52,7 +52,7 @@ class FaultWrapper(base_wsgi.Middleware):
             status = 500
 
         msg_dict = dict(url=req.url, status=status)
-        LOG.info(_("%(url)s returned with HTTP %(status)d") % msg_dict)
+        LOG.info(_("%(url)s returned with HTTP %(status)d"), msg_dict)
         outer = self.status_to_type(status)
         if headers:
             outer.headers = headers

manila/api/openstack/__init__.py

@@ -107,8 +107,8 @@ class APIRouter(base_wsgi.Router):
 
             if collection not in self.resources:
                 LOG.warning(_('Extension %(ext_name)s: Cannot extend '
-                              'resource %(collection)s: No such resource') %
-                            locals())
+                              'resource %(collection)s: No such resource'),
+                            {'ext_name': ext_name, 'collection': collection})
                 continue
 
             LOG.debug('Extension %(ext_name)s extending resource: '

manila/api/openstack/wsgi.py

@@ -579,9 +579,8 @@ class ResourceExceptionHandler(object):
                 code=ex_value.code, explanation=six.text_type(ex_value)))
         elif isinstance(ex_value, TypeError):
             exc_info = (ex_type, ex_value, ex_traceback)
-            LOG.error(_(
-                'Exception handling resource: %s') %
-                ex_value, exc_info=exc_info)
+            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"), six.text_type(ex_value))

manila/api/v1/share_networks.py

@@ -213,9 +213,9 @@ class ShareNetworkController(wsgi.Controller):
                 msg = _("Quota exceeded for %(s_pid)s, tried to create "
                         "share-network (%(d_consumed)d of %(d_quota)d "
                         "already consumed)")
-                LOG.warn(msg % {'s_pid': context.project_id,
-                                'd_consumed': _consumed('share_networks'),
-                                'd_quota': quotas['share_networks']})
+                LOG.warn(msg, {'s_pid': context.project_id,
+                               'd_consumed': _consumed('share_networks'),
+                               'd_quota': quotas['share_networks']})
                 raise exception.ShareNetworksLimitExceeded(
                     allowed=quotas['share_networks'])
         else:

manila/context.py

@@ -61,7 +61,7 @@ class RequestContext(object):
             because they possibly came in from older rpc messages.
         """
         if kwargs:
-            LOG.warn(_('Arguments dropped when creating context: %s') %
+            LOG.warn(_('Arguments dropped when creating context: %s'),
                      str(kwargs))
 
         self.user_id = user_id

manila/scheduler/filters/capacity_filter.py

@@ -50,8 +50,8 @@ class CapacityFilter(filters.BaseHostFilter):
         if free < volume_size:
             LOG.warning(_("Insufficient free space for volume creation "
                           "(requested / avail): "
-                          "%(requested)s/%(available)s")
-                        % {'requested': volume_size,
-                           'available': free})
+                          "%(requested)s/%(available)s"),
+                        {'requested': volume_size,
+                         'available': free})
 
         return free >= volume_size

manila/scheduler/manager.py

@@ -89,7 +89,7 @@ class SchedulerManager(manager.Manager):
 
     def _set_share_error_state_and_notify(self, method, context, ex,
                                           request_spec):
-        LOG.warning(_("Failed to schedule_%(method)s: %(ex)s") %
+        LOG.warning(_("Failed to schedule_%(method)s: %(ex)s"),
                     {"method": method, "ex": ex})
 
         share_state = {'status': 'error'}

manila/scheduler/scheduler_options.py

@@ -67,7 +67,7 @@ class SchedulerOptions(object):
             return os.path.getmtime(filename)
         except os.error as e:
             LOG.exception(_("Could not stat scheduler options file "
-                            "%(filename)s: '%(e)s'") %
+                            "%(filename)s: '%(e)s'"),
                           {"filename": filename, "e": e})
             raise
 
@@ -77,7 +77,7 @@ class SchedulerOptions(object):
             return jsonutils.load(handle)
         except ValueError as e:
             LOG.exception(_("Could not decode scheduler options: "
-                            "'%(e)s'") % {"e": e})
+                            "'%(e)s'"), {"e": e})
             return {}
 
     def _get_time_now(self):

manila/share/driver.py

@@ -74,7 +74,7 @@ class ExecuteMixin(object):
                 tries += 1
                 if tries >= self.configuration.num_shell_tries:
                     raise
-                LOG.exception(_("Recovering from a failed execute.  "
+                LOG.exception(_("Recovering from a failed execute. "
                                 "Try number %s"), tries)
                 time.sleep(tries ** 2)
 

manila/share/drivers/generic.py

@@ -239,7 +239,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
                     self._ssh_exec(server_details, mount_cmd)
                 else:
                     LOG.warning(_("Mount point '%(path)s' already exists on "
-                                  "server '%(server)s'.") % log_data)
+                                  "server '%(server)s'."), log_data)
             except exception.ProcessExecutionError as e:
                 raise exception.ShareBackendException(msg=six.text_type(e))
         return _mount_device_with_lock()
@@ -262,7 +262,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
                 self._ssh_exec(server_details, unmount_cmd)
             else:
                 LOG.warning(_("Mount point '%(path)s' does not exist on "
-                              "server '%(server)s'.") % log_data)
+                              "server '%(server)s'."), log_data)
         return _unmount_device_with_lock()
 
     def _get_mount_path(self, share):
@@ -729,7 +729,7 @@ class CIFSHelper(NASHelperBase):
                 server, ['sudo', 'net', 'conf', 'delshare', share_name])
         except exception.ProcessExecutionError as e:
             LOG.warning(_("Caught error trying delete share: %(error)s, try"
-                          "ing delete it forcibly.") % {'error': e.stderr})
+                          "ing delete it forcibly."), {'error': e.stderr})
             self._ssh_exec(server, ['sudo', 'smbcontrol', 'all', 'close-share',
                                     share_name])
 

manila/share/drivers/glusterfs.py

@@ -122,7 +122,7 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
         try:
             self._execute(*args, **kw)
         except exception.ProcessExecutionError as exc:
-            LOG.error(_("Error in gluster volume set: %s") % exc.stderr)
+            LOG.error(_("Error in gluster volume set: %s"), exc.stderr)
             raise
 
     def check_for_setup_error(self):
@@ -179,7 +179,7 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
             )
             out, err = self._execute(*args, **kw)
         except exception.ProcessExecutionError as exc:
-            LOG.error(_("Error retrieving volume info: %s") % exc.stderr)
+            LOG.error(_("Error retrieving volume info: %s"), exc.stderr)
             raise
 
         if not out:
@@ -349,7 +349,7 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.ShareDriver):
         try:
             self._execute(*args, **kw)
         except exception.ProcessExecutionError as exc:
-            LOG.error(_("Error in gluster volume set: %s") % exc.stderr)
+            LOG.error(_("Error in gluster volume set: %s"), exc.stderr)
             raise
 
     def allow_access(self, context, share, access, share_server=None):

manila/share/drivers/netapp/cluster_mode.py

@@ -160,7 +160,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
         try:
             licenses = self._client.send_request('license-v2-list-info')
         except naapi.NaApiError as e:
-            LOG.error(_("Could not get licenses list. %s.") % e)
+            LOG.error(_("Could not get licenses list. %s."), e)
         else:
             self._licenses = sorted([
                 l.get_child_content('package').lower()
@@ -171,7 +171,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
                 'licenses': ', '.join(self._licenses),
             }
             LOG.info(_("Available licenses on '%(backend)s' "
-                       "are %(licenses)s.") % log_data)
+                       "are %(licenses)s."), log_data)
         return self._licenses
 
     def _get_valid_share_name(self, share_id):
@@ -253,7 +253,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
                 .get_child_by_name('net-port-info')\
                 .get_child_content('port')
         except AttributeError:
-            msg = _("Data port does not exists for node %s") % node
+            msg = _("Data port does not exist for node %s.") % node
             LOG.error(msg)
             raise exception.NetAppException(msg)
         return port
@@ -479,7 +479,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
             vserver_client.send_request('net-dns-create', args)
         except naapi.NaApiError as e:
             if e.code == '13130':
-                LOG.error(_("Dns exists for vserver"))
+                LOG.error(_("DNS exists for vserver."))
             else:
                 raise exception.NetAppException(
                     _("Failed to configure DNS. %s") % e.message)
@@ -700,7 +700,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
             self._remove_export(share, vserver_client)
             self._deallocate_container(share, vserver_client)
         else:
-            LOG.info(_("Share %s does not exists") % share['id'])
+            LOG.info(_("Share %s does not exist."), share['id'])
 
     def _create_export(self, share, vserver, vserver_client):
         """Creates NAS storage."""
@@ -810,7 +810,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
         Deletes vserver.
         """
         if not self._vserver_exists(vserver_name):
-            LOG.error(_("Vserver %s does not exist.") % vserver_name)
+            LOG.error(_("Vserver %s does not exist."), vserver_name)
             return
         volumes_data = vserver_client.send_request('volume-get-iter')
         volumes_count = int(volumes_data.get_child_content('num-records'))
@@ -821,8 +821,8 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
                     {'name': self.configuration.netapp_root_volume_name})
             except naapi.NaApiError as e:
                 if e.code == '13042':
-                    LOG.error(_("Volume %s is already offline.")
-                              % self.configuration.netapp_root_volume_name)
+                    LOG.error(_("Volume %s is already offline."),
+                              self.configuration.netapp_root_volume_name)
                 else:
                     raise e
             vserver_client.send_request(
@@ -846,7 +846,7 @@ class NetAppClusteredShareDriver(driver.ShareDriver):
                     except naapi.NaApiError as e:
                         if e.code == "15661":
                             LOG.error(_("CIFS server does not exist for"
-                                      " vserver %s") % vserver_name)
+                                        " vserver %s"), vserver_name)
                         else:
                             vserver_client.send_request('cifs-server-delete')
         self._client.send_request('vserver-destroy',
@@ -1135,9 +1135,9 @@ class NetAppClusteredCIFSHelper(NetAppNASHelperBase):
             self._restrict_access(user, share_name)
         except naapi.NaApiError as e:
             if e.code == "22":
-                LOG.error(_("User %s does not exist.") % user)
+                LOG.error(_("User %s does not exist."), user)
             elif e.code == "15661":
-                LOG.error(_("Rule %s does not exist.") % user)
+                LOG.error(_("Rule %s does not exist."), user)
             else:
                 raise e
 

manila/share/drivers/service_instance.py

@@ -255,8 +255,8 @@ class ServiceInstanceManager(object):
             inst = self.compute_api.server_get(self.admin_context,
                                                server['instance_id'])
         except exception.InstanceNotFound:
-            LOG.warning(_("Service instance %s does not exists")
-                        % server['instance_id'])
+            LOG.warning(_("Service instance %s does not exist."),
+                        server['instance_id'])
             return False
         if inst['status'] == 'ACTIVE':
             return self._check_server_availability(server)

manila/share/manager.py

@@ -148,7 +148,7 @@ class ShareManager(manager.SchedulerDependentManager):
                 LOG.info(_("Share server created successfully."))
             else:
                 LOG.info(_("Used already existed share server '%(share_server"
-                           "_id)s'") % {'share_server_id': share_server['id']})
+                           "_id)s'"), {'share_server_id': share_server['id']})
             return share_server, share_ref
 
         return _provide_share_server_for_share()
@@ -187,8 +187,8 @@ class ShareManager(manager.SchedulerDependentManager):
                     context, share_id, {'share_server_id': share_server['id']})
             except exception.ShareServerNotFound:
                 with excutils.save_and_reraise_exception():
-                    LOG.error(_("Share server %s does not exist.")
-                              % parent_share_server_id)
+                    LOG.error(_("Share server %s does not exist."),
+                              parent_share_server_id)
                     self.db.share_update(context, share_id,
                                          {'status': 'error'})
         elif share_network_id:
@@ -216,7 +216,7 @@ class ShareManager(manager.SchedulerDependentManager):
                                  {'export_location': export_location})
         except Exception:
             with excutils.save_and_reraise_exception():
-                LOG.error(_("Share %s failed on creation.") % share_id)
+                LOG.error(_("Share %s failed on creation."), share_id)
                 self.db.share_update(context, share_id, {'status': 'error'})
         else:
             LOG.info(_("Share created successfully."))
@@ -454,8 +454,8 @@ class ShareManager(manager.SchedulerDependentManager):
                                             security_services=sec_services)
             except Exception:
                 with excutils.save_and_reraise_exception():
-                    LOG.error(_("Share server %s failed on deletion.")
-                              % share_server['id'])
+                    LOG.error(_("Share server %s failed on deletion."),
+                              share_server['id'])
                     self.db.share_server_update(
                         context, share_server['id'],
                         {'status': constants.STATUS_ERROR})

manila/share/volume_types.py

@@ -35,7 +35,7 @@ def create(context, name, extra_specs={}):
                                          dict(name=name,
                                               extra_specs=extra_specs))
     except db_exception.DBError as e:
-        LOG.exception(_('DB error: %s') % e)
+        LOG.exception(_('DB error: %s'), e)
         raise exception.VolumeTypeCreateFailed(name=name,
                                                extra_specs=extra_specs)
     return type_ref

manila/tests/integrated/api/client.py

@@ -105,10 +105,10 @@ class TestOpenStackClient(object):
         relative_url = parsed_url.path
         if parsed_url.query:
             relative_url = relative_url + "?" + parsed_url.query
-        LOG.info(_("Doing %(method)s on %(relative_url)s") %
+        LOG.info(_("Doing %(method)s on %(relative_url)s"),
                  {"method": method, "relative_url": relative_url})
         if body:
-            LOG.info(_("Body: %s") % body)
+            LOG.info(_("Body: %s"), body)
 
         conn.request(method, relative_url, body, _headers)
         response = conn.getresponse()

manila/tests/netapp/test_cmode_drv.py

@@ -573,7 +573,7 @@ class NetAppClusteredDrvTestCase(test.TestCase):
 
         self.driver._client.send_request.assert_called_once_with(
             'license-v2-list-info')
-        driver.LOG.info.assert_called_once_with(mock.ANY)
+        driver.LOG.info.assert_called_once_with(mock.ANY, mock.ANY)
         self.assertEqual(response, ['fake_license_1', 'fake_license_2'])
 
     def test_licenses_exception_raise(self):
@@ -585,7 +585,7 @@ class NetAppClusteredDrvTestCase(test.TestCase):
 
         self.driver._client.send_request.assert_called_once_with(
             'license-v2-list-info')
-        driver.LOG.error.assert_called_once_with(mock.ANY)
+        driver.LOG.error.assert_called_once_with(mock.ANY, mock.ANY)
 
 
 class NetAppNFSHelperTestCase(test.TestCase):

manila/tests/test_context.py

@@ -58,8 +58,8 @@ class ContextTestCase(test.TestCase):
     def test_extra_args_to_context_get_logged(self):
         info = {}
 
-        def fake_warn(log_msg):
-            info['log_msg'] = log_msg
+        def fake_warn(log_msg, other_args):
+            info['log_msg'] = log_msg % other_args
 
         self.stubs.Set(context.LOG, 'warn', fake_warn)
 

manila/tests/test_migrations.py

@@ -135,9 +135,9 @@ class ManilaMigrationsCheckers(test_migrations.WalkVersionsMixin):
         except Exception as e:
             LOG.error(_("Failed to migrate to version %(version)s on engine "
                         "%(engine)s. Exception while running the migration: "
-                        "%(exception)s") % {'version': version,
-                                            'engine': self.engine,
-                                            'exception': e})
+                        "%(exception)s"), {'version': version,
+                                           'engine': self.engine,
+                                           'exception': e})
             raise
 
     def test_walk_versions(self):

manila/tests/test_share_generic.py

@@ -218,7 +218,7 @@ class GenericShareDriverTestCase(test.TestCase):
         self._driver._get_mount_path.assert_called_once_with(self.share)
         self._driver._is_device_mounted.assert_called_once_with(
             self.share, server, volume)
-        generic.LOG.warning.assert_called_once_with(mock.ANY)
+        generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
 
     def test_mount_device_exception_raised(self):
         server = {'instance_id': 'fake_server_id'}
@@ -271,7 +271,7 @@ class GenericShareDriverTestCase(test.TestCase):
         self._driver._get_mount_path.assert_called_once_with(self.share)
         self._driver._is_device_mounted.assert_called_once_with(
             self.share, server)
-        generic.LOG.warning.assert_called_once_with(mock.ANY)
+        generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
 
     def test_is_device_mounted_true(self):
         volume = {'mountpoint': 'fake_mount_point', 'id': 'fake_id'}

manila/tests/test_share_glusterfs.py

@@ -169,8 +169,8 @@ class GlusterfsShareDriverTestCase(test.TestCase):
         else:
             self.fail('Expecting exception.ProcessExecutionError')
 
-        glusterfs.LOG.error.assert_called_with("Error in gluster volume set: "
-                                               "testvol")
+        glusterfs.LOG.error.assert_called_with(
+            "Error in gluster volume set: %s", "testvol")
 
     def test_do_mount(self):
         expected_exec = ['true']

manila/wsgi.py

@@ -215,7 +215,7 @@ class Server(object):
                                         backlog=backlog)
         self._server = eventlet.spawn(self._start)
         (self._host, self._port) = self._socket.getsockname()[0:2]
-        LOG.info(_("Started %(name)s on %(_host)s:%(_port)s") % self.__dict__)
+        LOG.info(_("Started %(name)s on %(_host)s:%(_port)s"), self.__dict__)
 
     @property
     def host(self):