justinpark opened a new pull request, #43349: URL: https://github.com/apache/superset/pull/43349
### SUMMARY `get_chart_data`'s docstring advertises `Cache control: use_cache, force_refresh, cache_timeout`, but only `force_refresh` was actually wired into the `QueryContext`'s `force` flag: - `use_cache=False` was silently ignored — the tool would still read from cache even when a caller explicitly asked it not to. - `cache_timeout` was never applied anywhere, despite being a documented, logged request field. This fixes both by: - Computing `effective_force = force_refresh or not use_cache` once per call path, and using it everywhere the tool builds or patches a `QueryContext` (the cached-form_data path, the no-saved-query_context fallback path, the saved-`query_context` patch path, and the unsaved-chart-only `_query_from_form_data` path), instead of `force=request.force_refresh`. - Threading `cache_timeout` through as `QueryContextFactory`'s existing `custom_cache_timeout` parameter (already supported by `QueryContext`/`QueryContextFactory` and by `ChartDataQueryContextSchema` as a top-level field) — added as a new optional param on `build_query_context_from_form_data` in `chart_helpers.py` and passed through at every call site. No behavior changes for existing callers that don't set `use_cache`/`cache_timeout`: defaults are `use_cache=True`, `cache_timeout=None`, so `effective_force` reduces to `force_refresh` as before. ### TESTING INSTRUCTIONS - Added `test_query_from_form_data_use_cache_false_bypasses_cache` (parametrized over `use_cache`/`force_refresh` combinations) to `tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py`, asserting the `force` and `custom_cache_timeout` kwargs passed to `build_query_context_from_form_data`. - `ruff check` / `ruff format --check` / `python -m py_compile` pass on all changed files. - Could not run the full unit test suite in this environment due to a pre-existing, unrelated local SQLAlchemy/Flask-AppBuilder init issue (`Connection.rollback()` missing) that reproduces identically on `master` before this change. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] 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]
