From d02a66d6bf1d222db23d1c14f8c0b53564d28cf2 Mon Sep 17 00:00:00 2001 From: ForestLee Date: Mon, 17 Jul 2017 13:39:19 +0800 Subject: [PATCH] Add a hacking rule for string interpolation at logging String interpolation should be delayed to be handled by the logging code, rather than being done at the point of the logging call. See the oslo i18n guideline * https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html#adding-variables-to-log-messages and * https://github.com/openstack-dev/hacking/blob/master/hacking/checks/other.py#L39 Closes-Bug: #1596829 Change-Id: Iba231be2476dcbeeb0edd76d6a921e549d183758 --- keystone/cmd/cli.py | 8 ++++---- keystone/common/fernet_utils.py | 5 ++--- keystone/resource/backends/sql.py | 2 +- keystone/resource/core.py | 2 +- keystone/tests/unit/fakeldap.py | 2 +- keystone/tests/unit/resource/test_core.py | 2 +- tox.ini | 2 +- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index 57a0be33e7..0904851d20 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -504,10 +504,10 @@ class DbSync(BaseApp): LOG.info('The latest installed migration script version is: ' '%(script)d.\nCurrent repository versions:\nExpand: ' '%(expand)d \nMigrate: %(migrate)d\nContract: ' - '%(contract)d' % {'script': migration_script_version, - 'expand': expand_version, - 'migrate': migrate_version, - 'contract': contract_version}) + '%(contract)d', {'script': migration_script_version, + 'expand': expand_version, + 'migrate': migrate_version, + 'contract': contract_version}) return status @staticmethod diff --git a/keystone/common/fernet_utils.py b/keystone/common/fernet_utils.py index 71953d13b5..2db0921eff 100644 --- a/keystone/common/fernet_utils.py +++ b/keystone/common/fernet_utils.py @@ -96,7 +96,7 @@ class FernetUtils(object): LOG.warning( 'Unable to change the ownership of key_repository without ' 'a keystone user ID and keystone group ID both being ' - 'provided: %s' % self.key_repository) + 'provided: %s', self.key_repository) def _create_new_key(self, keystone_user_id, keystone_group_id): """Securely create a new encryption key. @@ -128,8 +128,7 @@ class FernetUtils(object): LOG.warning( 'Unable to change the ownership of the new key without a ' 'keystone user ID and keystone group ID both being provided: ' - '%s' % - self.key_repository) + '%s', self.key_repository) # Determine the file name of the new key key_file = os.path.join(self.key_repository, '0.tmp') create_success = False diff --git a/keystone/resource/backends/sql.py b/keystone/resource/backends/sql.py index ca8c793168..f2da7d1962 100644 --- a/keystone/resource/backends/sql.py +++ b/keystone/resource/backends/sql.py @@ -214,7 +214,7 @@ class Resource(base.ResourceDriverBase): if (project_id not in project_ids_from_bd or project_id == base.NULL_DOMAIN_ID): LOG.warning('Project %s does not exist and was not ' - 'deleted.' % project_id) + 'deleted.', project_id) query.delete(synchronize_session=False) diff --git a/keystone/resource/core.py b/keystone/resource/core.py index 1143371099..322d4ab4a9 100644 --- a/keystone/resource/core.py +++ b/keystone/resource/core.py @@ -1353,7 +1353,7 @@ class DomainConfigManager(manager.Manager): 'value: %(value)s.') if warning_msg: - LOG.warning(warning_msg % { + LOG.warning(warning_msg, { 'domain': domain_id, 'group': each_whitelisted['group'], 'option': each_whitelisted['option'], diff --git a/keystone/tests/unit/fakeldap.py b/keystone/tests/unit/fakeldap.py index 9a87aaf4dc..f0bbb59ee7 100644 --- a/keystone/tests/unit/fakeldap.py +++ b/keystone/tests/unit/fakeldap.py @@ -347,7 +347,7 @@ class FakeLdap(common.LDAPHandler): id_attr_in_modlist = True if not id_attr_in_modlist: - LOG.debug('id_attribute=%(attr)s missing, attributes=%(attrs)s' % + LOG.debug('id_attribute=%(attr)s missing, attributes=%(attrs)s', {'attr': id_attr, 'attrs': modlist}) raise ldap.NAMING_VIOLATION key = self.key(dn) diff --git a/keystone/tests/unit/resource/test_core.py b/keystone/tests/unit/resource/test_core.py index 8fd5f5125c..18073e82b9 100644 --- a/keystone/tests/unit/resource/test_core.py +++ b/keystone/tests/unit/resource/test_core.py @@ -557,7 +557,7 @@ class DomainConfigTests(object): with mock.patch('keystone.resource.core.LOG', mock_log): res = self.domain_config_api.get_config_with_sensitive_info( self.domain['id']) - mock_log.warning.assert_any_call(mock.ANY) + mock_log.warning.assert_any_call(mock.ANY, mock.ANY) self.assertEqual( invalid_option_config['ldap']['url'], res['ldap']['url']) diff --git a/tox.ini b/tox.ini index ae01575f8b..5389144c28 100644 --- a/tox.ini +++ b/tox.ini @@ -99,7 +99,7 @@ passenv = [flake8] filename= *.py,keystone-manage show-source = true -enable-extensions = H203 +enable-extensions = H203,H904 # D100: Missing docstring in public module # D101: Missing docstring in public class