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.

Sign up for GitHub

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

Jump to bottom

Improve handle_errors context manager #657

Merged
merged 10 commits into from
Aug 10, 2023
Merged

Improve handle_errors context manager #657

merged 10 commits into from
Aug 10, 2023

Commits on Aug 10, 2023

  1. [web] modify concerns of handle_errors 

    1. doing the abort() inside of `handle_errors` is useful, because
currently, every call of this context manager resides in a `try: …
except PycroftError: return default_response()`-block.

2. The `session.begin_nested()` seems convenient, but just hides
something important from  the person programming the view.
Since in the view, `session.commit()` has to be called explicitly as
well, it is only consistent to also call `session.begin_nested()`
explicitly.

Even more importantly, blindly wrapping the whole cm body in
`begin_nested` has two disadvantages:
a) It discourages „commit often“, which is favorable to long-running
transactions
b) It allows you to think `commit` inside this context will just work,
while this actually will cause problems in production (undetected in
tests, because there `commit()` is a NOP): sqlalchemy disallows
committing while in a subtransaction.

Thus, it makes more sense to let the user call `begin_nested`
explicitly.
    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    15573ac View commit details
    Browse the repository at this point in the history

  2. [ci] add semgrep rule for lack of begin_nested 

    This will be removed after fixing all the usages
    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    8bca177 View commit details
    Browse the repository at this point in the history

  3. [web] explicitly use session.begin_nested()

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    4c506f7 View commit details
    Browse the repository at this point in the history

  4. Bump semgrep in pre-commit config to 1.31.2

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    a1e460a View commit details
    Browse the repository at this point in the history

  5. [ci] add semgrep rule against committing in begin_nested()

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    531e910 View commit details
    Browse the repository at this point in the history

  6. [web] move session committing outside begin_nested() 

    This will not cause errors to be unhandled, because exiting out of the
`begin_nested()` context manager will cause a `flush()`, triggering
all of the possible errors that might occur in the transaction.
    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    5c7929a View commit details
    Browse the repository at this point in the history

  7. [web] replace explicit try-except by handle_errors with parameter

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    446820e View commit details
    Browse the repository at this point in the history

  8. [web] remove session parameter from handle_errors context manager

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    4feb2e7 View commit details
    Browse the repository at this point in the history

  9. [web] remove unused web_execute function 

    It has been replaced by the `handle_errors` context manager.
    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    cbc29d1 View commit details
    Browse the repository at this point in the history

  10. [web] allow to inline constant callables in handle_errors

    @lukasjuhrich
    lukasjuhrich committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    73b4e31 View commit details
    Browse the repository at this point in the history