diff --git a/HACKING.rst b/HACKING.rst index 35391f29..dd1191e8 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -8,5 +8,6 @@ Blazar Style Commandments Blazar Specific Commandments ---------------------------- -- [C312] Validate that logs are not translated. +- [Bl301] Validate that logs are not translated. +- [Bl302] Use LOG.warning() rather than LOG.warn(). - [H904] Delay string interpolations at logging calls. diff --git a/blazar/hacking/checks.py b/blazar/hacking/checks.py index 772e042b..5a8cdad3 100644 --- a/blazar/hacking/checks.py +++ b/blazar/hacking/checks.py @@ -31,7 +31,7 @@ _log_translation_hint = re.compile( @core.flake8ext def no_translate_logs(logical_line): - """C312 - Don't translate logs. + """Bl301 - Don't translate logs. Check for 'LOG.*(_(' @@ -46,4 +46,14 @@ def no_translate_logs(logical_line): message describe the check validation failure. """ if _log_translation_hint.match(logical_line): - yield (0, "C312: Log messages should not be translated!") + yield (0, "Bl301: Log messages should not be translated!") + + +@core.flake8ext +def no_log_warn(logical_line): + """Bl302 - Use LOG.warning() rather than LOG.warn() + + https://bugs.launchpad.net/tempest/+bug/1508442 + """ + if logical_line.startswith('LOG.warn('): + yield(0, 'Bl302 Use LOG.warning() rather than LOG.warn()') diff --git a/blazar/plugins/instances/instance_plugin.py b/blazar/plugins/instances/instance_plugin.py index cece130f..8851fa98 100644 --- a/blazar/plugins/instances/instance_plugin.py +++ b/blazar/plugins/instances/instance_plugin.py @@ -714,9 +714,9 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): if new_host_id is None: for allocation in allocations: db_api.host_allocation_destroy(allocation['id']) - LOG.warn('Could not find alternative host for ' - 'reservation %s (lease: %s).', - reservation['id'], lease['name']) + LOG.warning('Could not find alternative host for ' + 'reservation %s (lease: %s).', + reservation['id'], lease['name']) ret = False else: for allocation in allocations: @@ -735,9 +735,9 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): if new_host_id is None: db_api.host_allocation_destroy(allocation['id']) - LOG.warn('Could not find alternative host for ' - 'reservation %s (lease: %s).', - reservation['id'], lease['name']) + LOG.warning('Could not find alternative host for ' + 'reservation %s (lease: %s).', + reservation['id'], lease['name']) ret = False continue @@ -796,8 +796,8 @@ class VirtualInstancePlugin(base.BasePlugin, nova.NovaClientWrapper): self.placement_client.update_reservation_inventory( new_host['hypervisor_hostname'], reservation['id'], num, additional=True) - LOG.warn('Resource changed for reservation %s (lease: %s).', - reservation['id'], lease['name']) + LOG.warning('Resource changed for reservation %s (lease: %s).', + reservation['id'], lease['name']) def _get_extra_capabilities(self, host_id): extra_capabilities = {} diff --git a/blazar/plugins/oshosts/host_plugin.py b/blazar/plugins/oshosts/host_plugin.py index 98837a23..ebed70a6 100644 --- a/blazar/plugins/oshosts/host_plugin.py +++ b/blazar/plugins/oshosts/host_plugin.py @@ -282,15 +282,15 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper): ) if not new_hostids: db_api.host_allocation_destroy(allocation['id']) - LOG.warn('Could not find alternative host for reservation %s ' - '(lease: %s).', reservation['id'], lease['name']) + LOG.warning('Could not find alternative host for reservation %s ' + '(lease: %s).', reservation['id'], lease['name']) return False else: new_hostid = new_hostids.pop() db_api.host_allocation_update(allocation['id'], {'compute_host_id': new_hostid}) - LOG.warn('Resource changed for reservation %s (lease: %s).', - reservation['id'], lease['name']) + LOG.warning('Resource changed for reservation %s (lease: %s).', + reservation['id'], lease['name']) if reservation['status'] == status.reservation.ACTIVE: # Add the alternative host into the aggregate. new_host = db_api.host_get(new_hostid) @@ -802,8 +802,8 @@ class PhysicalHostMonitorPlugin(base.BaseMonitorPlugin, failed_hosts = db_api.reservable_host_get_all_by_queries( ['hypervisor_hostname == ' + data['host']]) if failed_hosts: - LOG.warn('%s failed.', - failed_hosts[0]['hypervisor_hostname']) + LOG.warning('%s failed.', + failed_hosts[0]['hypervisor_hostname']) reservation_flags = self._handle_failures(failed_hosts) else: recovered_hosts = db_api.host_get_all_by_queries( @@ -812,8 +812,8 @@ class PhysicalHostMonitorPlugin(base.BaseMonitorPlugin, if recovered_hosts: db_api.host_update(recovered_hosts[0]['id'], {'reservable': True}) - LOG.warn('%s recovered.', - recovered_hosts[0]['hypervisor_hostname']) + LOG.warning('%s recovered.', + recovered_hosts[0]['hypervisor_hostname']) return reservation_flags @@ -838,12 +838,12 @@ class PhysicalHostMonitorPlugin(base.BaseMonitorPlugin, failed_hosts, recovered_hosts = self._poll_resource_failures() if failed_hosts: for host in failed_hosts: - LOG.warn('%s failed.', host['hypervisor_hostname']) + LOG.warning('%s failed.', host['hypervisor_hostname']) reservation_flags = self._handle_failures(failed_hosts) if recovered_hosts: for host in recovered_hosts: db_api.host_update(host['id'], {'reservable': True}) - LOG.warn('%s recovered.', host['hypervisor_hostname']) + LOG.warning('%s recovered.', host['hypervisor_hostname']) return reservation_flags diff --git a/blazar/status.py b/blazar/status.py index 805f46be..d2df8eea 100644 --- a/blazar/status.py +++ b/blazar/status.py @@ -38,8 +38,8 @@ class BaseStatus(object): """ if next_status not in cls.NEXT_STATUSES[current_status]: - LOG.warn('Invalid transition from %s to %s.', - current_status, next_status) + LOG.warning('Invalid transition from %s to %s.', + current_status, next_status) return False return True @@ -135,7 +135,7 @@ class LeaseStatus(BaseStatus): if cls.is_valid_combination(kwargs['lease_id'], next): return True else: - LOG.warn('Invalid combination of statuses.') + LOG.warning('Invalid combination of statuses.') return False @@ -203,10 +203,11 @@ class LeaseStatus(BaseStatus): LOG.debug('Status of lease %s changed from %s to %s.', lease_id, lease['status'], transition) else: - LOG.warn('Aborting %s. ' - 'Invalid lease status transition from %s to %s.', - func.__name__, lease['status'], - transition) + LOG.warning('Aborting %s. ' + 'Invalid lease status transition ' + 'from %s to %s.', + func.__name__, lease['status'], + transition) raise exceptions.InvalidStatus # Executing the wrapped function diff --git a/blazar/tests/local_hacking/test_hacking.py b/blazar/tests/local_hacking/test_hacking.py index b8051c40..e57e43af 100644 --- a/blazar/tests/local_hacking/test_hacking.py +++ b/blazar/tests/local_hacking/test_hacking.py @@ -25,3 +25,10 @@ class HackingTestCase(tests.TestCase): # Catch abuses when used with a variable and not a literal bad = 'LOG.%s(%s(msg))' % (log, hint) self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) + + def test_no_log_warn(self): + bad = 'LOG.warn("LOG.warn is deprecated")' + self.assertEqual(1, len(list(checks.no_log_warn(bad)))) + + good = 'LOG.warning("LOG.warn is deprecated")' + self.assertEqual(0, len(list(checks.no_log_warn(good)))) diff --git a/blazar/utils/openstack/nova.py b/blazar/utils/openstack/nova.py index ba7a6a2b..7b16bd4c 100644 --- a/blazar/utils/openstack/nova.py +++ b/blazar/utils/openstack/nova.py @@ -281,7 +281,7 @@ class ReservationPool(NovaClientWrapper): try: agg = self.get_aggregate_from_name_or_id(pool) except manager_exceptions.AggregateNotFound: - LOG.warn("Aggregate '%s' not found, skipping deletion", pool) + LOG.warning("Aggregate '%s' not found, skipping deletion", pool) return hosts = agg.hosts @@ -373,12 +373,12 @@ class ReservationPool(NovaClientWrapper): pool=pool, host=host, nova_exception=str(e)) except Exception as e: if added_hosts: - LOG.warn('Removing hosts added to aggregate %s: %s', - agg.id, added_hosts) + LOG.warning('Removing hosts added to aggregate %s: %s', + agg.id, added_hosts) for host in added_hosts: self.nova.aggregates.remove_host(agg.id, host) if removed_hosts: - LOG.warn('Adding hosts back to freepool: %s', removed_hosts) + LOG.warning('Adding hosts back to freepool: %s', removed_hosts) for host in removed_hosts: self.nova.aggregates.add_host(freepool_agg.id, host) raise e diff --git a/tox.ini b/tox.ini index 7ca4d675..eedcad03 100644 --- a/tox.ini +++ b/tox.ini @@ -74,7 +74,8 @@ max-complexity=23 [flake8:local-plugins] extension = - C312 = checks:no_translate_logs + Bl301 = checks:no_translate_logs + Bl302 = checks:no_log_warn paths = ./blazar/hacking [testenv:pylint]