-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
PEP 760: No more bare excepts #4015
Conversation
944fe81
to
f05871c
Compare
Signed-off-by: Pablo Galindo <[email protected]>
There's one pretty common use case that this will break: transaction handling in the face of exceptions. E.g. from Mailman: @public
@contextmanager
def transaction():
"""Context manager for ensuring the transaction is complete."""
try:
yield
except: # noqa: E722
config.db.abort()
raise
else:
config.db.commit() This guarantees that no matter what exception occurs, any open transaction will be aborted, while in the successful condition, the transaction will be committed. This is a general case of "when anything goes wrong, do this thing and reraise the original exception". If you want to argue that this should use @public
@contextmanager
def transaction():
"""Context manager for ensuring the transaction is complete."""
try:
yield
except AnyException:
config.db.abort()
raise
else:
config.db.commit() Another thought, though probably harder to support, would be to allow bare |
Co-authored-by: Hugo van Kemenade <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Adam Turner <[email protected]> Co-authored-by: Alex Waygood <[email protected]>
Signed-off-by: Pablo Galindo <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
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.
Let's get this merged, especially since people already started discussing the idea on Discourse.
Thanks everyone for you suggestions, comments and fixes. You all rock 🤘 |
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approvalAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
📚 Documentation preview 📚: https://pep-previews--4015.org.readthedocs.build/