Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

support multi db atomic_requests #1

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

jmo-qap
Copy link

@jmo-qap jmo-qap commented Mar 2, 2021

Description

When using https://docs.djangoproject.com/en/3.1/ref/settings/#std:setting-DATABASE-ATOMIC_REQUESTS, django wraps each invoked view with transactions to all databases (https://github.com/django/django/blob/master/django/core/handlers/base.py#L323-L330). encode#2887 resolved encode#2034 for the default database, only.

In particular, https://github.com/encode/django-rest-framework/blob/master/rest_framework/views.py#L67-103 does not handle the fact that multiple transactions can be created by django. This PR adds a unit test and associated fix to ensure DRF rollsback all transactions setup by django for ATOMIC_REQUEST rather than just the default db's transaction.

Test Plan

Added unit tests. Fail with the old code, passes with the new code.

@jmo-qap jmo-qap requested a review from adamf March 2, 2021 23:22
@jmo-qap jmo-qap merged commit 6cc1a88 into master Mar 3, 2021
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.

Handle ATOMIC_REQUESTS correctly.
2 participants