From 24290a1cf14c96a2551651760c9a3a0e82aa6fec Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Wed, 13 Mar 2024 14:56:06 -0400 Subject: [PATCH] feat: chunked tasks in remind_all view ENT-8635 | The remind_all view now only reads the data necessary from the DB. This makes it less likely that the request to remind-all times out. Also, it now invokes more than 1 big celery task - it chunks the emails to send into smaller pieces, and invokes one task per chunk. This makes it less likely that the celery task will fail or retry too big a chunk of work (i.e. every assigned licenses in the plan). --- .../apps/api/v1/tests/test_views.py | 27 ++++++++++++++----- license_manager/apps/api/v1/views.py | 23 +++++++++------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/license_manager/apps/api/v1/tests/test_views.py b/license_manager/apps/api/v1/tests/test_views.py index a3ccff8b..961b6763 100644 --- a/license_manager/apps/api/v1/tests/test_views.py +++ b/license_manager/apps/api/v1/tests/test_views.py @@ -1986,15 +1986,30 @@ def test_remind_all(self, mock_send_reminder_emails_task): pending_licenses = LicenseFactory.create_batch(3, status=constants.ASSIGNED) self.subscription_plan.licenses.set(unassigned_licenses + pending_licenses) - response = self.api_client.post(self.remind_all_url, {'greeting': self.greeting, 'closing': self.closing}) + with mock.patch('license_manager.apps.subscriptions.constants.LICENSE_BULK_OPERATION_BATCH_SIZE', new=2): + response = self.api_client.post(self.remind_all_url, {'greeting': self.greeting, 'closing': self.closing}) + assert response.status_code == status.HTTP_204_NO_CONTENT # Verify emails sent to only the pending licenses - mock_send_reminder_emails_task.assert_called_with( - {'greeting': self.greeting, 'closing': self.closing}, - sorted([license.user_email for license in pending_licenses]), - str(self.subscription_plan.uuid), - ) + mock_send_reminder_emails_task.assert_has_calls([ + mock.call( + {'greeting': self.greeting, 'closing': self.closing}, + mock.ANY, + str(self.subscription_plan.uuid), + ), + mock.call( + {'greeting': self.greeting, 'closing': self.closing}, + mock.ANY, + str(self.subscription_plan.uuid), + ), + ]) + pending_emails = [license.user_email for license in pending_licenses] + actually_called_emails = [] + for call in mock_send_reminder_emails_task.call_args_list: + actually_called_emails.extend(call.args[1]) + + assert set(pending_emails) == set(actually_called_emails) @ddt.data(True, False) def test_license_overview(self, ignore_null_emails): diff --git a/license_manager/apps/api/v1/views.py b/license_manager/apps/api/v1/views.py index df765862..747c17e5 100644 --- a/license_manager/apps/api/v1/views.py +++ b/license_manager/apps/api/v1/views.py @@ -997,17 +997,22 @@ def remind_all(self, request, subscription_uuid=None): self._validate_data(request.data) subscription_plan = self._get_subscription_plan() - pending_licenses = subscription_plan.licenses.filter(status=constants.ASSIGNED) - if not pending_licenses: + assigned_license_emails = list( + subscription_plan.licenses.filter( + status=constants.ASSIGNED, + ).values_list('user_email', flat=True) + ) + if not assigned_license_emails: return Response('Could not find any licenses pending activation', status=status.HTTP_404_NOT_FOUND) - pending_user_emails = [license.user_email for license in pending_licenses] - # Send activation reminder email to all pending users - send_reminder_email_task.delay( - utils.get_custom_text(request.data), - sorted(pending_user_emails), - subscription_uuid, - ) + # Send reminder emails in batches. + chunked_lists = chunks(assigned_license_emails, constants.LICENSE_BULK_OPERATION_BATCH_SIZE) + for assigned_license_email_chunk in chunked_lists: + send_reminder_email_task.delay( + utils.get_custom_text(request.data), + sorted(assigned_license_email_chunk), + subscription_uuid, + ) return Response(status=status.HTTP_204_NO_CONTENT)