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
Next Next commit
Use post instead of get for sanity of use-case.
  • Loading branch information
@ticosax
ticosax committed Jun 2, 2015
commit d1371cc949afcc66c7e7f497bab62ec655cddf31
12 changes: 6 additions & 6 deletions tests/test_atomic_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@


class BasicView(APIView):
def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
BasicModel.objects.create()
return Response({'method': 'GET'})


class ErrorView(APIView):
def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
BasicModel.objects.create()
raise Exception


class APIExceptionView(APIView):
def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
BasicModel.objects.create()
raise APIException

Expand All @@ -43,7 +43,7 @@ def tearDown(self):
connections.databases['default']['ATOMIC_REQUESTS'] = False

def test_no_exception_conmmit_transaction(self):
request = factory.get('/')
request = factory.post('/')

with self.assertNumQueries(1):
response = self.view(request)
Expand All @@ -66,7 +66,7 @@ def test_error_rollback_transaction(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.

"""
request = factory.get('/')
request = factory.post('/')
with self.assertNumQueries(3):
# 1 - begin savepoint
# 2 - insert
Expand All @@ -90,7 +90,7 @@ def test_api_exception_rollback_transaction(self):
"""
Transaction is rollbacked by our transaction atomic block.
"""
request = factory.get('/')
request = factory.post('/')
num_queries = (4 if getattr(connection.features,
'can_release_savepoints', False) else 3)
with self.assertNumQueries(num_queries):
Expand Down