From 83f6e560b76ab33a8724e42ae50e3f8ca0aa8a9b Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Wed, 15 Feb 2023 18:20:50 +0000 Subject: [PATCH] fix: Add code_owner decorator to remaining Celery tasks (#31762) This will ensure that errors raised by these tasks will alert the right team. `send_course_enrollment_email` is the one I set out to fix, but I discovered a few others. I located tasks that were missing decorators by running the following search and visually inspecting the results, although semgrep might be able to do better: ``` ack '^@.*task|^@set_code_owner_attribute' cms lms common openedx xmodule --ignore-dir=tests --python ``` Also, add more detailed explanation of why a couple of tasks can't use the decorator. This should only be an issue on tasks inheriting from UserTaskMixin, which in practice is just CourseExportTask and CourseImportTask (and the apparently unused EnrollmentReadTask and EnrollmentWriteTask), via UserTask. --- cms/djangoapps/contentstore/tasks.py | 11 ++++++----- cms/djangoapps/export_course_metadata/tasks.py | 2 ++ common/djangoapps/student/tasks.py | 2 ++ lms/djangoapps/ccx/tasks.py | 2 ++ lms/djangoapps/lti_provider/tasks.py | 3 +++ openedx/core/djangoapps/content_libraries/tasks.py | 3 ++- openedx/features/survey_report/tasks.py | 3 +++ 7 files changed, 20 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index d08ac886e2..4374eacdb9 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -296,8 +296,8 @@ class CourseExportTask(UserTask): # pylint: disable=abstract-method @shared_task(base=CourseExportTask, bind=True) -# Note: The decorator @set_code_owner_attribute could not be used because -# the implementation of this task breaks with any additional decorators. +# Note: The decorator @set_code_owner_attribute cannot be used here because the UserTaskMixin +# does stack inspection and can't handle additional decorators. def export_olx(self, user_id, course_key_string, language): """ Export a course or library to an OLX .tar.gz archive and prepare it for download. @@ -431,15 +431,16 @@ class CourseImportTask(UserTask): # pylint: disable=abstract-method @shared_task(base=CourseImportTask, bind=True) -# Note: The decorator @set_code_owner_attribute could not be used because # lint-amnesty, pylint: disable=too-many-statements -# the implementation of this task breaks with any additional decorators. +# Note: The decorator @set_code_owner_attribute cannot be used here because the UserTaskMixin +# does stack inspection and can't handle additional decorators. +# lint-amnesty, pylint: disable=too-many-statements def import_olx(self, user_id, course_key_string, archive_path, archive_name, language): """ Import a course or library from a provided OLX .tar.gz archive. """ + set_code_owner_attribute_from_module(__name__) current_step = 'Unpacking' courselike_key = CourseKey.from_string(course_key_string) - set_code_owner_attribute_from_module(__name__) set_custom_attributes_for_course_key(courselike_key) log_prefix = f'Course import {courselike_key}' self.status.set_state(current_step) diff --git a/cms/djangoapps/export_course_metadata/tasks.py b/cms/djangoapps/export_course_metadata/tasks.py index 2ef37dda9e..2fb33cd0e8 100644 --- a/cms/djangoapps/export_course_metadata/tasks.py +++ b/cms/djangoapps/export_course_metadata/tasks.py @@ -6,6 +6,7 @@ import json from celery import shared_task from django.core.files.base import ContentFile +from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.schedules.content_highlights import get_all_course_highlights @@ -13,6 +14,7 @@ from .storage import course_metadata_export_storage @shared_task(bind=True) +@set_code_owner_attribute def export_course_metadata_task(self, course_key_string): # pylint: disable=unused-argument """ Export course metadata diff --git a/common/djangoapps/student/tasks.py b/common/djangoapps/student/tasks.py index 99009e0156..3865afd1b2 100644 --- a/common/djangoapps/student/tasks.py +++ b/common/djangoapps/student/tasks.py @@ -5,6 +5,7 @@ import logging from celery import shared_task from django.conf import settings from django.contrib.auth import get_user_model +from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey from common.djangoapps.track import segment @@ -30,6 +31,7 @@ COUNTDOWN = 60 @shared_task(bind=True, ignore_result=True) +@set_code_owner_attribute def send_course_enrollment_email( self, user_id, course_id, course_title, short_description, course_ended, pacing_type, track_mode ): diff --git a/lms/djangoapps/ccx/tasks.py b/lms/djangoapps/ccx/tasks.py index db53976a33..4328e561d4 100644 --- a/lms/djangoapps/ccx/tasks.py +++ b/lms/djangoapps/ccx/tasks.py @@ -7,6 +7,7 @@ import logging from ccx_keys.locator import CCXLocator from django.dispatch import receiver +from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import CourseLocator @@ -27,6 +28,7 @@ def course_published_handler(sender, course_key, **kwargs): # pylint: disable=u @CELERY_APP.task +@set_code_owner_attribute def send_ccx_course_published(course_key): """ Find all CCX derived from this course, and send course published event for them. diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 55430b5edf..343e56926d 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -6,6 +6,7 @@ Asynchronous tasks for the LTI provider app. import logging from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user +from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey import lms.djangoapps.lti_provider.outcomes as outcomes @@ -18,6 +19,7 @@ log = logging.getLogger(__name__) @CELERY_APP.task(name='lms.djangoapps.lti_provider.tasks.send_composite_outcome') +@set_code_owner_attribute def send_composite_outcome(user_id, course_id, assignment_id, version): """ Calculate and transmit the score for a composite module (such as a @@ -67,6 +69,7 @@ def send_composite_outcome(user_id, course_id, assignment_id, version): @CELERY_APP.task +@set_code_owner_attribute def send_leaf_outcome(assignment_id, points_earned, points_possible): """ Calculate and transmit the score for a single problem. This method assumes diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 1cb097fad5..195de23fde 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -7,7 +7,7 @@ import logging from celery import shared_task from celery_utils.logged_task import LoggedTask - +from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey from . import api @@ -18,6 +18,7 @@ logger = logging.getLogger(__name__) @shared_task(base=LoggedTask) +@set_code_owner_attribute def import_blocks_from_course(import_task_id, course_key_str): """ A Celery task to import blocks from a course through modulestore. diff --git a/openedx/features/survey_report/tasks.py b/openedx/features/survey_report/tasks.py index 9f02b9795c..a9065fb325 100644 --- a/openedx/features/survey_report/tasks.py +++ b/openedx/features/survey_report/tasks.py @@ -6,12 +6,15 @@ Tasks for Survey Report. import logging from celery import shared_task +from edx_django_utils.monitoring import set_code_owner_attribute + from .api import generate_report log = logging.getLogger('edx.celery.task') @shared_task(name='openedx.features.survey_report.tasks.generate_survey_report') +@set_code_owner_attribute def generate_survey_report(): """ Tasks to generate a new survey report with non-sensitive data.