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

add first class transaction manager support #2606

Closed
mmerickel opened this issue Jun 1, 2016 · 4 comments
Closed

add first class transaction manager support #2606

mmerickel opened this issue Jun 1, 2016 · 4 comments
Milestone

Comments

@mmerickel
Copy link
Member

The current story around transaction management is that it "80% works". There are some related issues:

The TLDR is that 1) retries are completely broken when cached state on the request is shared across transactions in RDBMS data managers and 2) the transaction manager is not capable of wrapping the entire lifecycle of the request. This is a fundamental problem with using tweens which are not capable of wrapping the entire request. Goals:

  • Allow the transaction manager to create new request objects.
  • Allow the transaction manager to wrap the entire lifecycle including response callbacks and exception views.

I'm proposing adding transaction manager support to the core of Pyramid, providing necessary hooks for pyramid_tm to do its job.

middleware -> tm -> tweens -> main

Possible API written off the cuff:

class ITransactionManagementPolicy(Interface):
    def handle_request(environ, request_factory, handler):
        """
        Use the request factory to create a request and pass it through the handler.

        request = request_factory(environ)
        response = handler(request)
        return response
        """

config.set_transaction_manager(policy)
@mmerickel mmerickel added this to the 1.8 milestone Jun 1, 2016
@mcdonc
Copy link
Member

mcdonc commented Jun 1, 2016

This doesn't help with "tm should be able to create request" but Mike and I talked about moving the code that calls the response/finished callbacks into a tween that would be autoincluded for the most part. That would allow a TM tween to wrap as much of the request lifecycle as the suggestion above would allow.

@jinty
Copy link
Contributor

jinty commented Jun 2, 2016

I have a few random thoughts which may (or may not) be of interest:

  • rendering the exception view, sometimes this needs a new transaction. At least in my case:
    • I need data from the DB to render the exception.
    • I run on PostgreSQL which fails all SQL after the first error and forces a rollback.
  • error logging:
    • Serialization errors from the DB shouldn't spam the logfiles.
    • But I do really want to know when the user starts to see them (after too many transaction retries)

Those two are handled by tweens in my system and should ideally be "outside" of the transaction retry logic. I really would prefer to not to put them in middleware.

@mmerickel
Copy link
Member Author

Hey @jinty thanks for commenting!

rendering the exception view, sometimes this needs a new transaction. At least in my case:

This should be fine but you would need to consider this on a per-exception basis. If you are just reading from the database then everything will just work because the original transaction is still open. The only time this is not the case is if the exception is from the database at which point you may need to abort the transaction and start a new one (which will be later aborted).

IF you need to write to the database then you should abort/commit inside the exception view because the transaction manager will be configured to always abort upon exceptions. Alternatively if this is for error reporting you might consider using a database session that is not connected to the transaction manager (which is used for normal request operations).

error logging

I think this will need to be controlled by your logging handler. For example, pyramid_exclog can ignore certain types of exceptions.

I really would prefer to not to put them in middleware.

Yep, we are not talking about middleware anymore I don't think. The transaction manager will be a tween.

@mmerickel
Copy link
Member Author

This issue in Pyramid should finally be sorted thanks to pyramid_retry + changes in pyramid_tm + #2964.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants