aminghadersohi opened a new pull request, #42252:
URL: https://github.com/apache/superset/pull/42252
### SUMMARY
`QueryCacheManager.get()` reads from the configured cache backend with:
```python
if cache_value := _cache[region].get(key):
```
This read is not wrapped in any error handling. If the cache backend (e.g.
Redis) raises a connection, timeout, or other backend-specific error on read,
the exception propagates all the way up through the chart/dashboard data API
(`POST /api/v1/chart/data`) and surfaces as an unhandled 500, instead of
gracefully falling back to querying live data — which is exactly what already
happens one branch up on a normal cache miss (`if not key or not _cache[region]
or force_query: return query_cache`).
In other words: any transient cache backend hiccup — not just a full outage
— turns into hard failures for chart and dashboard rendering, even though the
data can still be queried live.
This PR wraps the cache read in a try/except, logs the failure at a level
appropriate for a potentially noisy/flaky backend, and falls through to the
existing cache-miss path so the behavior matches a normal cache miss rather
than raising.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend error-handling change, no UI impact.
### TESTING INSTRUCTIONS
- Added `tests/unit_tests/common/test_query_cache_manager.py`:
- `test_get_cache_miss` — baseline: a normal cache miss returns a
non-loaded `QueryCacheManager`.
- `test_get_cache_backend_error_fails_open` — mocks the cache backend's
`.get()` to raise a `ConnectionError`, and asserts `QueryCacheManager.get()`
returns the same result as a normal cache miss instead of propagating the
exception.
- Manual repro: point `CACHE_CONFIG`/`DATA_CACHE_CONFIG` at a Redis
instance, then stop/block Redis while calling `POST /api/v1/chart/data` for a
datasource with caching enabled. Before this change: request fails with a 500.
After this change: request falls back to querying live data.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
