From 549f4b661e93ad4d9b4c510c3065deb7886d3081 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri <nasthagiri@edx.org> Date: Mon, 4 Dec 2017 00:51:10 -0500 Subject: [PATCH] Schedules: Add celery task logging --- lms/djangoapps/certificates/tasks.py | 12 ++--------- lms/djangoapps/grades/tasks.py | 18 +++++------------ .../content/course_overviews/tasks.py | 12 ++--------- .../commands/tests/send_email_base.py | 20 +++++++++---------- .../commands/tests/test_send_course_update.py | 2 +- .../tests/test_send_upgrade_reminder.py | 6 +++--- .../management/commands/tests/upsell_base.py | 2 +- openedx/core/djangoapps/schedules/tasks.py | 20 ++++++++++++------- requirements/edx/base.txt | 2 +- requirements/edx/github.txt | 1 + 10 files changed, 39 insertions(+), 56 deletions(-) diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 7c5988d99a6..2bb2c7fc166 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -1,8 +1,7 @@ from celery import task from logging import getLogger -from celery_utils.logged_task import LoggedTask -from celery_utils.persist_on_failure import PersistOnFailureTask +from celery_utils.persist_on_failure import LoggedPersistOnFailureTask from django.contrib.auth.models import User from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from opaque_keys.edx.keys import CourseKey @@ -12,14 +11,7 @@ from .api import generate_user_certificates logger = getLogger(__name__) -class _BaseCertificateTask(PersistOnFailureTask, LoggedTask): # pylint: disable=abstract-method - """ - Include persistence features, as well as logging of task invocation. - """ - abstract = True - - -@task(base=_BaseCertificateTask, bind=True, default_retry_delay=30, max_retries=2) +@task(base=LoggedPersistOnFailureTask, bind=True, default_retry_delay=30, max_retries=2) def generate_certificate(self, **kwargs): """ Generates a certificate for a single user. diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index a800d026cf6..2839834bbac 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -6,8 +6,7 @@ from logging import getLogger import six from celery import task -from celery_utils.logged_task import LoggedTask -from celery_utils.persist_on_failure import PersistOnFailureTask +from celery_utils.persist_on_failure import LoggedPersistOnFailureTask from courseware.model_data import get_score from django.conf import settings from django.contrib.auth.models import User @@ -46,14 +45,7 @@ RETRY_DELAY_SECONDS = 30 SUBSECTION_GRADE_TIMEOUT_SECONDS = 300 -class _BaseTask(PersistOnFailureTask, LoggedTask): # pylint: disable=abstract-method - """ - Include persistence features, as well as logging of task invocation. - """ - abstract = True - - -@task(base=_BaseTask, routing_key=settings.POLICY_CHANGE_GRADES_ROUTING_KEY) +@task(base=LoggedPersistOnFailureTask, routing_key=settings.POLICY_CHANGE_GRADES_ROUTING_KEY) def compute_all_grades_for_course(**kwargs): """ Compute grades for all students in the specified course. @@ -77,7 +69,7 @@ def compute_all_grades_for_course(**kwargs): @task( bind=True, - base=_BaseTask, + base=LoggedPersistOnFailureTask, default_retry_delay=RETRY_DELAY_SECONDS, max_retries=1, time_limit=COURSE_GRADE_TIMEOUT_SECONDS @@ -105,7 +97,7 @@ def compute_grades_for_course_v2(self, **kwargs): raise self.retry(kwargs=kwargs, exc=exc) -@task(base=_BaseTask) +@task(base=LoggedPersistOnFailureTask) def compute_grades_for_course(course_key, offset, batch_size, **kwargs): # pylint: disable=unused-argument """ Compute and save grades for a set of students in the specified course. @@ -124,7 +116,7 @@ def compute_grades_for_course(course_key, offset, batch_size, **kwargs): # pyli @task( bind=True, - base=_BaseTask, + base=LoggedPersistOnFailureTask, time_limit=SUBSECTION_GRADE_TIMEOUT_SECONDS, max_retries=2, default_retry_delay=RETRY_DELAY_SECONDS, diff --git a/openedx/core/djangoapps/content/course_overviews/tasks.py b/openedx/core/djangoapps/content/course_overviews/tasks.py index 573393897be..d813670600b 100644 --- a/openedx/core/djangoapps/content/course_overviews/tasks.py +++ b/openedx/core/djangoapps/content/course_overviews/tasks.py @@ -1,8 +1,7 @@ import logging from celery import task -from celery_utils.logged_task import LoggedTask -from celery_utils.persist_on_failure import PersistOnFailureTask +from celery_utils.persist_on_failure import LoggedPersistOnFailureTask from django.conf import settings from opaque_keys.edx.keys import CourseKey @@ -25,13 +24,6 @@ def chunks(sequence, chunk_size): return (sequence[index: index + chunk_size] for index in xrange(0, len(sequence), chunk_size)) -class _BaseTask(PersistOnFailureTask, LoggedTask): # pylint: disable=abstract-method - """ - Include persistence features, as well as logging of task invocation. - """ - abstract = True - - def _task_options(routing_key): task_options = {} if getattr(settings, 'HIGH_MEM_QUEUE', None): @@ -64,7 +56,7 @@ def enqueue_async_course_overview_update_tasks( ) -@task(base=_BaseTask) +@task(base=LoggedPersistOnFailureTask) def async_course_overview_update(*args, **kwargs): course_keys = [CourseKey.from_string(arg) for arg in args] CourseOverview.update_select_courses(course_keys, force_update=kwargs['force_update']) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index 781b43c2768..b682f149dda 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -200,7 +200,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): is_first_match = False with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES): - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=target_day_str, day_offset=offset, bin_num=b, )) @@ -220,15 +220,15 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): course_id=CourseKey.from_string('edX/toy/Not_2012_Fall'), user=UserFactory.create(), ) - schedule = self._schedule_factory(enrollment=enrollment) + self._schedule_factory(enrollment=enrollment) with patch.object(self.task, 'async_send_task') as mock_schedule_send: - for b in range(self.task.num_bins): - self.task.apply(kwargs=dict( + for bin_num in range(self.task().num_bins): + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, - bin_num=b, + bin_num=bin_num, )) # There is no database constraint that enforces that enrollment.course_id points @@ -308,7 +308,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): ) with patch.object(self.task, 'async_send_task') as mock_schedule_send: - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=this_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=0 )) @@ -328,7 +328,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): ) with patch.object(self.task, 'async_send_task') as mock_schedule_send: - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=0, )) @@ -354,7 +354,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): expected_query_count = NUM_QUERIES_FIRST_MATCH + additional_course_queries with self.assertNumQueries(expected_query_count, table_blacklist=WAFFLE_TABLES): with patch.object(self.task, 'async_send_task') as mock_schedule_send: - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(user), )) @@ -402,7 +402,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): num_expected_queries += 1 with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES): - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(user), )) @@ -436,7 +436,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): schedule = self._schedule_factory(**kwargs) with patch.object(tasks, 'ace') as mock_ace: - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(schedule.enrollment.user), )) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py index 973f139741e..1b4b0a4efc1 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py @@ -87,7 +87,7 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestMixin, enrollment.schedule.save() with patch.object(tasks, 'ace') as mock_ace: - self.task.apply(kwargs=dict( # pylint: disable=no-value-for-parameter + self.task().apply(kwargs=dict( # pylint: disable=no-value-for-parameter site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py index bd37d758951..97439c47d8e 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py @@ -52,7 +52,7 @@ class TestUpgradeReminder(ScheduleSendEmailTestMixin, CacheIsolationTestCase): enrollment__mode=CourseMode.VERIFIED if is_verified else CourseMode.AUDIT, ) - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(schedule.enrollment.user), )) @@ -76,7 +76,7 @@ class TestUpgradeReminder(ScheduleSendEmailTestMixin, CacheIsolationTestCase): with patch.object(self.task, 'async_send_task') as mock_schedule_send: mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args[1]) - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(user), )) @@ -95,7 +95,7 @@ class TestUpgradeReminder(ScheduleSendEmailTestMixin, CacheIsolationTestCase): schedule = self._schedule_factory() schedule.enrollment.course.modes.filter(mode_slug=CourseMode.VERIFIED).delete() - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(schedule.enrollment.user), )) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py index 229227142aa..7b38c522854 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py @@ -40,7 +40,7 @@ class ScheduleUpsellTestMixin(object): sent_messages = [] with patch.object(self.task, 'async_send_task') as mock_schedule_send: mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args[1]) - self.task.apply(kwargs=dict( + self.task().apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, bin_num=self._calculate_bin_for_user(schedule.enrollment.user), )) diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 2ac8db78cb6..c368773c392 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -2,7 +2,7 @@ import datetime import logging import analytics -from celery.task import task, Task +from celery import task from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site @@ -10,6 +10,8 @@ from django.core.exceptions import ValidationError from django.db.utils import DatabaseError +from celery_utils.logged_task import LoggedTask +from celery_utils.persist_on_failure import LoggedPersistOnFailureTask from edx_ace import ace from edx_ace.message import Message from edx_ace.utils.date import deserialize, serialize @@ -36,7 +38,7 @@ UPGRADE_REMINDER_LOG_PREFIX = 'Upgrade Reminder' COURSE_UPDATE_LOG_PREFIX = 'Course Update' -@task(bind=True, default_retry_delay=30, routing_key=ROUTING_KEY) +@task(base=LoggedPersistOnFailureTask, bind=True, default_retry_delay=30, routing_key=ROUTING_KEY) def update_course_schedules(self, **kwargs): course_key = CourseKey.from_string(kwargs['course_id']) new_start_date = deserialize(kwargs['new_start_date_str']) @@ -53,7 +55,11 @@ def update_course_schedules(self, **kwargs): raise self.retry(kwargs=kwargs, exc=exc) -class ScheduleMessageBaseTask(Task): +class ScheduleMessageBaseTask(LoggedTask): + """ + Base class for top-level Schedule tasks that create subtasks + for each Bin. + """ ignore_result = True routing_key = ROUTING_KEY num_bins = resolvers.DEFAULT_NUM_BINS @@ -95,7 +101,7 @@ class ScheduleMessageBaseTask(Task): override_recipient_email, ) cls.log_info('Launching task with args = %r', task_args) - cls.apply_async( + cls().apply_async( task_args, retry=False, ) @@ -126,7 +132,7 @@ class ScheduleMessageBaseTask(Task): raise NotImplementedError -@task(ignore_result=True, routing_key=ROUTING_KEY) +@task(base=LoggedTask, ignore_result=True, routing_key=ROUTING_KEY) def _recurring_nudge_schedule_send(site_id, msg_str): _schedule_send( msg_str, @@ -136,7 +142,7 @@ def _recurring_nudge_schedule_send(site_id, msg_str): ) -@task(ignore_result=True, routing_key=ROUTING_KEY) +@task(base=LoggedTask, ignore_result=True, routing_key=ROUTING_KEY) def _upgrade_reminder_schedule_send(site_id, msg_str): _schedule_send( msg_str, @@ -146,7 +152,7 @@ def _upgrade_reminder_schedule_send(site_id, msg_str): ) -@task(ignore_result=True, routing_key=ROUTING_KEY) +@task(base=LoggedTask, ignore_result=True, routing_key=ROUTING_KEY) def _course_update_schedule_send(site_id, msg_str): _schedule_send( msg_str, diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 59abf69e7af..b177507bb50 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -38,7 +38,7 @@ djangorestframework-jwt==1.11.0 enum34==1.1.6 edx-ace==0.1.6 edx-ccx-keys==0.2.1 -edx-celeryutils==0.2.6 +edx-celeryutils==0.2.7 edx-drf-extensions==1.2.3 edx-i18n-tools==0.3.10 edx-lint==0.4.3 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 3425c405327..26dde69890c 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -104,6 +104,7 @@ git+https://github.com/edx/edx-proctoring.git@1.3.1#egg=edx-proctoring==1.3.1 # This is here because all of the other XBlocks are located here. However, it is published to PyPI and will be installed that way xblock-review==1.1.1 + # Third Party XBlocks git+https://github.com/mitodl/edx-sga.git@d019b8a050c056db535e3ff13c93096145a932de#egg=edx-sga==0.7.1 -- GitLab