From e57a1c829df3f77d2c3a9c2712f1decef6284500 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 18 Oct 2019 13:43:45 +0200 Subject: Don't warn on lock removal if file doesn't exist There may be cases were library users want to request the removal of an external lock that may not have been created and don't want to pollute the logs when the file is not present. We raise the log level from info to warning, but we don't log it if the file didn't exist, since the end result the caller wanted is there. Change-Id: I5ce8be34c9f2c4c59ea99dabc6760c3300f743a3 --- oslo_concurrency/lockutils.py | 8 +++++--- oslo_concurrency/tests/unit/test_lockutils.py | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/oslo_concurrency/lockutils.py b/oslo_concurrency/lockutils.py index 7fe049b..3236289 100644 --- a/oslo_concurrency/lockutils.py +++ b/oslo_concurrency/lockutils.py @@ -14,6 +14,7 @@ # under the License. import contextlib +import errno import functools import logging import os @@ -200,9 +201,10 @@ def remove_external_lock_file(name, lock_file_prefix=None, lock_path=None, lock_file_path = _get_lock_path(name, lock_file_prefix, lock_path) try: os.remove(lock_file_path) - except OSError: - LOG.info('Failed to remove file %(file)s', - {'file': lock_file_path}) + except OSError as exc: + if exc.errno != errno.ENOENT: + LOG.warning('Failed to remove file %(file)s', + {'file': lock_file_path}) def internal_lock(name, semaphores=None): diff --git a/oslo_concurrency/tests/unit/test_lockutils.py b/oslo_concurrency/tests/unit/test_lockutils.py index 1bd74a2..2aea215 100644 --- a/oslo_concurrency/tests/unit/test_lockutils.py +++ b/oslo_concurrency/tests/unit/test_lockutils.py @@ -13,6 +13,7 @@ # under the License. import collections +import errno import multiprocessing import os import signal @@ -327,8 +328,8 @@ class LockTestCase(test_base.BaseTestCase): remove_mock.assert_called_once_with(path_mock.return_value) log_mock.assert_not_called() - @mock.patch('logging.Logger.info') - @mock.patch('os.remove', side_effect=OSError) + @mock.patch('logging.Logger.warning') + @mock.patch('os.remove', side_effect=OSError(errno.ENOENT, None)) @mock.patch('oslo_concurrency.lockutils._get_lock_path') def test_remove_lock_external_file_doesnt_exists(self, path_mock, remove_mock, log_mock): @@ -339,6 +340,20 @@ class LockTestCase(test_base.BaseTestCase): mock.sentinel.prefix, mock.sentinel.lock_path) remove_mock.assert_called_once_with(path_mock.return_value) + log_mock.assert_not_called() + + @mock.patch('logging.Logger.warning') + @mock.patch('os.remove', side_effect=OSError(errno.EPERM, None)) + @mock.patch('oslo_concurrency.lockutils._get_lock_path') + def test_remove_lock_external_file_permission_error( + self, path_mock, remove_mock, log_mock): + lockutils.remove_external_lock_file(mock.sentinel.name, + mock.sentinel.prefix, + mock.sentinel.lock_path) + path_mock.assert_called_once_with(mock.sentinel.name, + mock.sentinel.prefix, + mock.sentinel.lock_path) + remove_mock.assert_called_once_with(path_mock.return_value) log_mock.assert_called() def test_no_slash_in_b64(self): -- cgit v1.2.1