-
Notifications
You must be signed in to change notification settings - Fork 14
207255_Celery_tasks_refactor #5815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 23 commits
900aeb9
56c9a05
11e8c78
c44fad9
451ce51
7a1d54d
a9bece9
5671f78
223d4fe
b45077e
c27c0e6
6944208
1e58257
9309cf9
4e79125
5f4cfaa
2474787
14d575f
7c72cd9
aacf455
454e10a
47d1b19
4ae7c01
42f6f09
c8a6b6c
0387204
df14bbb
c6e0e47
8f3e8af
21941b6
841e69c
9846f2b
7961c78
a8aaa60
a1fff80
74f3d27
d0b438d
46194c9
b279496
a680852
b22044d
dd47c61
f363e1f
baf0e22
9c4d77e
d688683
f5a60ba
182a29c
0a210e8
4a77d7f
5b6277a
2451f0b
190b7fd
7e1c9d5
e1c2f2d
765b466
3cdf63f
5e3f9d0
4ca8d62
1ea4ebc
ce9f054
96a0cbc
50e6cbb
81843eb
25457a0
63e4965
f01f4c7
ac7816c
e58004d
9a77f46
d3003a1
22fb74a
56af457
c9e8fba
da9f028
cdc082d
9f3cddd
5678c3d
8a0cffd
5cc90ed
e46a236
67fe048
c6f760d
cb883f2
5b3877f
1a1f2ba
406edd3
8f59242
baa51b5
f188109
0e40658
b0ebaad
c6e03db
c2b1453
22c3382
ba2e28c
57072e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -347,7 +347,7 @@ def enroll_to_program(self, request: HttpRequest, pk: UUID) -> HttpResponse | No | |
| program_for_enroll = form.cleaned_data["program_for_enroll"] | ||
| households_ids = list(qs.distinct("unicef_id").values_list("id", flat=True)) | ||
| enroll_households_to_program_task.delay( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as before |
||
| households_ids=households_ids, | ||
| households_ids=[str(_id) for (_id) in households_ids], | ||
| program_for_enroll_id=str(program_for_enroll.id), | ||
| user_id=str(request.user.id), | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,25 +4,44 @@ | |
|
|
||
| from django.db.models import Q | ||
| from django.utils import timezone | ||
| from django_celery_boost.models import AsyncJobModel | ||
|
|
||
| from hope.apps.account.signals import _invalidate_user_permissions_cache | ||
| from hope.apps.core.celery import app | ||
| from hope.apps.utils.logs import log_start_and_end | ||
| from hope.apps.utils.sentry import sentry_tags | ||
| from hope.models import AsyncRetryJob | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def invalidate_permissions_cache_for_user_if_expired_role_action(job: AsyncRetryJob) -> bool: | ||
| # Invalidate permissions cache for users with roles that expired a day before | ||
| from hope.models import User | ||
|
|
||
| try: | ||
| day_ago = timezone.now() - datetime.timedelta(days=1) | ||
| users = User.objects.filter( | ||
| Q(role_assignments__expiry_date=day_ago.date()) | Q(partner__role_assignments__expiry_date=day_ago.date()) | ||
| ).distinct() | ||
| _invalidate_user_permissions_cache(users) | ||
| return True | ||
| except Exception: | ||
| logger.exception("Failed to invalidate permissions cache for users with expired roles") | ||
| raise | ||
|
|
||
|
|
||
| @app.task(bind=True, default_retry_delay=60, max_retries=3) | ||
| @log_start_and_end | ||
| @sentry_tags | ||
| def invalidate_permissions_cache_for_user_if_expired_role(self: Any) -> bool: | ||
| # Invalidate permissions cache for users with roles that expired a day before | ||
| from hope.models import User | ||
|
|
||
| day_ago = timezone.now() - datetime.timedelta(days=1) | ||
| users = User.objects.filter( | ||
| Q(role_assignments__expiry_date=day_ago.date()) | Q(partner__role_assignments__expiry_date=day_ago.date()) | ||
| ).distinct() | ||
| _invalidate_user_permissions_cache(users) | ||
| job = AsyncRetryJob.objects.create( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what paradigm this code is implementing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Celery Beat cannot schedule a plain Python function, so the periodic entrypoint still has to be a real Celery task. That Celery task does not do the work itself and only creates an |
||
| owner=None, | ||
| type=AsyncJobModel.JobType.JOB_TASK, | ||
| action="hope.apps.account.celery_tasks.invalidate_permissions_cache_for_user_if_expired_role_action", | ||
| config={}, | ||
| group_key="invalidate_permissions_cache_for_user_if_expired_role", | ||
| description="Invalidate permissions cache for users with expired roles", | ||
| ) | ||
| job.queue() | ||
| return True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| import logging | ||
|
|
||
| from django_celery_boost.models import AsyncJobModel | ||
|
|
||
| from hope.apps.accountability.services.export_survey_sample_service import ( | ||
| ExportSurveySampleService, | ||
| ) | ||
|
|
@@ -8,17 +10,17 @@ | |
| from hope.apps.core.utils import send_email_notification | ||
| from hope.apps.utils.logs import log_start_and_end | ||
| from hope.apps.utils.sentry import sentry_tags, set_sentry_business_area_tag | ||
| from hope.models import BusinessArea, Survey | ||
| from hope.models import AsyncJob, BusinessArea, Survey | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| @app.task | ||
| @log_start_and_end | ||
| @sentry_tags | ||
| def export_survey_sample_task(survey_id: str, user_id: str) -> None: | ||
| def export_survey_sample_task_action(job: AsyncJob) -> None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this code under transaction management?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a reason to put it into transaction, this code generates and stores sample file and notifies user |
||
| from hope.models import User | ||
|
|
||
| survey_id = job.config["survey_id"] | ||
| user_id = job.config["user_id"] | ||
|
|
||
| try: | ||
| survey = Survey.objects.get(id=survey_id) | ||
| user = User.objects.get(pk=user_id) | ||
|
|
@@ -30,43 +32,92 @@ def export_survey_sample_task(survey_id: str, user_id: str) -> None: | |
| if survey.business_area.enable_email_notification: | ||
| send_email_notification(service, user) | ||
|
|
||
| except Exception as e: | ||
| logger.warning(e) | ||
| except Exception as exc: | ||
| job.errors = { | ||
| "error": str(exc), | ||
| } | ||
| job.save(update_fields=["errors"]) | ||
| logger.exception("Failed to export survey sample") | ||
| raise | ||
|
|
||
|
|
||
| def send_survey_to_users_action(job: AsyncJob) -> None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, is this code executed inside a trasanction?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A transaction is not needed here because the task’s critical work is an external RapidPro call, which cannot be rolled back by the database, and the only local write is a small follow-up metadata update. |
||
| survey_id = job.config["survey_id"] | ||
| try: | ||
| survey = Survey.objects.get(id=survey_id) | ||
| set_sentry_business_area_tag(survey.business_area.name) | ||
| if survey.category == Survey.CATEGORY_MANUAL: | ||
| return | ||
| phone_numbers = survey.recipients.filter(head_of_household__phone_no_valid=True).values_list( | ||
| "head_of_household__phone_no", flat=True | ||
| ) | ||
| if survey.category == Survey.CATEGORY_SMS: | ||
| api = RapidProAPI(survey.business_area.slug, RapidProAPI.MODE_MESSAGE) | ||
| api.broadcast_message(phone_numbers, survey.body) | ||
| return | ||
| business_area = BusinessArea.objects.get(id=survey.business_area_id) | ||
| api = RapidProAPI(business_area.slug, RapidProAPI.MODE_VERIFICATION) | ||
|
|
||
| already_received = [ | ||
| phone_number | ||
| for successful_call in survey.successful_rapid_pro_calls | ||
| for phone_number in successful_call["urns"] | ||
| ] | ||
| phone_numbers = [phone_number for phone_number in phone_numbers if phone_number not in already_received] | ||
|
|
||
| successful_flows, error = api.start_flow(survey.flow_id, phone_numbers) | ||
| if error: | ||
| job.errors = { | ||
| "start_flow_error": str(error), | ||
| } | ||
| job.save(update_fields=["errors"]) | ||
|
|
||
| for successful_flow in successful_flows: | ||
| survey.successful_rapid_pro_calls.append( | ||
| { | ||
| "flow_uuid": successful_flow.response["uuid"], | ||
| "urns": list(map(str, successful_flow.urns)), | ||
| } | ||
| ) | ||
| survey.save() | ||
| except Exception as exc: | ||
| job.errors = { | ||
| "error": str(exc), | ||
| } | ||
| job.save(update_fields=["errors"]) | ||
| logger.exception("Failed to send survey to users") | ||
| raise | ||
|
|
||
|
|
||
| @app.task | ||
| @log_start_and_end | ||
| @sentry_tags | ||
| def export_survey_sample_task(survey_id: str, user_id: str) -> None: | ||
|
MarekBiczysko marked this conversation as resolved.
Outdated
|
||
| survey = Survey.objects.get(id=survey_id) | ||
| job = AsyncJob.objects.create( | ||
| owner_id=user_id, | ||
| program=survey.program, | ||
| type=AsyncJobModel.JobType.JOB_TASK, | ||
| action="hope.apps.accountability.celery_tasks.export_survey_sample_task_action", | ||
| config={"survey_id": str(survey_id), "user_id": str(user_id)}, | ||
| group_key=f"export_survey_sample_task:{survey_id}", | ||
| description=f"Export survey sample for survey {survey_id}", | ||
| ) | ||
| job.queue() | ||
|
|
||
|
|
||
| @app.task | ||
| @log_start_and_end | ||
| @sentry_tags | ||
| def send_survey_to_users(survey_id: str) -> None: | ||
| survey = Survey.objects.get(id=survey_id) | ||
| set_sentry_business_area_tag(survey.business_area.name) | ||
| if survey.category == Survey.CATEGORY_MANUAL: | ||
| return | ||
| phone_numbers = survey.recipients.filter(head_of_household__phone_no_valid=True).values_list( | ||
| "head_of_household__phone_no", flat=True | ||
| job = AsyncJob.objects.create( | ||
| owner=survey.created_by, | ||
| program=survey.program, | ||
| type=AsyncJobModel.JobType.JOB_TASK, | ||
| action="hope.apps.accountability.celery_tasks.send_survey_to_users_action", | ||
| config={"survey_id": str(survey_id)}, | ||
| group_key=f"send_survey_to_users:{survey_id}", | ||
| description=f"Send survey to users for survey {survey_id}", | ||
| ) | ||
| if survey.category == Survey.CATEGORY_SMS: | ||
| api = RapidProAPI(survey.business_area.slug, RapidProAPI.MODE_MESSAGE) | ||
| api.broadcast_message(phone_numbers, survey.body) | ||
| return | ||
| business_area = BusinessArea.objects.get(id=survey.business_area_id) | ||
| api = RapidProAPI(business_area.slug, RapidProAPI.MODE_VERIFICATION) | ||
|
|
||
| already_received = [ | ||
| phone_number | ||
| for successful_call in survey.successful_rapid_pro_calls | ||
| for phone_number in successful_call["urns"] | ||
| ] | ||
| phone_numbers = [phone_number for phone_number in phone_numbers if phone_number not in already_received] | ||
|
|
||
| successful_flows, error = api.start_flow(survey.flow_id, phone_numbers) | ||
|
|
||
| for successful_flow in successful_flows: | ||
| survey.successful_rapid_pro_calls.append( | ||
| { | ||
| "flow_uuid": successful_flow.response["uuid"], | ||
| "urns": list(map(str, successful_flow.urns)), | ||
| } | ||
| ) | ||
| survey.save() | ||
| job.queue() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we do no pass the
unicef_idto the task? we could save a query and a cast iteration