-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: block_structure.storage_backing_for_cache toggle removed depr32 #33151
Conversation
Thanks for the pull request, @Yagnesh1998! 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:
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. |
Hi @Yagnesh1998! Just flagging that there are some failing tests here. |
@mphilbrick211 I am working on it. thanks |
Friendly ping on this @Yagnesh1998! |
@dianakhuang all test are passed successfully let me know if we need to do some changes. |
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.
I think I just had one nitpick, but other than that, I think this looks good.
@@ -228,12 +217,7 @@ def _encode_root_cache_key(bs_model): | |||
Returns the cache key to use for the given | |||
BlockStructureModel or StubModel. |
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.
nit: this no longer returns a StubModel, so we should update this docstring.
@Yagnesh1998 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. |
We close this PR and create another PR according to the multiple commits in this PR. |
Description
As Per this DEPR: openedx/public-engineering#32
Removing: Toggle labeled 'block_structure.storage_backing_for_cache'. The toggle has the following annotation as a description:
When enabled, block structures are stored in a more permanent storage, like a database, which provides an additional backup for cache misses, instead having them regenerated. The regeneration of block structures is a time consuming process. Therefore, enabling this switch is recommended for Production.
Supporting information
as per the original jeera issue https://openedx.atlassian.net/browse/DEPR-145, this toggle is removed