openedx
fix: caching was removed
Description
Caching is removed to represent an actual count of learners.
Related: open-release/quince.master
Thanks for the pull request, @Inferato! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
Thanks for fixing this issue. Now, I'd like to know the performance impact this could have in the long run. Have you folks tested this with a more significant volume of users? Can the caching strategy time be set up to be lowered instead of removing it altogether?
@mphilbrick211 I want to update and finalize the PR, but I have a problem with modifying this PR.
@mariajgrimaldi You're right, we have checked it on courses with around 1000 users, and removing this caching might affect the performance on the page, we decided to just set the default caching to 10 minutes (600 seconds).
I will reopen the same change in a new PR.
@mariajgrimaldi do you think opening PR against quince.master is still necessary since there won't be quince.4?
@GlugovGrGlib: As you mentioned, if merged to quince.master, it won't be included in a new release. But folks could still use quince.master branch. If that works for you, I could review the PR. Let me know!
@Inferato Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.