feat: add ability to disable cache #23439
Conversation
villebro
force-pushed
the
villebro/bypass-cache
branch
from
47a1535
to
8a43a2f
Compare
Codecov Report
@@ Coverage Diff @@
## master #23439 +/- ##
=======================================
Coverage 67.63% 67.63%
=======================================
Files 1908 1908
Lines 73716 73731 +15
Branches 7989 7990 +1
=======================================
+ Hits 49856 49871 +15
+ Misses 21812 21811 -1
- Partials 2048 2049 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -101,6 +101,10 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({ | |||
| postPayload: { | |||
| data: { | |||
| ...currentDatasource, | |||
| cache_timeout: | |||
There was a problem hiding this comment.
Can you also add this logic to the server-side endpoint to make sure this treatment is applied when invoking the API directly?
There was a problem hiding this comment.
Currently the Datasource modal is unfortunately using the legacy endpoint, which is why this check isn't happening in the API. If it were in fact using the V1 API, the old request would produce an error, as it does not satisfy the Marshmallow schema.
@dpgaspar Oh, I thought 0 expiry on Redis made it never expire (that's what we're also claiming in one of the modals). Let me read up on docs + do testing and come back with a revised approach. |
|
your right, sorry, got confused because of: https://redis.io/commands/ttl/ TTL = -1 when a key has no associated TTL. |
mistercrunch
added
🏷️ bot
SUMMARY
Currently it's only possible to bypass the chart data cache by adding the
force=truequery param if a chart cache has been defined. This can be problematic if one wants to eitherThis PR adds support for a
-1chart data timeout which is interpreted similarly toforce=true, and is implemented both in the V1 chart data endpoint andviz.py. A test is also added to the chart data test suite. Note that the timeout value0is already reserved and ambiguous, and its behavior is different depending on the cache backend: sometimes is means "never expire", and other times "expire instantly" (I was not able to find a documentation reference to this, but I remember seeing it in the Flask-Caching source code while working on something else).AFTER
By setting the cache timeout to -1 the cache is bypassed similar to what happens if making the request with
force=true:cache_disabled.mp4
Note, that the value is still persisted in the cache if a cache is defined. This is to ensure that Global Async Queries will still work, and that other chart data requests that don't want to bypass the cache are able to retrieve the cached data (if available).
BEFORE
Previously it was not possible to disable the cache if
CHART_DATA_CACHEwas defined (at a minimum, it was possible to set it to 1 second, which almost did the same):cache_enabled.mp4
In addition, if the "cache timeout" value was removed in the dataset modal, it would get saved as an empty string, which caused a 500 in the backend when trying to reopen the modal (it expects either a
nullor a number). To make sure that doesn't happen, we change the value tonullif the value is an empty string when the dataset is saved.TESTING INSTRUCTIONS
ADDITIONAL INFORMATION