-
-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow users to subclass FileLock with custom keyword arguments #284
Conversation
With the updated test:
|
Thank you for your consideration |
78dfb46
to
1ac2251
Compare
hmm. the cis were running. now they are not. strange. do they need to be approved one at a time for me??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's your use case for subclassing?
I guess I had an implementation of #192 a while back (this got merged in in 2022, but we have had our own class since 2020) and at the time we decided to use subclasses. We have since learned that subclassing creates these kinds of potential issues and we have generally moved away from using new subclasses. However this code is still in use today. |
your help resolving the mypy issues would be greatly appreciated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the fast review |
and for the fast release |
Note the added test gives a
You need to call the parent class --- a/tests/test_filelock.py
+++ b/tests/test_filelock.py
@@ -624,7 +624,8 @@ def test_subclass_compatibility(tmp_path: Path) -> None:
my_param: int = 0,
**kwargs: dict[str, Any],
) -> None:
- pass
+ FileLock.__init__(self, lock_file, timeout, mode,
+ thread_local, **kwargs)
lock_path = tmp_path / "a"
MyFileLock(str(lock_path), my_param=1)
@@ -639,7 +640,8 @@ def test_subclass_compatibility(tmp_path: Path) -> None:
my_param: int = 0,
**kwargs: dict[str, Any],
) -> None:
- pass
+ SoftFileLock.__init__(self, lock_file, timeout, mode,
+ thread_local, **kwargs)
MySoftFileLock(str(lock_path), my_param=1)
|
Without this patch:
Resolves the unstated issues in the comment: #282 (comment)