Skip to content

Conversation

@shanbady
Copy link
Contributor

@shanbady shanbady commented Sep 3, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5246

Description (What does it do?)

Currently the "programs" and "courses counts displayed on the unit page(/units/) comes from opensearch aggregations which has been determined to be expensive for just getting counts of things that change infrequently. This PR moves the counts off to their own endpoint which is then cached and invalidated along with learning resources (which get invalidated when new resources are fetched).

How can this be tested?

  1. checkout this branch and make sure you have learning resources and units loaded
  2. visit /units/ and see that counts show up.
  3. go into django admin and delete some resources associated with a unit
  4. go back to /units/ and note that the counts do not changed (they are fetched from cache)
  5. try this as a logged in and logged out user (cache behavior should be the same)

@shanbady shanbady added Needs Review An open Pull Request that is ready for review and removed Work in Progress labels Sep 4, 2024
@shanbady shanbady marked this pull request as ready for review September 4, 2024 15:07
@mbertrand mbertrand self-assigned this Sep 5, 2024
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, only question is regarding this:

which get invalidated when new resources are fetched

I'm not seeing that, if I add new OCW courses, the OCW count remains the same. Should the cache get invalidated when courses are added?

Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache is invalidated now after adding more courses, 👍



class ChannelCountsSerializer(serializers.ModelSerializer):
counts = serializers.SerializerMethodField(read_only=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a docstring

@mbertrand mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 5, 2024
@shanbady shanbady merged commit b8a5901 into main Sep 5, 2024
@shanbady shanbady deleted the shanbady/cache-units-page-counts branch September 5, 2024 18:19
@odlbot odlbot mentioned this pull request Sep 6, 2024
11 tasks
@odlbot odlbot mentioned this pull request Sep 6, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants