From 4700656980476efed45c59bbf4d3ec8bc49a9520 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 17 Oct 2017 21:10:06 -0400 Subject: [PATCH] Move bin-count constants into resolvers.py nearer to the queries that use them --- .../tests/test_send_recurring_nudge.py | 18 +++++++++--------- .../tests/test_send_upgrade_reminder.py | 16 ++++++++-------- openedx/core/djangoapps/schedules/resolvers.py | 9 +++++---- openedx/core/djangoapps/schedules/tasks.py | 4 ---- .../schedules/tests/test_resolvers.py | 2 +- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py index 85c9c8477f..761ec19816 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py @@ -81,7 +81,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): retry=False, ) mock_schedule_bin.apply_async.assert_any_call( - (self.site_config.site.id, serialize(test_day), -3, tasks.RECURRING_NUDGE_NUM_BINS - 1, [], True, None), + (self.site_config.site.id, serialize(test_day), -3, resolvers.RECURRING_NUDGE_NUM_BINS - 1, [], True, None), retry=False, ) self.assertFalse(mock_ace.send.called) @@ -97,11 +97,11 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): ) for i in range(schedule_count) ] - bins_in_use = frozenset((s.enrollment.user.id % tasks.RECURRING_NUDGE_NUM_BINS) for s in schedules) + bins_in_use = frozenset((s.enrollment.user.id % resolvers.RECURRING_NUDGE_NUM_BINS) for s in schedules) test_datetime = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + for b in range(resolvers.RECURRING_NUDGE_NUM_BINS): expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES if b in bins_in_use: # to fetch course modes for valid schedules @@ -126,7 +126,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): test_datetime = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + for b in range(resolvers.RECURRING_NUDGE_NUM_BINS): with self.assertNumQueries(NUM_QUERIES_NO_MATCHING_SCHEDULES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3, bin_num=b, @@ -143,7 +143,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): @patch.object(tasks, '_recurring_nudge_schedule_send') def test_send_after_course_end(self, mock_schedule_send): - user1 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS) + user1 = UserFactory.create(id=resolvers.RECURRING_NUDGE_NUM_BINS) schedule = ScheduleFactory.create( start=datetime.datetime(2017, 8, 3, 20, 34, 30, tzinfo=pytz.UTC), @@ -199,8 +199,8 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): for config in (limited_config, unlimited_config): ScheduleConfigFactory.create(site=config.site) - user1 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS) - user2 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS * 2) + user1 = UserFactory.create(id=resolvers.RECURRING_NUDGE_NUM_BINS) + user2 = UserFactory.create(id=resolvers.RECURRING_NUDGE_NUM_BINS * 2) ScheduleFactory.create( start=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), @@ -247,7 +247,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3, - bin_num=user.id % tasks.RECURRING_NUDGE_NUM_BINS, + bin_num=user.id % resolvers.RECURRING_NUDGE_NUM_BINS, org_list=[schedules[0].enrollment.course.org], ) self.assertEqual(mock_schedule_send.apply_async.call_count, 1) @@ -434,7 +434,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): return templates_override def _calculate_bin_for_user(self, user): - return user.id % tasks.RECURRING_NUDGE_NUM_BINS + return user.id % resolvers.RECURRING_NUDGE_NUM_BINS def _contains_upsell_attribute(self, msg_attr): msg = Message.from_string(msg_attr) 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 67a8967d46..1134bfbb06 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 @@ -85,7 +85,7 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): retry=False, ) mock_schedule_bin.apply_async.assert_any_call( - (self.site_config.site.id, serialize(test_day), 2, tasks.UPGRADE_REMINDER_NUM_BINS - 1, [], True, None), + (self.site_config.site.id, serialize(test_day), 2, resolvers.UPGRADE_REMINDER_NUM_BINS - 1, [], True, None), retry=False, ) self.assertFalse(mock_ace.send.called) @@ -101,11 +101,11 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): ) for i in range(schedule_count) ] - bins_in_use = frozenset((s.enrollment.user.id % tasks.UPGRADE_REMINDER_NUM_BINS) for s in schedules) + bins_in_use = frozenset((s.enrollment.user.id % resolvers.UPGRADE_REMINDER_NUM_BINS) for s in schedules) test_datetime = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + for b in range(resolvers.UPGRADE_REMINDER_NUM_BINS): expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES if b in bins_in_use: # to fetch course modes for valid schedules @@ -131,7 +131,7 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): test_datetime = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + for b in range(resolvers.UPGRADE_REMINDER_NUM_BINS): with self.assertNumQueries(NUM_QUERIES_NO_MATCHING_SCHEDULES, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=b, @@ -183,8 +183,8 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): for config in (limited_config, unlimited_config): ScheduleConfigFactory.create(site=config.site) - user1 = UserFactory.create(id=tasks.UPGRADE_REMINDER_NUM_BINS) - user2 = UserFactory.create(id=tasks.UPGRADE_REMINDER_NUM_BINS * 2) + user1 = UserFactory.create(id=resolvers.UPGRADE_REMINDER_NUM_BINS) + user2 = UserFactory.create(id=resolvers.UPGRADE_REMINDER_NUM_BINS * 2) ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), @@ -231,7 +231,7 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, - bin_num=user.id % tasks.UPGRADE_REMINDER_NUM_BINS, + bin_num=user.id % resolvers.UPGRADE_REMINDER_NUM_BINS, org_list=[schedules[0].enrollment.course.org], ) self.assertEqual(mock_schedule_send.apply_async.call_count, 1) @@ -309,4 +309,4 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): return templates_override def _calculate_bin_for_user(self, user): - return user.id % tasks.RECURRING_NUDGE_NUM_BINS + return user.id % resolvers.RECURRING_NUDGE_NUM_BINS diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 71cbd9490b..7fe74d6e87 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -18,10 +18,6 @@ from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig from openedx.core.djangoapps.schedules.tasks import ( - DEFAULT_NUM_BINS, - RECURRING_NUDGE_NUM_BINS, - UPGRADE_REMINDER_NUM_BINS, - COURSE_UPDATE_NUM_BINS, recurring_nudge_schedule_bin, upgrade_reminder_schedule_bin, course_update_schedule_bin, @@ -41,6 +37,11 @@ from xmodule.modulestore.django import modulestore LOG = logging.getLogger(__name__) +DEFAULT_NUM_BINS = 24 +RECURRING_NUDGE_NUM_BINS = DEFAULT_NUM_BINS +UPGRADE_REMINDER_NUM_BINS = DEFAULT_NUM_BINS +COURSE_UPDATE_NUM_BINS = DEFAULT_NUM_BINS + class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): """ Starts num_bins number of async tasks, each of which sends emails to an equal group of learners. diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 3a7ced3841..45b76e1a6a 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -31,10 +31,6 @@ KNOWN_RETRY_ERRORS = ( # Errors we expect occasionally that could resolve on re DatabaseError, ValidationError, ) -DEFAULT_NUM_BINS = 24 -RECURRING_NUDGE_NUM_BINS = DEFAULT_NUM_BINS -UPGRADE_REMINDER_NUM_BINS = DEFAULT_NUM_BINS -COURSE_UPDATE_NUM_BINS = DEFAULT_NUM_BINS @task(bind=True, default_retry_delay=30, routing_key=ROUTING_KEY) diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 0854c52b56..1638f2158a 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -6,7 +6,7 @@ from django.conf import settings from mock import patch from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver -from openedx.core.djangoapps.schedules.tasks import DEFAULT_NUM_BINS +from openedx.core.djangoapps.schedules.resolvers import DEFAULT_NUM_BINS from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms