From 832d0448294d12aa3a4003e1a573ab1451538d4f Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 9 May 2024 13:16:46 -0300 Subject: [PATCH] Update out of office task to not retry on HttpError (#4328) Do not keep retrying on HttpErrors (eg. 403). Also, we will re-queued periodically later. --- engine/apps/google/client.py | 35 ++++++++++++------- engine/apps/google/tasks.py | 10 ++++-- ..._out_of_office_calendar_events_for_user.py | 23 ++++++++++++ engine/pyproject.toml | 1 + 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/engine/apps/google/client.py b/engine/apps/google/client.py index 906194ca..a0544ab3 100644 --- a/engine/apps/google/client.py +++ b/engine/apps/google/client.py @@ -5,6 +5,7 @@ import typing from django.conf import settings from google.oauth2.credentials import Credentials from googleapiclient.discovery import build +from googleapiclient.errors import HttpError from apps.google import constants, utils from apps.google.types import GoogleCalendarEvent as GoogleCalendarEventType @@ -23,6 +24,11 @@ class GoogleCalendarEvent: self.end_time_utc = self._end_time.astimezone(datetime.timezone.utc) +class GoogleCalendarHTTPError(Exception): + def __init__(self, http_error) -> None: + self.error = http_error + + class GoogleCalendarAPIClient: MAX_NUMBER_OF_CALENDAR_EVENTS_TO_FETCH = 250 """ @@ -68,17 +74,22 @@ class GoogleCalendarAPIClient: now + datetime.timedelta(days=constants.DAYS_IN_FUTURE_TO_CONSIDER_OUT_OF_OFFICE_EVENTS) ) - events_result = ( - self.service.events() - .list( - calendarId=self.CALENDAR_ID, - timeMin=time_min, - timeMax=time_max, - maxResults=self.MAX_NUMBER_OF_CALENDAR_EVENTS_TO_FETCH, - singleEvents=True, - orderBy="startTime", - eventTypes="outOfOffice", + try: + events_result = ( + self.service.events() + .list( + calendarId=self.CALENDAR_ID, + timeMin=time_min, + timeMax=time_max, + maxResults=self.MAX_NUMBER_OF_CALENDAR_EVENTS_TO_FETCH, + singleEvents=True, + orderBy="startTime", + eventTypes="outOfOffice", + ) + .execute() ) - .execute() - ) + except HttpError as e: + logger.error(f"GoogleCalendarAPIClient - Error fetching out of office events: {e}") + raise GoogleCalendarHTTPError(e) + return [GoogleCalendarEvent(event) for event in events_result.get("items", [])] diff --git a/engine/apps/google/tasks.py b/engine/apps/google/tasks.py index b1b601d4..77b3d967 100644 --- a/engine/apps/google/tasks.py +++ b/engine/apps/google/tasks.py @@ -3,7 +3,7 @@ import logging from celery.utils.log import get_task_logger from apps.google import constants -from apps.google.client import GoogleCalendarAPIClient +from apps.google.client import GoogleCalendarAPIClient, GoogleCalendarHTTPError from apps.google.models import GoogleOAuth2User from apps.schedules.models import OnCallSchedule, ShiftSwapRequest from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -31,7 +31,13 @@ def sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk: int) -> N if oncall_schedules_to_consider_for_shift_swaps: users_schedules = users_schedules.filter(public_primary_key__in=oncall_schedules_to_consider_for_shift_swaps) - for out_of_office_event in google_api_client.fetch_out_of_office_events(): + try: + out_of_office_events = google_api_client.fetch_out_of_office_events() + except GoogleCalendarHTTPError: + logger.info(f"Failed to fetch out of office events for user {user_id}") + return + + for out_of_office_event in out_of_office_events: raw_event = out_of_office_event.raw_event event_title = raw_event["summary"] diff --git a/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py b/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py index d930af13..69f1020b 100644 --- a/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py +++ b/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py @@ -3,6 +3,7 @@ from unittest.mock import patch import pytest from django.utils import timezone +from googleapiclient.errors import HttpError from apps.google import constants, tasks from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb, ShiftSwapRequest @@ -140,6 +141,28 @@ def test_setup( return _test_setup +class MockResponse: + def __init__(self, reason=None, status=200) -> None: + self.reason = reason or "" + self.status = status + + +@patch("apps.google.client.build") +@pytest.mark.django_db +def test_sync_out_of_office_calendar_events_for_user_httperror(mock_google_api_client_build, test_setup): + mock_response = MockResponse(reason="forbidden", status=403) + mock_google_api_client_build.return_value.events.return_value.list.return_value.execute.side_effect = HttpError( + resp=mock_response, content=b"error" + ) + + google_oauth2_user, schedule = test_setup([]) + user = google_oauth2_user.user + + tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk) + + assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 0 + + @patch("apps.google.client.build") @pytest.mark.django_db def test_sync_out_of_office_calendar_events_for_user_no_ooo_events(mock_google_api_client_build, test_setup): diff --git a/engine/pyproject.toml b/engine/pyproject.toml index f2ea3e21..a1bab7d7 100644 --- a/engine/pyproject.toml +++ b/engine/pyproject.toml @@ -58,6 +58,7 @@ module = [ "fcm_django.*", "firebase_admin.*", "googleapiclient.discovery.*", + "googleapiclient.errors.*", "google.oauth2.credentials.*", "httpretty.*", "humanize.*",