-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Support for fcntl.lockf (and some minor bugfixes) #95
Conversation
…out errors and hiding them until the timeout expires
…e isn't left open in case of failure
Sorry for not responding sooner... I'm not getting all Github mails it seems... I didn't even see your previous pull request, sorry about that :( Thank you so much for your contribution! I'm going to merge it and create a new release asap! |
Thank you so much for the help! I'm going to create a new release today :) |
raise exceptions.LockException(exc_value, fh=file_) from exc_value | ||
else: | ||
# Something else went wrong; don't wrap this so we stop immediately. | ||
raise |
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.
I'm not too keen on implementing this since it could break backwards compatibility. I'm not sure what kind of errors can pop up at all platforms so it should always be wrapped in a LockException
to be compatible with the rest of the code and not break existing implementations
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.
Fair enough, generally I prefer not to wrap errors and just re-raise but here I think it could make sense, especially if it's already established functionality.
The main thing is to raise a different exception for timeout as opposed to any other exception, so the outer loop only tries again for timeouts. Otherwise a non-timeout exception (like giving the wrong syntax to fcntl
) won't propagate until the end of the timeout. For an infinite timeout (which would be unwise anyway but is possible) the loop would never exit otherwise...
No problem, glad it was helpful 👍 Not sure why one of the tests wasn't passing (it was fine on mys system), but if all is green now then all good |
See #93