From e5fdb3d3e15689c9101d361c7f37009ca8e696b2 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Mon, 14 Feb 2022 13:49:57 +0100 Subject: [PATCH 01/10] Raise PlatformMismatchError if using wrong class Instead of leaving abstract methods without definition when running on the other platform (Unix vs Windows), define them and raise a PlatformMismatchError if called on the wrong platform. This fixes pylint warnings such as Abstract class 'WindowsFileLock' with abstract methods instantiated Abstract class 'UnixFileLock' with abstract methods instantiated Fixes tox-dev/py-filelock#102 --- src/filelock/_api.py | 5 +++++ src/filelock/_unix.py | 10 ++++++++-- src/filelock/_windows.py | 10 ++++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index 3551d5d..0289a55 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -14,6 +14,10 @@ _LOGGER = logging.getLogger("filelock") +class PlatformMismatchError(Exception): + """Exception raised when attempting Unix locks on Windows, or vice versa.""" + + # This is a helper class which is returned by :meth:`BaseFileLock.acquire` and wraps the lock to make sure __enter__ # is not called twice when entering the with statement. If we would simply return *self*, the lock would be acquired # again in the *__enter__* method of the BaseFileLock, but not released again automatically. issue #37 (memory leak) @@ -236,4 +240,5 @@ def __del__(self) -> None: __all__ = [ "BaseFileLock", "AcquireReturnProxy", + "PlatformMismatchError", ] diff --git a/src/filelock/_unix.py b/src/filelock/_unix.py index e988380..1b3fabb 100644 --- a/src/filelock/_unix.py +++ b/src/filelock/_unix.py @@ -5,15 +5,21 @@ from abc import ABC from typing import cast -from ._api import BaseFileLock +from ._api import BaseFileLock, PlatformMismatchError #: a flag to indicate if the fcntl API is available has_fcntl = False if sys.platform == "win32": # pragma: win32 cover - class UnixFileLock(BaseFileLock, ABC): + class UnixFileLock(BaseFileLock): """Uses the :func:`fcntl.flock` to hard lock the lock file on unix systems.""" + def _acquire(self) -> None: + raise PlatformMismatchError() + + def _release(self) -> None: + raise PlatformMismatchError() + else: # pragma: win32 no cover try: import fcntl diff --git a/src/filelock/_windows.py b/src/filelock/_windows.py index affd93e..d4c2908 100644 --- a/src/filelock/_windows.py +++ b/src/filelock/_windows.py @@ -6,7 +6,7 @@ from errno import ENOENT from typing import cast -from ._api import BaseFileLock +from ._api import BaseFileLock, PlatformMismatchError from ._util import raise_on_exist_ro_file if sys.platform == "win32": # pragma: win32 cover @@ -49,9 +49,15 @@ def _release(self) -> None: else: # pragma: win32 no cover - class WindowsFileLock(BaseFileLock, ABC): + class WindowsFileLock(BaseFileLock): """Uses the :func:`msvcrt.locking` function to hard lock the lock file on windows systems.""" + def _acquire(self) -> None: + raise PlatformMismatchError() + + def _release(self) -> None: + raise PlatformMismatchError() + __all__ = [ "WindowsFileLock", From a2fa7858efae38753158a8e2fc80175dc0adae68 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Mon, 14 Feb 2022 14:17:58 +0100 Subject: [PATCH 02/10] Remove unused ABC import --- src/filelock/_unix.py | 1 - src/filelock/_windows.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/filelock/_unix.py b/src/filelock/_unix.py index 1b3fabb..002110c 100644 --- a/src/filelock/_unix.py +++ b/src/filelock/_unix.py @@ -2,7 +2,6 @@ import os import sys -from abc import ABC from typing import cast from ._api import BaseFileLock, PlatformMismatchError diff --git a/src/filelock/_windows.py b/src/filelock/_windows.py index d4c2908..8cd6561 100644 --- a/src/filelock/_windows.py +++ b/src/filelock/_windows.py @@ -2,7 +2,6 @@ import os import sys -from abc import ABC from errno import ENOENT from typing import cast From 62f649f124615018f4c7da866a705664e92e49e7 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 11:17:39 +0100 Subject: [PATCH 03/10] Use NotImplementedError, not PlatformMismatchError Instead of defining a custom exception, PlatformMismatchError, use the standard NotImplementedError. --- src/filelock/_api.py | 5 ----- src/filelock/_unix.py | 6 +++--- src/filelock/_windows.py | 6 +++--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index 0289a55..3551d5d 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -14,10 +14,6 @@ _LOGGER = logging.getLogger("filelock") -class PlatformMismatchError(Exception): - """Exception raised when attempting Unix locks on Windows, or vice versa.""" - - # This is a helper class which is returned by :meth:`BaseFileLock.acquire` and wraps the lock to make sure __enter__ # is not called twice when entering the with statement. If we would simply return *self*, the lock would be acquired # again in the *__enter__* method of the BaseFileLock, but not released again automatically. issue #37 (memory leak) @@ -240,5 +236,4 @@ def __del__(self) -> None: __all__ = [ "BaseFileLock", "AcquireReturnProxy", - "PlatformMismatchError", ] diff --git a/src/filelock/_unix.py b/src/filelock/_unix.py index 002110c..03b612c 100644 --- a/src/filelock/_unix.py +++ b/src/filelock/_unix.py @@ -4,7 +4,7 @@ import sys from typing import cast -from ._api import BaseFileLock, PlatformMismatchError +from ._api import BaseFileLock #: a flag to indicate if the fcntl API is available has_fcntl = False @@ -14,10 +14,10 @@ class UnixFileLock(BaseFileLock): """Uses the :func:`fcntl.flock` to hard lock the lock file on unix systems.""" def _acquire(self) -> None: - raise PlatformMismatchError() + raise NotImplementedError def _release(self) -> None: - raise PlatformMismatchError() + raise NotImplementedError else: # pragma: win32 no cover try: diff --git a/src/filelock/_windows.py b/src/filelock/_windows.py index 8cd6561..60e68cb 100644 --- a/src/filelock/_windows.py +++ b/src/filelock/_windows.py @@ -5,7 +5,7 @@ from errno import ENOENT from typing import cast -from ._api import BaseFileLock, PlatformMismatchError +from ._api import BaseFileLock from ._util import raise_on_exist_ro_file if sys.platform == "win32": # pragma: win32 cover @@ -52,10 +52,10 @@ class WindowsFileLock(BaseFileLock): """Uses the :func:`msvcrt.locking` function to hard lock the lock file on windows systems.""" def _acquire(self) -> None: - raise PlatformMismatchError() + raise NotImplementedError def _release(self) -> None: - raise PlatformMismatchError() + raise NotImplementedError __all__ = [ From 1d61cdb5e9826217b9a9118ab12ba47953e50398 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 14:58:25 +0100 Subject: [PATCH 04/10] Add tests and a note in the change log Add tests of UnixFileLock on Windows systems and WindowsFileLock on non-Windows systems, to make sure they are not abstract (but still raise NotImplementedError). This avoids pylint errors in code using filelock. Also added a note to the change log. --- docs/changelog.rst | 7 +++++++ tests/test_filelock.py | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index acd9c56..396ce13 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,13 @@ Changelog ========= +Future +------ +- Fix pylint warning "Abstract class :class:`WindowsFileLock ` with abstract methods instantiated" + :pr:`135` - by :user:`vonschultz` +- Fix pylint warning "Abstract class :class:`UnixFileLock ` with abstract methods instantiated" + :pr:`135` - by :user:`vonschultz` + v3.4.2 (2021-12-16) ------------------- - Drop support for python ``3.6`` diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 6991137..6b7f003 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -1,5 +1,6 @@ from __future__ import annotations +import inspect import logging import sys import threading @@ -13,7 +14,7 @@ import pytest from _pytest.logging import LogCaptureFixture -from filelock import BaseFileLock, FileLock, SoftFileLock, Timeout +from filelock import BaseFileLock, FileLock, SoftFileLock, Timeout, UnixFileLock, WindowsFileLock @pytest.mark.parametrize( @@ -368,3 +369,35 @@ def test_poll_intervall_deprecated(lock_type: type[BaseFileLock], tmp_path: Path break else: # pragma: no cover pytest.fail("No warnings of stacklevel=2 matching.") + + +@pytest.mark.skipif(sys.platform != "win32", reason="Tests behavior of UnixFileLock on Windows systems") +def test_unix_lock_on_windows(tmp_path: Path) -> None: + lock = UnixFileLock(str(tmp_path / "lockfile")) + + assert not inspect.isabstract( + UnixFileLock + ), "UnixFileLock must not be an abstract class, or pylint will complain in client code" + assert inspect.isabstract(BaseFileLock) + + with pytest.raises(NotImplementedError): + lock.acquire() + + with pytest.raises(NotImplementedError): + lock._release() + + +@pytest.mark.skipif(sys.platform == "win32", reason="Tests behavior of WindowsFileLock on non-Windows systems") +def test_windows_lock_on_unix(tmp_path: Path) -> None: + lock = WindowsFileLock(str(tmp_path / "lockfile")) + + assert not inspect.isabstract( + WindowsFileLock + ), "WindowsFileLock must not be an abstract class, or pylint will complain in client code" + assert inspect.isabstract(BaseFileLock) + + with pytest.raises(NotImplementedError): + lock.acquire() + + with pytest.raises(NotImplementedError): + lock._release() From 8954719d889e65c9a4c6e8a92c29c8bcbb213c6a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 17 Feb 2022 14:13:55 +0000 Subject: [PATCH 05/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_filelock.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 2967cac..3afd971 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -371,7 +371,6 @@ def test_poll_intervall_deprecated(lock_type: type[BaseFileLock], tmp_path: Path pytest.fail("No warnings of stacklevel=2 matching.") - @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) def test_context_decorator(lock_type: type[BaseFileLock], tmp_path: Path) -> None: lock_path = tmp_path / "a" From 6d1f52f44cc839535c13977792370d5c115d6ccc Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 15:18:55 +0100 Subject: [PATCH 06/10] Tell flake8 that isabstract is not a misspelling The Python standard library module "inspect" has a function called "isabstract". That is not a spelling mistake. Silence flake8 SC200 warning about "isabstract". --- tests/test_filelock.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 3afd971..fc697b1 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -389,10 +389,10 @@ def decorated_method() -> None: def test_unix_lock_on_windows(tmp_path: Path) -> None: lock = UnixFileLock(str(tmp_path / "lockfile")) - assert not inspect.isabstract( + assert not inspect.isabstract( # noqa: SC200 UnixFileLock ), "UnixFileLock must not be an abstract class, or pylint will complain in client code" - assert inspect.isabstract(BaseFileLock) + assert inspect.isabstract(BaseFileLock) # noqa: SC200 with pytest.raises(NotImplementedError): lock.acquire() @@ -405,10 +405,10 @@ def test_unix_lock_on_windows(tmp_path: Path) -> None: def test_windows_lock_on_unix(tmp_path: Path) -> None: lock = WindowsFileLock(str(tmp_path / "lockfile")) - assert not inspect.isabstract( + assert not inspect.isabstract( # noqa: SC200 WindowsFileLock ), "WindowsFileLock must not be an abstract class, or pylint will complain in client code" - assert inspect.isabstract(BaseFileLock) + assert inspect.isabstract(BaseFileLock) # noqa: SC200 with pytest.raises(NotImplementedError): lock.acquire() From dc782e297fbc777fadf5bda814a24b495624dfc0 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 15:29:19 +0100 Subject: [PATCH 07/10] Make the wrong-platform tests a single test Make a single test, and select lock_type based on sys.platform. --- tests/test_filelock.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index fc697b1..ff0fe39 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -385,26 +385,13 @@ def decorated_method() -> None: assert not lock.is_locked -@pytest.mark.skipif(sys.platform != "win32", reason="Tests behavior of UnixFileLock on Windows systems") -def test_unix_lock_on_windows(tmp_path: Path) -> None: - lock = UnixFileLock(str(tmp_path / "lockfile")) +def test_wrong_platform(tmp_path: Path) -> None: + lock_type = UnixFileLock if sys.platform == "win32" else WindowsFileLock + lock = lock_type(str(tmp_path / "lockfile")) assert not inspect.isabstract( # noqa: SC200 UnixFileLock ), "UnixFileLock must not be an abstract class, or pylint will complain in client code" - assert inspect.isabstract(BaseFileLock) # noqa: SC200 - - with pytest.raises(NotImplementedError): - lock.acquire() - - with pytest.raises(NotImplementedError): - lock._release() - - -@pytest.mark.skipif(sys.platform == "win32", reason="Tests behavior of WindowsFileLock on non-Windows systems") -def test_windows_lock_on_unix(tmp_path: Path) -> None: - lock = WindowsFileLock(str(tmp_path / "lockfile")) - assert not inspect.isabstract( # noqa: SC200 WindowsFileLock ), "WindowsFileLock must not be an abstract class, or pylint will complain in client code" From a2458d9b1cac9961d8dfca0d70fdcfeed2e6b9a4 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 15:30:00 +0100 Subject: [PATCH 08/10] Set release date to today Next minor release, and today's release date in the change log. --- docs/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 780fe58..80d92c7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,8 +1,8 @@ Changelog ========= -Future ------- +v3.6.0 (2022-02-17) +------------------- - Fix pylint warning "Abstract class :class:`WindowsFileLock ` with abstract methods instantiated" :pr:`135` - by :user:`vonschultz` - Fix pylint warning "Abstract class :class:`UnixFileLock ` with abstract methods instantiated" From 399576e348d8ee26f3053d646a86434427fe83e3 Mon Sep 17 00:00:00 2001 From: Christian von Schultz Date: Thu, 17 Feb 2022 16:01:31 +0100 Subject: [PATCH 09/10] Move isabstract check before lock instantiation To make it a bit clearer, move the preliminary check of abstractness before the instantiation of a lock, so that the creation of the lock and the pytest.raises(NotImplementedError) are adjacent. --- tests/test_filelock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index ff0fe39..a66cdf8 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -386,9 +386,6 @@ def decorated_method() -> None: def test_wrong_platform(tmp_path: Path) -> None: - lock_type = UnixFileLock if sys.platform == "win32" else WindowsFileLock - lock = lock_type(str(tmp_path / "lockfile")) - assert not inspect.isabstract( # noqa: SC200 UnixFileLock ), "UnixFileLock must not be an abstract class, or pylint will complain in client code" @@ -397,6 +394,9 @@ def test_wrong_platform(tmp_path: Path) -> None: ), "WindowsFileLock must not be an abstract class, or pylint will complain in client code" assert inspect.isabstract(BaseFileLock) # noqa: SC200 + lock_type = UnixFileLock if sys.platform == "win32" else WindowsFileLock + lock = lock_type(str(tmp_path / "lockfile")) + with pytest.raises(NotImplementedError): lock.acquire() From 4f0e0207691cbabc58b5260ab6006a588b57f8b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Thu, 17 Feb 2022 18:09:12 +0000 Subject: [PATCH 10/10] PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- tests/test_filelock.py | 11 +++-------- whitelist.txt | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index a66cdf8..f439ff6 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -386,19 +386,14 @@ def decorated_method() -> None: def test_wrong_platform(tmp_path: Path) -> None: - assert not inspect.isabstract( # noqa: SC200 - UnixFileLock - ), "UnixFileLock must not be an abstract class, or pylint will complain in client code" - assert not inspect.isabstract( # noqa: SC200 - WindowsFileLock - ), "WindowsFileLock must not be an abstract class, or pylint will complain in client code" - assert inspect.isabstract(BaseFileLock) # noqa: SC200 + assert not inspect.isabstract(UnixFileLock) + assert not inspect.isabstract(WindowsFileLock) + assert inspect.isabstract(BaseFileLock) lock_type = UnixFileLock if sys.platform == "win32" else WindowsFileLock lock = lock_type(str(tmp_path / "lockfile")) with pytest.raises(NotImplementedError): lock.acquire() - with pytest.raises(NotImplementedError): lock._release() diff --git a/whitelist.txt b/whitelist.txt index 6882459..7eeb6b9 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -15,6 +15,7 @@ fmt fspath intersphinx intervall +isabstract iwgrp iwoth iwusr