Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

One transaction per request #510

Merged
merged 6 commits into from
Nov 12, 2015
Merged

One transaction per request #510

merged 6 commits into from
Nov 12, 2015

Conversation

leplatrem
Copy link
Contributor

  • Switch to SQLA sessions
  • Plug SQLA session with pyramid transaction manager
  • Share session among backend instances
  • Change batch response content when one subrequest error occurs
  • Test that crash within request rollbacks global transaction
  • Two-phase commit accross backends ? Session is shared accross backends if URL is the same

@leplatrem leplatrem force-pushed the a-transaction-per-request branch 3 times, most recently from e932fdd to db66ad3 Compare November 6, 2015 17:33
@leplatrem leplatrem changed the title [WIP] Transaction per request One transaction per request Nov 6, 2015
@leplatrem leplatrem force-pushed the a-transaction-per-request branch 5 times, most recently from 480713b to a8b8cec Compare November 9, 2015 10:53

- A transaction now covers the whole request/response cycle (#510, Kinto/kinto#194).
If an error occurs during the request processing, every operation performed
is rolled back. **Note:** This is only enabled with *PostgreSQL* backends.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when I have a postgres backend for the storage and redis for the cache ? can you confirm cache changes are also rolled back ?

@leplatrem leplatrem force-pushed the a-transaction-per-request branch 2 times, most recently from 13668b9 to cb3d55c Compare November 10, 2015 11:40
@leplatrem
Copy link
Contributor Author

This is now ready to review/merge

@almet @tarekziade r?

"""
connection = None
trans = None
with_transaction = (not readonly and self.commit_manually)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need parentheses

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, it's a custom we have for boolean exprs. Do you want me to get rid of it ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reference is always the (recent) Python stdlib for style. It's not considered "pythonic" to have superfluous characters. The typical example is for tuple.

one = two, three, four     # do this
one = (two, three, four)     # not this

One exception of avoiding \ to break a line.

e.g. do:

import (one, two,
            three)     # yes!!
import one, two, \
           three   # boooo!

So I guess my question is: what's the benefit for our code base to do this differently ? If none, I am +1 to use Python stdlib conventions. (We should check of course that it's wlays done like that there :))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 agree

localtimestamp always returns the same timestamp within
a transaction.

clock_timestamp() is a lot more precise, and differs even
within the same statement.

   SELECT clock_timestamp() = clock_timestamp();

   > false
@leplatrem leplatrem force-pushed the a-transaction-per-request branch from cb3d55c to 534cdb4 Compare November 11, 2015 09:35
@tarekziade
Copy link
Contributor

r+

leplatrem added a commit that referenced this pull request Nov 12, 2015
@leplatrem leplatrem merged commit f5dfeb5 into master Nov 12, 2015
@leplatrem leplatrem deleted the a-transaction-per-request branch November 12, 2015 10:58
leplatrem added a commit that referenced this pull request Nov 13, 2015
…er-request"

This reverts commit f5dfeb5, reversing
changes made to 2cb2436.
glasserc pushed a commit that referenced this pull request May 20, 2016
Relax content-type validation when no body is posted (fixes #507)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants