From 0e67e4f1a144f9ceaf544b91b68592dc3574d890 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Mon, 4 Nov 2024 11:02:53 +0530 Subject: [PATCH] compose_views: Add `savepoint=False` to avoid creating savepoints. 'compose_views' is used inside an outer db transaction created in 'update_user_group_backend'. `transaction.atomic()` block in 'compose_views' resulted in savepoint creation. This commit adds `savepoint=False` to avoid that. --- zerver/tests/test_subs.py | 5 ++++- zerver/transaction_tests/test_user_groups.py | 1 - zerver/views/streams.py | 2 +- zerver/views/user_groups.py | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 8321bc651d..46c8eb6ef9 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -9,6 +9,7 @@ from unittest import mock import orjson from django.conf import settings from django.core.exceptions import ValidationError +from django.db import transaction from django.http import HttpResponse from django.utils.timezone import now as timezone_now from typing_extensions import override @@ -4016,7 +4017,9 @@ class SubscriptionRestApiTest(ZulipTestCase): def thunk2() -> HttpResponse: raise JsonableError("random failure") - with self.assertRaises(JsonableError): + with transaction.atomic(), self.assertRaises(JsonableError): + # The atomic() wrapper helps to avoid JsonableError breaking + # the test's transaction. compose_views([thunk1, thunk2]) user_profile = self.example_user("hamlet") diff --git a/zerver/transaction_tests/test_user_groups.py b/zerver/transaction_tests/test_user_groups.py index 3b5f6be325..1cdeff32ec 100644 --- a/zerver/transaction_tests/test_user_groups.py +++ b/zerver/transaction_tests/test_user_groups.py @@ -28,7 +28,6 @@ def dev_update_subgroups( assert BARRIER is not None try: with ( - transaction.atomic(), mock.patch("zerver.lib.user_groups.access_user_group_for_update") as m, ): diff --git a/zerver/views/streams.py b/zerver/views/streams.py index d618d473e5..6f0246cba5 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -468,7 +468,7 @@ def compose_views(thunks: list[Callable[[], HttpResponse]]) -> dict[str, Any]: """ json_dict: dict[str, Any] = {} - with transaction.atomic(): + with transaction.atomic(savepoint=False): for thunk in thunks: response = thunk() json_dict.update(orjson.loads(response.content)) diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 2f44c2be18..98e0d0bf64 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -468,6 +468,7 @@ def remove_subgroups_from_group_backend( @require_member_or_admin @typed_endpoint +@transaction.atomic(durable=True) def update_subgroups_of_user_group( request: HttpRequest, user_profile: UserProfile,