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

reraise is complicated #7

Closed
faassen opened this Issue Jul 18, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@faassen
Member

faassen commented Jul 18, 2016

There's complicated logic to implement a reraise functionality, different in Python 2 and 3. I am wondering why this is not simply implemented with raise (without arguments). Perhaps this has to do with some kind of traceback manipulation I don't quite understand.

Note also that the PY3 compat layer defines an exec_ function that doesn't appear to be used.

If we can get rid of the complexity that would be nice -- running exec and frame manipulation is pretty extreme.

@faassen

This comment has been minimized.

Show comment
Hide comment
@faassen

faassen Jul 18, 2016

Member

To figure out why it is this way we should investigate the pyramid_tm logic and commit history, hopefully it explains things.

Member

faassen commented Jul 18, 2016

To figure out why it is this way we should investigate the pyramid_tm logic and commit history, hopefully it explains things.

@taschini

This comment has been minimized.

Show comment
Hide comment
@taschini

taschini Jul 18, 2016

Member

That had caught my eye too. AFAICS there is no reason why a naked raise shouldn't be enough.

Member

taschini commented Jul 18, 2016

That had caught my eye too. AFAICS there is no reason why a naked raise shouldn't be enough.

@taschini

This comment has been minimized.

Show comment
Hide comment
@taschini

taschini Jul 18, 2016

Member

Here's the relevant commit: Pylons/pyramid_tm@1c4c389

They went from a context manager, which requires reraise as part of the __exit__ special method, to a try…except clause, and simply copied over the code fragment.

I'm definitely of the opinion that reraise can be safely removed, compat.py deleted and the except clause simplified to:

            except:
                ex_type, ex_value = sys.exc_info()[:2]
                retryable = manager._retryable(ex_type, ex_value)
                manager.abort()
                if (number <= 0) or (not retryable):
                    raise
Member

taschini commented Jul 18, 2016

Here's the relevant commit: Pylons/pyramid_tm@1c4c389

They went from a context manager, which requires reraise as part of the __exit__ special method, to a try…except clause, and simply copied over the code fragment.

I'm definitely of the opinion that reraise can be safely removed, compat.py deleted and the except clause simplified to:

            except:
                ex_type, ex_value = sys.exc_info()[:2]
                retryable = manager._retryable(ex_type, ex_value)
                manager.abort()
                if (number <= 0) or (not retryable):
                    raise
@faassen

This comment has been minimized.

Show comment
Hide comment
@faassen

faassen Jul 18, 2016

Member

@taschini thanks for looking into it! Go ahead and apply this simplication.

Member

faassen commented Jul 18, 2016

@taschini thanks for looking into it! Go ahead and apply this simplication.

@taschini taschini closed this in 2684852 Jul 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment