Skip to content
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

MountLock context manager (was: Exception handling in mount/unmount; Enable lint check W0706) #1799

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

feman323
Copy link
Contributor

W0706 complains if an Exception is directly raised back without any handling. First I wanted to remove the try / catch blocks completely, but the I've seen that there are also "finally" statements. Therefore I just added some logging, which caused the lint check to run successful.

@buhtz
Copy link
Member

buhtz commented Jul 17, 2024

Hi Manual,
thank your for this contribution. I would set this as draft because it needs further investigation and resources.
Your solution might silence the linter but does not improve the quality of code or BIT behavior. The log output is polluted without any reason.

There is more wrong then just a not handled exception. On a quick view I would assume that the goal of this code construct was that the code block in finally will be executed no matter if there is an exception or not. It might have been the only way in older Python versions but not in today Python.

Today you would solve something like this with a context manager.

But I am not sure at all and would need some time to dive into it.

Best,
Christian

As an example you see here that the exception is re-raised but finally is also executed.

>>> try:
...   x = 7 / 0
... except Exception:
...    raise
... finally:
...   print('FF')
... 
FF
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero
>>> 

@buhtz buhtz added the Code Quality About code quality, refactoring, (unit) testing, linting, ... label Jul 17, 2024
@buhtz buhtz added this to the 2nd release from now milestone Jul 17, 2024
@buhtz buhtz marked this pull request as draft July 17, 2024 13:34
@buhtz buhtz changed the title Enable lint check W0706 Enable lint check W0706 (refactor exception handling in mount/unmount; implement MountLock context manager) Jul 17, 2024
@buhtz buhtz assigned buhtz and unassigned buhtz Jul 17, 2024
@feman323
Copy link
Contributor Author

Hi Christian,

Okay, I will try to embed this in a context manager.

Regards,

Manuel

@feman323 feman323 marked this pull request as ready for review July 23, 2024 07:30
@buhtz buhtz added the PR: Modifications requested Maintenance team requested modifications and waiting for their implementation label Aug 11, 2024
@buhtz buhtz marked this pull request as draft August 11, 2024 15:23
@buhtz
Copy link
Member

buhtz commented Aug 14, 2024

Please have a look if our (quit fresh) flock module and its base class _FlockContext might help you. I am assuming that base class need to be modified somehow to be more flexible.

@buhtz
Copy link
Member

buhtz commented Sep 16, 2024

My advice would be to extract the five methods mountLock*() and mountProcess*() into something else. A context manager class for example.

@buhtz buhtz removed the Code Quality About code quality, refactoring, (unit) testing, linting, ... label Sep 16, 2024
@buhtz buhtz changed the title Enable lint check W0706 (refactor exception handling in mount/unmount; implement MountLock context manager) MountLock context manager (was: Exception handling in mount/unmount; Enable lint check W0706) Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Modifications requested Maintenance team requested modifications and waiting for their implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants