-
Notifications
You must be signed in to change notification settings - Fork 3
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: dont cache dataset metadata #177
Conversation
Codecov Report
@@ Coverage Diff @@
## main #177 +/- ##
=========================================
+ Coverage 0 71.71% +71.71%
=========================================
Files 0 126 +126
Lines 0 10098 +10098
=========================================
+ Hits 0 7242 +7242
- Misses 0 2856 +2856
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
server/app/app.py
Outdated
@@ -232,7 +232,7 @@ def get(self, data_adaptor): | |||
|
|||
|
|||
class DatasetMetadataAPI(DatasetResource): | |||
@cache_control(public=True, max_age=ONE_WEEK) | |||
@cache_control(public=True, no_store=True, max_age=0) |
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.
Don't we want to eliminate public
as well? I would think this is in conflict with no_store
. Though public
should be ignored, considering no other directives have an effect when used with no-store
(see below).
public: The response may be stored by any cache, even if the response is normally non-cacheable.
no-store: The response may not be stored in any cache. Note that this does not prevent a valid pre-existing cached response being returned. Clients can set max-age=0 to also clear existing cache responses, as this forces the cache to revalidate with the server (no other directives have an effect when used with no-store).
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.
Where did you find this 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.
woops, sorry, usually try to cite my sources! https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#cacheability
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.
That's a good policy to have. Thank you!
* main: Delete schema_guide.md (#131) chore(deps): bump urllib3 from 1.25.9 to 1.26.5 in /hosted (#186) chore(deps): bump jinja2 from 2.11.2 to 2.11.3 in /hosted (#185) fix: dont cache dataset metadata (#177) explorer rdev (#144) Updated load indicator to match skeleton. (#139) (#171) Always show scrollbar on dataset menu if scrollable. (#138) (#173) fix: e2e base url (#179) # Conflicts: # hosted/requirements.txt # server/app/app.py # server/tests/unit/common/apis/test_api_v2.py
https://app.zenhub.com/workspaces/single-cell-5e2a191dad828d52cc78b028/issues/chanzuckerberg/single-cell-explorer/166
Reviewers
Functional:
Readability:
Changes