-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: added caching to XBlockSkillsViewSet list endpoint #208
Conversation
9430a9a
to
0fc496c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
436a650
to
ef65d4c
Compare
ef65d4c
to
9f3aadf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add the necessary details along with the ticket link in the PR description? Also, try to resolve the coverage issues before merging.
Other than that, everything looks good to me 👍.
|
||
cached_response = TieredCache.get_cached_response(cache_key) | ||
if cached_response.is_found: | ||
return Response(cached_response.value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a unit test for this.
42aed42
to
5316ea8
Compare
5316ea8
to
0441cad
Compare
PROD 4135
Merge checklist:
requirements/*.txt
files)make upgrade && make requirements
have been run to regenerate requirements./manage.py makemigrations
has been runPost merge:
(so basically once your build finishes, after maybe a minute you should see the new version in PyPi automatically (on refresh))
make upgrade
in course-discovery will look for the latest version in PyPi.