From 3cb0c8e9956772a10afebf0d082de4d7b27915c0 Mon Sep 17 00:00:00 2001 From: Helder Souza <42891390+helllllllder@users.noreply.github.com> Date: Mon, 14 Oct 2024 14:20:07 -0300 Subject: [PATCH] Revert "feature: retry when closing room (#399)" This reverts commit 31d81e06db5334ca5ee9f0b668447a72bab71dab. --- chats/apps/api/v1/rooms/viewsets.py | 36 +++++-------- chats/apps/rooms/models.py | 28 ++++------ chats/apps/rooms/tests/test_models.py | 76 --------------------------- chats/settings.py | 1 - 4 files changed, 22 insertions(+), 119 deletions(-) diff --git a/chats/apps/api/v1/rooms/viewsets.py b/chats/apps/api/v1/rooms/viewsets.py index f21579e7..36518b3c 100644 --- a/chats/apps/api/v1/rooms/viewsets.py +++ b/chats/apps/api/v1/rooms/viewsets.py @@ -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 @@ -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() diff --git a/chats/apps/rooms/models.py b/chats/apps/rooms/models.py index 63b87e7f..cf40f191 100644 --- a/chats/apps/rooms/models.py +++ b/chats/apps/rooms/models.py @@ -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 @@ -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: diff --git a/chats/apps/rooms/tests/test_models.py b/chats/apps/rooms/tests/test_models.py index 38e554ad..9ba824c2 100644 --- a/chats/apps/rooms/tests/test_models.py +++ b/chats/apps/rooms/tests/test_models.py @@ -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 @@ -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 diff --git a/chats/settings.py b/chats/settings.py index 1e8b5d20..936afc89 100644 --- a/chats/settings.py +++ b/chats/settings.py @@ -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)