Optimize block date retrieval for course outlines. #99

doctoryes · 2021-08-24T23:16:13Z

Description:

When a course outline is requested, currently all block dates in the course are returned. This PR adds an optional optimization that allows only the dates for outline-relevant block types to be returned (course, chapter, sequential), which should greatly improve response times.

Also, caching is re-worked to use the TieredCache which caches data in both the ephemeral request cache as well as Django's long-term cache.

I've also created an edx-platform PR which uses this edx-when version for testing and proving out these changes - it's here:
https://github.com/edx/edx-platform/pull/28551

JIRA: https://openedx.atlassian.net/browse/TNL-8061

Dependencies: None

Merge deadline: None

Installation instructions: None

Testing instructions:

The change has been deployed to this sandbox:
https://edx-when-juliasq.sandbox.edx.org/
in order to test normal operation.

Reviewers:

Merge checklist:

Post merge:

Create a tag
Check new version is pushed to PyPi after tag-triggered build is
finished.
Delete working branch (if not needed anymore)

Author concerns: I've added notes throughout.

ormsbee · 2021-08-25T18:25:26Z

edx_when/api.py

-    return f"edx-when.content_dates:{course_key}:{query_dict_str}"
+    published_version_str = ''
+    if published_version:
+        published_version_str = published_version


Is published_version supposed to be a str at this point, or still the BSON obj?

In theory, published_version would be a str at this point. But since I still haven't gotten my hands on an actual published_version, I'm not yet sure what type it will be.

I'm trying to get hold of a published_version here:
https://github.com/edx/edx-platform/blob/7e146e04879944c956ee89fb5ae126ae0e3371c7/openedx/core/djangoapps/course_date_signals/handlers.py#L130
And haven't had any luck yet...

It's a str at this point - verified it.

edx_when/api.py

ormsbee · 2021-08-25T19:20:36Z

edx_when/api.py

+        version = published_version
+        if published_version:
+            version, __ = models.CourseVersion.objects.get_or_create(published_version=published_version)


Minor and optional, but personally, I find it easier to digest this when it's broken out something like:

version = None if published_version: version, __ = models.CourseVersion.objects.get_or_create(published_version=published_version)

or

if published_version: version, __ = models.CourseVersion.objects.get_or_create(published_version=published_version) else: version = None

Mostly because it's easier for my head to grok that version can be None or this model object, but it's weird when I think that it can also be a string, but only for a really limited time.

ormsbee · 2021-08-25T19:29:41Z

edx_when/models.py


    class Meta:
        """Django Metadata."""

        unique_together = ('policy', 'location', 'field')
+        constraints = [
+            models.UniqueConstraint(fields=('location', 'block_type'), name='unique_location_block_type'),


If you have a unique constraint, I think the extra index is redundant.

ormsbee · 2021-08-25T19:30:08Z

edx_when/models.py

@@ -96,11 +113,19 @@ class ContentDate(models.Model):
    location = UsageKeyField(null=True, default=None, db_index=True, max_length=255)
    field = models.CharField(max_length=255, default='')
    active = models.BooleanField(default=True)
+    block_type = models.CharField(max_length=255, null=True)


We probably also want a composite index on (course_id, block_type)?

Added below.

doctoryes · 2021-08-27T20:32:12Z

edx_when/api.py

+            needs_save = True
+        if existing_date.block_type != block_id.block_type:
+            existing_date.block_type = block_id.block_type
+            needs_save = True


As the new block_type field is added as NULL, this code will lazily fill the field with block types.

doctoryes · 2021-08-27T20:34:43Z

edx_when/api.py

+        qset = models.ContentDate.objects.filter(course_id=course_id, active=True, **rel_lookup)
+        if subsection_and_higher_only:
+            # Include NULL block_type values as well because of lazy rollout.
+            qset = qset.filter(Q(block_type__in=('course', 'chapter', 'sequential', None)))


This filter is the new one that limits the queryset to the block types shown. None is also included because of the lazy filling of the block_type field.

Do we have a condition on which we can remove the None? Or just always keep because we won't ever be able to guarantee everything has it filled?

This PR adds the new column with null=True to avoid DB locking on deployment. And I've chosen to do a lazy load of the column upon course publish. With this strategy, it's possible that some courses will never be published and won't have values for block_type ever - unless we populate the value for all courses using some asynchronous process. Once a course is published, no None values would be expected for block_type.

Summary: With the current plan, just always keep.

doctoryes · 2021-08-27T20:36:15Z

edx_when/api.py

-    qset = cache.get(external_cache_key) if use_cached else None
+    raw_results_cache_key = _content_dates_cache_key(
+        course_id, rel_lookup, subsection_and_higher_only, published_version
+    )


This is the cache key for the raw, unprocessed query results before the additional processing below.

doctoryes · 2021-08-27T20:36:42Z

edx_when/api.py

+    # query set to be returned.
+    processed_results_cache_key = _processed_results_cache_key(
+        course_id, user_id, schedule, allow_relative_dates, subsection_and_higher_only, published_version
+    )


This is the cache key for the queried and processed results.

doctoryes · 2021-08-27T20:38:35Z

edx_when/api.py

+                cache_key = _content_dates_cache_key(
+                    course_key, query_dict, subsection_and_higher_only, course_version
+                )
+                TieredCache.delete_all_tiers(cache_key)


This cache clearing got a bit uglier because of the new additions to the cache key. 😿

Does it need to be called though? If the version information is encoded into the cache key?

I would have just removed it - but the fact that a None published_version can still be sent in made me think that the code still should clear the cache in that case. (I also hadn't made the edx-platform PR yet which uses the new edx-when, so I wasn't sure at the time if a published course version could be passed-in for all calls. I've since found out that a course version can be passed-in everywhere.)

Also, in the edx-platform PR, I'm getting the published version from course.course_version in most places. And I don't understand completely when I can be assured that attribute will be present and filled-in. In Old Mongo courses, wouldn't that attribute be None or non-existent?

Dillon-Dumesnil

So I did my best to understand this, but I'm honestly not particularly well versed in caching so feel free to take all my comments with a grain of salt. I think it would be good to wait (if possible) for Mike Terry to return from vacation so he can also take a look. He is back Thursday

Dillon-Dumesnil · 2021-08-30T16:11:54Z

edx_when/api.py

+        qset = models.ContentDate.objects.filter(course_id=course_id, active=True, **rel_lookup)
+        if subsection_and_higher_only:
+            # Include NULL block_type values as well because of lazy rollout.
+            qset = qset.filter(Q(block_type__in=('course', 'chapter', 'sequential', None)))


Do we have a condition on which we can remove the None? Or just always keep because we won't ever be able to guarantee everything has it filled?

Dillon-Dumesnil · 2021-08-30T16:18:11Z

edx_when/models.py


    class Meta:
        """Django Metadata."""

        unique_together = ('policy', 'location', 'field')
+        indexes = [


Nit: Add course_id and location here using the Index format instead of db_index on the field definition.

Interesting - I'll try this and see how it changes the migration file.

Dillon-Dumesnil · 2021-08-30T16:31:54Z

edx_when/api.py

@@ -296,14 +364,17 @@ def get_overrides_for_user(course_id, user):
        yield {'location': udate.content_date.location, 'actual_date': udate.actual_date}


-def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None, reason='', actor=None):
+def set_date_for_block(


Should this method also be clearing the cache when a new date is set? I know we do it when we clear dates, but wouldn't this be a similar process?

The assumption I'd made here is that a set_date_for_block would be preceded by a course publish - and therefore a new published_version would be passed-in. This new published_version would leave the old cached value behind. Clearing all course dates on blocks might not necessarily be preceded by a course publish.

Dillon-Dumesnil · 2021-08-30T16:34:43Z

edx_when/models.py

@@ -84,6 +84,16 @@ def clean(self):
            raise ValidationError(_("Absolute and relative dates cannot both be used"))


+class CourseVersion(models.Model):


Why have this as its own model rather than just a field on a ContentDate?

Good question! Because I just can't help myself from normalizing DB data? 😸 It would also work fine as a field on ContentDate - I arbitrarily chose to use a new model.

doctoryes

Thanks for the review, @Dillon-Dumesnil ! I'll discuss with my team about waiting for a merge. We'd like to get a PR approval from an owner of this repo ideally before proceeding.

doctoryes · 2021-08-30T18:27:55Z

edx_when/api.py

+        qset = models.ContentDate.objects.filter(course_id=course_id, active=True, **rel_lookup)
+        if subsection_and_higher_only:
+            # Include NULL block_type values as well because of lazy rollout.
+            qset = qset.filter(Q(block_type__in=('course', 'chapter', 'sequential', None)))


This PR adds the new column with null=True to avoid DB locking on deployment. And I've chosen to do a lazy load of the column upon course publish. With this strategy, it's possible that some courses will never be published and won't have values for block_type ever - unless we populate the value for all courses using some asynchronous process. Once a course is published, no None values would be expected for block_type.

Summary: With the current plan, just always keep.

doctoryes · 2021-08-30T18:52:32Z

edx_when/api.py

@@ -296,14 +364,17 @@ def get_overrides_for_user(course_id, user):
        yield {'location': udate.content_date.location, 'actual_date': udate.actual_date}


-def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None, reason='', actor=None):
+def set_date_for_block(


The assumption I'd made here is that a set_date_for_block would be preceded by a course publish - and therefore a new published_version would be passed-in. This new published_version would leave the old cached value behind. Clearing all course dates on blocks might not necessarily be preceded by a course publish.

doctoryes · 2021-08-30T18:52:39Z

edx_when/api.py

-def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None, reason='', actor=None):
+def set_date_for_block(
+        course_id, block_id, field, date_or_timedelta,
+        user=None, reason='', actor=None, published_version=None):


FYI: while the published_version is saved on each ContentDate object, it's not used as a filter when querying course dates (though it is returned for each block).

doctoryes · 2021-08-30T18:55:04Z

edx_when/models.py

@@ -84,6 +84,16 @@ def clean(self):
            raise ValidationError(_("Absolute and relative dates cannot both be used"))


+class CourseVersion(models.Model):


Good question! Because I just can't help myself from normalizing DB data? 😸 It would also work fine as a field on ContentDate - I arbitrarily chose to use a new model.

doctoryes · 2021-08-30T18:57:08Z

edx_when/models.py


    class Meta:
        """Django Metadata."""

        unique_together = ('policy', 'location', 'field')
+        indexes = [


Interesting - I'll try this and see how it changes the migration file.

doctoryes · 2021-08-31T15:56:10Z

edx_when/models.py

@@ -96,18 +106,23 @@ class ContentDate(models.Model):
    location = UsageKeyField(null=True, default=None, db_index=True, max_length=255)
    field = models.CharField(max_length=255, default='')
    active = models.BooleanField(default=True)
+    block_type = models.CharField(max_length=255, null=True)
+    course_version = models.ForeignKey(CourseVersion, null=True, on_delete=models.CASCADE)


@ormsbee In thinking more about this field, does it need to be present at all? The code currently does not use it to filter against. It's stored for ContentDate rows but never used to filter against in the current code - so it's really there just as a piece of info that might be useful for future debugging. The course_version/published_version is currently only used to invalidate old cache keys upon a new course publish.

It sounds fine to remove.

doctoryes · 2021-08-31T23:10:54Z

edx_when/migrations/0008_courseversion_block_type.py

+            model_name='contentdate',
+            index=models.Index(fields=['location'], name='edx_when_location_idx'),
+        ),
+    ]


Here's the SQL which this migration will run - generated with ./manage.py cms sqlmigrate edx_when 0008:

BEGIN; -- -- Add field block_type to contentdate -- ALTER TABLE `edx_when_contentdate` ADD COLUMN `block_type` varchar(255) NULL; -- -- Alter field course_id on contentdate -- DROP INDEX `edx_when_contentdate_course_id_e6c39fdc` ON `edx_when_contentdate`; -- -- Alter field location on contentdate -- DROP INDEX `edx_when_contentdate_location_485206ea` ON `edx_when_contentdate`; -- -- Create index edx_when_course_block_type_idx on field(s) course_id, block_type of model contentdate -- CREATE INDEX `edx_when_course_block_type_idx` ON `edx_when_contentdate` (`course_id`, `block_type`); -- -- Create index edx_when_course_id_idx on field(s) course_id of model contentdate -- CREATE INDEX `edx_when_course_id_idx` ON `edx_when_contentdate` (`course_id`); -- -- Create index edx_when_location_idx on field(s) location of model contentdate -- CREATE INDEX `edx_when_location_idx` ON `edx_when_contentdate` (`location`); COMMIT;

Ah whoops. I didn't mean for additional SQL to be executed via the switch to models.Index vs the db_index parameter. Happy to have you keep it, but if you'd rather not keep in these additional changes, totally understand.

I'm actually going to remove it - just to avoid risk by doing less index creation and dropping for this PR. Thanks!

I've removed the index changes as mentioned - the SQL that will now be executed is below:

BEGIN; -- -- Add field block_type to contentdate -- ALTER TABLE `edx_when_contentdate` ADD COLUMN `block_type` varchar(255) NULL; -- -- Create index edx_when_course_block_type_idx on field(s) course_id, block_type of model contentdate -- CREATE INDEX `edx_when_course_block_type_idx` ON `edx_when_contentdate` (`course_id`, `block_type`); COMMIT;

doctoryes · 2021-08-31T23:16:16Z

@Dillon-Dumesnil To clarify the concerns about approving this PR now:

Is @mikix 's approval a pre-requisite for merging?
Would your team feel comfortable with us merging/deploying this change based on our own review - and having @mikix review the changes post-deploy?
- Naturally, our team would assume responsibility for the deployment and any needed fixes/rollback.
  Thanks!

Dillon-Dumesnil

Sorry for all the nits late in the game, but hey, they're nits. Do with them what you will. Biggest question is probably regarding the functionality (in the comment about subsection_and_higher_only variable name), but trust y'all's decision on that.
Thanks for this PR and all of the little improvements you made throughout the repo. We always appreciate a PR that leaves the place better than it found it and this certainly hits that!

Dillon-Dumesnil · 2021-09-01T13:26:51Z

edx_when/api.py

@@ -26,15 +25,27 @@
 FIELDS_TO_EXTRACT = ('due', 'start', 'end')


-def _content_dates_cache_key(course_key, query_dict):
-    """Memcached key for ContentDates given course_key and filter args."""
+def _content_dates_cache_key(course_key, query_dict, subsection_and_higher_only, published_version):


Nit: I don't love the name subsection_and_higher_only. It's so specific to a use case, but I don't ave any better ideas. I do appreciate the change from outline_only as I know we discussed that before and that it bled business logic.

I feel like ideally, this feature would be able to just specify different block types and have those returned and then the cache key would reflect those. I'm not sure how feasible/realistic that is so this is all just a nit and me thinking about different ideas. But being able to say "get me only units" or "sections and subsections" and pass in ['vertical'] or ['chapter', 'sequential'] would reduce any specificity of this feature. And enable the outline to still say, give me ['course', 'chapter', 'sequential']. Or maybe even ['course', 'sequential'] (I'm not sure we even use the section dates on the outline).

I'm not in 😍 with the name subsection_or_higher_only either - but it's the best I could think of and it's at least explicit without mentioning outlines. I agree with your proposed ideal solution as well. But I pulled back from that solution (passing in a list of block types) to avoid the extra scope & testing - and KISS/YAGNI.

Yeah, that's fair. Thank you!

Dillon-Dumesnil · 2021-09-01T13:27:52Z

edx_when/api.py

    query_dict_str = ".".join(
        sorted(
            "{},{}".format(key, value or "")
            for key, value in query_dict.items()
        )
    )
-    return f"edx-when.content_dates:{course_key}:{query_dict_str}"
+    high_level_block_str = ''


Nit: switching this variable to be more like the string it will actually be. It's the only spot we use high_level_block vs subsection_higher

Agree - I'll change it.

Dillon-Dumesnil · 2021-09-01T13:32:57Z

edx_when/api.py

@@ -122,9 +135,32 @@ def _get_end_dates_from_content_dates(qset):
    return end_datetime, cutoff_datetime


+def _processed_results_cache_key(
+        course_id, user_id, schedule, allow_relative_dates,
+        subsection_and_higher_only, published_version):


Super Nit: bring the ): to be in line with the function name

Will do here and elsewhere in the file.

Dillon-Dumesnil · 2021-09-01T13:35:00Z

edx_when/api.py

-    return f"edx-when.content_dates:{course_key}:{query_dict_str}"
+    high_level_block_str = ''
+    if subsection_and_higher_only:
+        high_level_block_str = 'subsection_higher'


Sorry if it's just me not understanding cache keys particularly well, but why use subsection_higher here, but in _processed_results_cache_key, you use subsection_and_higher_only. I also realize the keys are different anyway (: vs .).

Thanks for pointing that out! I'll change this to stay consistent.

Dillon-Dumesnil · 2021-09-01T13:35:23Z

edx_when/api.py

+    This method changes currently-cached rows - but the cache key contains the course's published version.
+    And the calling method is *only* invoked upon course publish, which guarantees that a new course published
+    version will be used to get course dates after this call is made, invalidating existing cache entries.


Thank you for this comment. I'm sure it will be a life-saver in the future

Dillon-Dumesnil · 2021-09-01T14:12:43Z

edx_when/api.py

@@ -327,21 +394,14 @@ def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None,
            existing_date.active = True
        except models.ContentDate.DoesNotExist as error:
            if user:
+                # A UserDate creation below requires an existing ContentDate.


Thank you for these awesome comments! Always good to have some fresh eyes

It took awhile to figure it out! So thought I'd capture it.

Dillon-Dumesnil · 2021-09-01T14:14:37Z

edx_when/api.py

@@ -318,6 +377,14 @@ def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None,
    else:
        date_kwargs = {'abs_date': date_or_timedelta}

+    def _set_content_date_policy(date_kwargs, existing_content_date):
+        # Race conditions were creating multiple DatePolicies w/ the same values. Handle that case.
+        existing_policies = list(models.DatePolicy.objects.filter(**date_kwargs).order_by('id'))


Just me wondering, and no obligation at all to test or try or change anything, but I wonder why do the list and [0] vs just using .first() on the query set. Not sure the error it throws if none (maybe none) but maybe an option. Not at all for you to do, just commenting while in my head.

According to https://docs.djangoproject.com/en/2.2/ref/models/querysets/#first, using first() would work just as well. I simply copied the existing code, removing the never-thrown exception.

Dillon-Dumesnil · 2021-09-01T14:17:18Z

edx_when/api.py

            needs_save = True

        if user and not user.is_anonymous:
+            # Course block date is for a particular user.


Super Nit: Put the comment here and on line 419 above the if/else lines (one line higher) rather than just below. I personally always interpret python comments on what is directly beneath so seeing this comment and then the UserDate below, I was like yeah, cool, checks out. But then on line 419, the if statement didn't click with the comment and I realized it was applying to the whole else block. It's a small thing, feel free to ignore

Dillon-Dumesnil · 2021-09-01T14:17:41Z

edx_when/api.py

@@ -318,6 +377,14 @@ def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None,
    else:
        date_kwargs = {'abs_date': date_or_timedelta}

+    def _set_content_date_policy(date_kwargs, existing_content_date):


Thanks for DRYing it up in here

Dillon-Dumesnil · 2021-09-01T14:19:40Z

edx_when/migrations/0008_courseversion_block_type.py

+            model_name='contentdate',
+            index=models.Index(fields=['location'], name='edx_when_location_idx'),
+        ),
+    ]


Ah whoops. I didn't mean for additional SQL to be executed via the switch to models.Index vs the db_index parameter. Happy to have you keep it, but if you'd rather not keep in these additional changes, totally understand.

doctoryes

Thanks so much for the re-review, @Dillon-Dumesnil ! I've addressed all your comments and will incorporate most of them.

doctoryes · 2021-09-01T15:12:13Z

edx_when/api.py

@@ -26,15 +25,27 @@
 FIELDS_TO_EXTRACT = ('due', 'start', 'end')


-def _content_dates_cache_key(course_key, query_dict):
-    """Memcached key for ContentDates given course_key and filter args."""
+def _content_dates_cache_key(course_key, query_dict, subsection_and_higher_only, published_version):


I'm not in 😍 with the name subsection_or_higher_only either - but it's the best I could think of and it's at least explicit without mentioning outlines. I agree with your proposed ideal solution as well. But I pulled back from that solution (passing in a list of block types) to avoid the extra scope & testing - and KISS/YAGNI.

doctoryes · 2021-09-01T15:16:13Z

edx_when/api.py

-    return f"edx-when.content_dates:{course_key}:{query_dict_str}"
+    high_level_block_str = ''
+    if subsection_and_higher_only:
+        high_level_block_str = 'subsection_higher'


Thanks for pointing that out! I'll change this to stay consistent.

doctoryes · 2021-09-01T15:17:03Z

edx_when/api.py

    query_dict_str = ".".join(
        sorted(
            "{},{}".format(key, value or "")
            for key, value in query_dict.items()
        )
    )
-    return f"edx-when.content_dates:{course_key}:{query_dict_str}"
+    high_level_block_str = ''


Agree - I'll change it.

doctoryes · 2021-09-01T15:20:27Z

edx_when/api.py

+        cache_key += '.with-rel-dates'
+    if subsection_and_higher_only:
+        cache_key += '.subsection_and_higher_only'
+    # Always add published version to the cache key.


I'll remove - states the obvious.

doctoryes · 2021-09-01T15:21:19Z

edx_when/api.py

+        use_cached: bool (optional) - skips cache lookups (but not saves) if False
+        schedule: Schedule obj (optional) - override for a user's enrollment Schedule, used
+            for relative date calculations
+        subsection_and_higher_only: bool (optional) - used to generate course outlines; only returns dates for blocks


doctoryes · 2021-09-01T15:26:50Z

edx_when/api.py

@@ -318,6 +377,14 @@ def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None,
    else:
        date_kwargs = {'abs_date': date_or_timedelta}

+    def _set_content_date_policy(date_kwargs, existing_content_date):
+        # Race conditions were creating multiple DatePolicies w/ the same values. Handle that case.
+        existing_policies = list(models.DatePolicy.objects.filter(**date_kwargs).order_by('id'))


According to https://docs.djangoproject.com/en/2.2/ref/models/querysets/#first, using first() would work just as well. I simply copied the existing code, removing the never-thrown exception.

doctoryes · 2021-09-01T15:27:22Z

edx_when/api.py

@@ -327,21 +394,14 @@ def set_date_for_block(course_id, block_id, field, date_or_timedelta, user=None,
            existing_date.active = True
        except models.ContentDate.DoesNotExist as error:
            if user:
+                # A UserDate creation below requires an existing ContentDate.


It took awhile to figure it out! So thought I'd capture it.

doctoryes · 2021-09-01T15:29:15Z

edx_when/api.py

            needs_save = True

        if user and not user.is_anonymous:
+            # Course block date is for a particular user.


doctoryes · 2021-09-01T15:30:31Z

edx_when/migrations/0008_courseversion_block_type.py

+            model_name='contentdate',
+            index=models.Index(fields=['location'], name='edx_when_location_idx'),
+        ),
+    ]


I'm actually going to remove it - just to avoid risk by doing less index creation and dropping for this PR. Thanks!

doctoryes · 2021-09-01T15:40:21Z

edx_when/api.py

@@ -122,9 +135,32 @@ def _get_end_dates_from_content_dates(qset):
    return end_datetime, cutoff_datetime


+def _processed_results_cache_key(
+        course_id, user_id, schedule, allow_relative_dates,
+        subsection_and_higher_only, published_version):


Will do here and elsewhere in the file.

… for use in a course outline, which don't need block dates below the subsection level of a course. Also, rework the block date caching to use TieredCache, as well as using the current published version of a course in the cache key to remove the need for cache invalidation. TNL-8061

mikix

Just a quick post-merge comment to say this looks good and thank you Julia for your work!

mikix · 2021-09-02T14:17:56Z

edx_when/__init__.py

@@ -1,7 +1,7 @@
 """
-Your project description goes here.
+Central source of course block dates for the LMS.


doctoryes changed the title ~~Juliasq/tnl 8061 optimize outline block data retrieval~~ Optimize block date retrieval for course outlines. Aug 25, 2021

ormsbee reviewed Aug 25, 2021

View reviewed changes

edx_when/api.py Outdated Show resolved Hide resolved

ormsbee reviewed Aug 25, 2021

View reviewed changes

doctoryes force-pushed the juliasq/TNL_8061_optimize_outline_block_data_retrieval branch 6 times, most recently from 2232f61 to 61798b4 Compare August 27, 2021 16:51

doctoryes commented Aug 27, 2021

View reviewed changes

doctoryes marked this pull request as ready for review August 27, 2021 22:40

doctoryes requested review from kenclary, kdmccormick, mikix, connorhaugh and Dillon-Dumesnil August 27, 2021 22:41

Dillon-Dumesnil reviewed Aug 30, 2021

View reviewed changes

doctoryes commented Aug 30, 2021

View reviewed changes

doctoryes commented Aug 31, 2021

View reviewed changes

doctoryes force-pushed the juliasq/TNL_8061_optimize_outline_block_data_retrieval branch 3 times, most recently from 3d16a0e to dc00e3d Compare August 31, 2021 20:34

doctoryes commented Aug 31, 2021

View reviewed changes

Dillon-Dumesnil approved these changes Sep 1, 2021

View reviewed changes

doctoryes commented Sep 1, 2021

View reviewed changes

doctoryes force-pushed the juliasq/TNL_8061_optimize_outline_block_data_retrieval branch from dc00e3d to b9930ec Compare September 1, 2021 15:47

ormsbee approved these changes Sep 1, 2021

View reviewed changes

doctoryes merged commit 5b233d5 into master Sep 1, 2021

doctoryes deleted the juliasq/TNL_8061_optimize_outline_block_data_retrieval branch September 1, 2021 17:53

mikix reviewed Sep 2, 2021

View reviewed changes

Dillon-Dumesnil mentioned this pull request Sep 3, 2021

Allow for pulling back only outline-related block types. #48

Closed

		@@ -84,6 +84,16 @@ def clean(self):
		raise ValidationError(_("Absolute and relative dates cannot both be used"))


		class CourseVersion(models.Model):

Optimize block date retrieval for course outlines. #99

Optimize block date retrieval for course outlines. #99

Conversation

doctoryes commented Aug 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dillon-Dumesnil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doctoryes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doctoryes Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

doctoryes commented Aug 31, 2021

Dillon-Dumesnil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doctoryes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doctoryes commented Aug 24, 2021 •

edited

Loading

doctoryes Sep 1, 2021 •

edited

Loading