-
Notifications
You must be signed in to change notification settings - Fork 212
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
Fix dynamically excluded providers caching #3381
Conversation
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.
Excellent find, and thank you for also improving the tests and documentation!
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.
LGTM!
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.
Well seen! LGTM!
@@ -21,7 +21,7 @@ | |||
def excluded_providers_cache(): | |||
cache_key = "filtered_providers" |
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.
This could be replaced with the new FILTERED_PROVIDERS_CACHE_KEY
constant from api.controllers.search_controller.
def excluded_providers_cache(): | ||
cache_key = "filtered_providers" | ||
excluded_provider = "excluded_provider" | ||
cache_value = [{"provider_identifier": excluded_provider}] | ||
cache_value = [excluded_provider] | ||
cache.set(cache_key, cache_value, timeout=1) |
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.
And the same here!
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Co-authored-by: sarayourfriend <[email protected]>
Co-authored-by: Krystle Salazar <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
415c74b
to
0ecd3e0
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
This reverts commit f37a0cd.
Fixes
Fixes #3380 by @obulat
Description
This PR saves a list of
provider_identifier
s for the dynamically-excluded providers to the redis cache, instead of saving the QuerySet.It also adds two unit tests for the specific functions, but there are also several other tests that test dynamically excluded providers using both cache and the
ContentProvider
model saved in the database.Testing Instructions
Run the app, go to
http://localhost:50280/admin
, select one of theContentProvider
models and check its "Hide Content" property.Execute a search (http://localhost:50280/v1/images). Check Redis value for
:1:filtered_providers
(I used the RedisInsights GUI):Also, check that on
main
, if you set "Hide content" to true for one of the providers, the value saved in Redis should look like the one in the related issue instead of a list of filtered providers.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin