remote_server_post_analytics: Add durable=True to outermost transaction.

This commit adds 'durable=True' to the outermost transaction
in 'remote_server_post_analytics'.

It also adds 'savepoint=False' to inner transaction.atomic
decorator to avoid creating savepoint.

This is as a part of our plan to explicitly mark all the
transaction.atomic decorators with either 'savepoint=False' or
'durable=True' as required.

* 'savepoint=True' is used in special cases.
This commit is contained in:
Prakhar Pratyush 2024-11-01 19:53:30 +05:30 committed by Tim Abbott
parent 3d597bb9b0
commit c4f74f470d
3 changed files with 7 additions and 9 deletions

View File

@ -2197,7 +2197,7 @@ class BillingSession(ABC):
# event_time should roughly be timezone_now(). Not designed to handle
# event_times in the past or future
@transaction.atomic
@transaction.atomic(savepoint=False)
def make_end_of_cycle_updates_if_needed(
self, plan: CustomerPlan, event_time: datetime
) -> tuple[CustomerPlan | None, LicenseLedger | None]:

View File

@ -14,7 +14,6 @@ import orjson
import responses
import time_machine
from django.conf import settings
from django.db import transaction
from django.db.models import F, Q
from django.http.response import ResponseHeaders
from django.test import override_settings
@ -2145,9 +2144,7 @@ class AnalyticsBouncerTest(BouncerTestCase):
plan_type=RemoteRealm.PLAN_TYPE_SELF_MANAGED,
)
with transaction.atomic(), self.assertLogs("zulip.analytics", level="WARNING") as m:
# The usual atomic() wrapper to avoid IntegrityError breaking the test's
# transaction.
with self.assertLogs("zulip.analytics", level="WARNING") as m:
send_server_data_to_push_bouncer()
self.assertEqual(m.output, ["WARNING:zulip.analytics:Duplicate registration detected."])
@ -2705,6 +2702,7 @@ class AnalyticsBouncerTest(BouncerTestCase):
# Now we want to test the other side of this - bouncer's handling
# of a deleted realm.
with (
self.captureOnCommitCallbacks(execute=True),
self.assertLogs(logger, level="WARNING") as analytics_logger,
mock.patch(
"corporate.lib.stripe.RemoteRealmBillingSession.on_paid_plan", return_value=True

View File

@ -50,7 +50,7 @@ from zerver.lib.push_notifications import (
send_apple_push_notification,
send_test_push_notification_directly_to_devices,
)
from zerver.lib.queue import queue_json_publish
from zerver.lib.queue import queue_event_on_commit
from zerver.lib.remote_server import (
InstallationCountDataForAnalytics,
RealmAuditLogDataForAnalytics,
@ -1012,7 +1012,7 @@ def update_remote_realm_data_for_server(
}
for context in new_locally_deleted_remote_realms_on_paid_plan_contexts:
email_dict["context"] = context
queue_json_publish("email_senders", email_dict)
queue_event_on_commit("email_senders", email_dict)
def get_human_user_realm_uuids(
@ -1156,7 +1156,7 @@ def handle_customer_migration_from_server_to_realm(
@typed_endpoint
@transaction.atomic
@transaction.atomic(durable=True)
def remote_server_post_analytics(
request: HttpRequest,
server: RemoteZulipServer,
@ -1267,7 +1267,7 @@ def remote_server_post_analytics(
# 'last_audit_log_update' needs to be an atomic operation.
# This helps to rely on 'last_audit_log_update' to assume
# RemoteRealmAuditLog and LicenseLedger are up-to-date.
with transaction.atomic():
with transaction.atomic(savepoint=False):
# Important: Do not return early if we receive 0 rows; we must
# updated last_audit_log_update even if there are no new rows,
# to help identify server whose ability to connect to this