Skip to content

Rollback the transaction on error if ATOMIC_REQUESTS is set. #2887

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

Merged
xordoquy merged 5 commits into from
Jun 2, 2015
Merged

Rollback the transaction on error if ATOMIC_REQUESTS is set. #2887

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
27fd485
allow to pass arbitrary arguments to py.test
ticosax Apr 22, 2015
c2d2417
Tell default error handler to doom the transaction on error
ticosax Apr 29, 2015
d1371cc
Use post instead of get for sanity of use-case.
ticosax Apr 29, 2015
8ad3820
more assertions make the test more readable
ticosax Apr 29, 2015
34dc98e
improve wording
ticosax Apr 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
improve wording
  • Loading branch information
@ticosax
ticosax committed Jun 2, 2015
commit 34dc98e8ad7ff82f82e58c6bf2170bacfdb449c7
4 changes: 3 additions & 1 deletion tests/test_atomic_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@ def setUp(self):
def tearDown(self):
connections.databases['default']['ATOMIC_REQUESTS'] = False

def test_error_rollback_transaction(self):
def test_generic_exception_delegate_transaction_management(self):
"""
Transaction is eventually managed by outer-most transaction atomic
block. DRF do not try to interfere here.
Copy link
Contributor

@lovelydinosaur lovelydinosaur Apr 29, 2015

Choose a reason for hiding this comment

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

I'm not immediately sure what this means - would the test be different if we were using the test client, rather than calling the view directly with the request?

Copy link
Contributor Author

@ticosax ticosax Apr 29, 2015

Choose a reason for hiding this comment

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

yes, views are wrapped by django.core.handlers.base.make_view_atomic(view) if one of the database defines ATOMIC_REQUESTS to True.

https://github.com/django/django/blob/master/django/core/handlers/base.py#L75

In the context of the test, I just check that drf's exception_handler doesn't intefere and let
django.core.handlers.base.make_view_atomic(view) do the job because the exception is raised.


We let django deal with the transaction when it will catch the Exception.
"""
request = factory.post('/')
with self.assertNumQueries(3):
Expand Down