Skip to content

Commit

Permalink
Revert "feature: retry when closing room (#399)"
Browse files Browse the repository at this point in the history
This reverts commit 31d81e0.
  • Loading branch information
helllllllder authored Oct 14, 2024
1 parent 31d81e0 commit 3cb0c8e
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 119 deletions.
36 changes: 12 additions & 24 deletions chats/apps/api/v1/rooms/viewsets.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from datetime import timedelta

from django.conf import settings
from django.db import DatabaseError, transaction
from django.db.models import BooleanField, Case, Count, Max, OuterRef, Q, Subquery, When
from django.utils import timezone
from django_filters.rest_framework import DjangoFilterBackend
Expand Down Expand Up @@ -149,29 +148,18 @@ def close(
"""
# Add send room notification to the channels group
instance = self.get_object()
for attempt in range(settings.MAX_RETRIES):
try:
with transaction.atomic():
tags = request.data.get("tags", None)
instance.close(tags, "agent")
serialized_data = RoomSerializer(instance=instance)
instance.notify_queue("close", callback=True)
instance.notify_user("close")

if not settings.ACTIVATE_CALC_METRICS:
return Response(serialized_data.data, status=status.HTTP_200_OK)

close_room(str(instance.pk))
return Response(serialized_data.data, status=status.HTTP_200_OK)

except DatabaseError as error:
if attempt < settings.MAX_RETRIES - 1:
continue
else:
return Response(
{"error": f"Transaction failed after retries: {str(error)}"},
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)

tags = request.data.get("tags", None)
instance.close(tags, "agent")
serialized_data = RoomSerializer(instance=instance)
instance.notify_queue("close", callback=True)
instance.notify_user("close")

if not settings.ACTIVATE_CALC_METRICS:
return Response(serialized_data.data, status=status.HTTP_200_OK)

close_room(str(instance.pk))
return Response(serialized_data.data, status=status.HTTP_200_OK)

def perform_create(self, serializer):
serializer.save()
Expand Down
28 changes: 10 additions & 18 deletions chats/apps/rooms/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from django.db import models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from requests.exceptions import RequestException
from rest_framework.exceptions import ValidationError

from chats.core.models import BaseModel
Expand Down Expand Up @@ -180,23 +179,16 @@ def request_callback(self, room_data: dict):
if self.callback_url is None:
return None

try:
response = requests.post(
self.callback_url,
data=json.dumps(
{"type": "room.update", "content": room_data},
sort_keys=True,
indent=1,
cls=DjangoJSONEncoder,
),
headers={"content-type": "application/json"},
)
response.raise_for_status()

except RequestException as error:
raise RuntimeError(
f"Failed to send callback to {self.callback_url}: {str(error)}"
)
requests.post(
self.callback_url,
data=json.dumps(
{"type": "room.update", "content": room_data},
sort_keys=True,
indent=1,
cls=DjangoJSONEncoder,
),
headers={"content-type": "application/json"},
)

def base_notification(self, content, action):
if self.user:
Expand Down
76 changes: 0 additions & 76 deletions chats/apps/rooms/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
from unittest.mock import patch

from django.conf import settings
from django.db import IntegrityError
from django.db.utils import DatabaseError
from django.utils import timezone
from rest_framework import status
from rest_framework.test import APITestCase

from chats.apps.rooms.models import Room
Expand All @@ -23,73 +17,3 @@ def test_unique_contact_queue_is_activetrue_room_constraint(self):
'duplicate key value violates unique constraint "unique_contact_queue_is_activetrue_room"'
in str(context.exception)
)


class RetryCloseRoomTests(APITestCase):
fixtures = ["chats/fixtures/fixture_sector.json"]

def setUp(self) -> None:
self.room = Room.objects.get(uuid="090da6d1-959e-4dea-994a-41bf0d38ba26")
self.agent_token = "8c60c164-32bc-11ed-a261-0242ac120002"

def _close_room(self, token: str, data: dict):
url = f"/v1/room/{self.room.uuid}/close/"
client = self.client
client.credentials(HTTP_AUTHORIZATION=f"Token {token}")
return client.patch(url, data=data, format="json")

def test_atomic_transaction_rollback(self):
"""
Ensure that the database is rolled back if an
exception occurs during the transaction and that
no changes are committed.
"""
instance = self.room
with patch("chats.apps.rooms.models.Room.close", side_effect=DatabaseError):
response = self._close_room(self.agent_token, data={})

self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR)

instance.refresh_from_db()
self.assertTrue(instance.is_active)
self.assertIsNone(instance.ended_at)

def test_atomic_transaction_retries_on_database_error(self):
"""
Verify that the transaction is retried up to
MAX_RETRIES times when a DatabaseError occurs.
"""
with patch(
"chats.apps.rooms.models.Room.close", side_effect=DatabaseError
) as mock_close:
response = self._close_room(self.agent_token, data={})

assert mock_close.call_count == settings.MAX_RETRIES
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR

@patch("chats.apps.rooms.models.Room.request_callback")
@patch("chats.apps.rooms.models.Room.close")
def test_atomic_transaction_succeeds_after_retry(
self, mock_close, mock_request_callback
):
"""
Simulate a DatabaseError on the first attempt,
but allow the transaction to succeed on subsequent retries.
"""
instance = self.room
mock_request_callback.return_value = None

instance.ended_at = timezone.now()
instance.is_active = False
instance.save()

mock_close.side_effect = [DatabaseError, None]

response = self._close_room(self.agent_token, data={})

assert response.status_code == status.HTTP_200_OK

instance.refresh_from_db()
assert mock_close.call_count == 2
assert not instance.is_active
assert instance.ended_at is not None
1 change: 0 additions & 1 deletion chats/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,3 @@

WS_MESSAGE_RETRIES = env.int("WS_MESSAGE_RETRIES", default=5)
WEBSOCKET_RETRY_SLEEP = env.int("WEBSOCKET_RETRY_SLEEP", default=0.5)
MAX_RETRIES = env.int("WS_MESSAGE_RETRIES", default=3)

0 comments on commit 3cb0c8e

Please sign in to comment.