rusackas commented on PR #37516: URL: https://github.com/apache/superset/pull/37516#issuecomment-4910185370
Thanks @YuriyKrasilnikov, the per-phase timing instrumentation is nice, and gating the response field behind `CHART_DATA_INCLUDE_TIMING` is the right call. One thing to sort before merge: `get_df_payload` always injects the internal `_chart_data_timing` sentinel into each query's payload (line 249), and only the chart-data path finalizes it. But `/api/v1/query/` (`views/api.py:75`) and the datasource samples endpoint call `get_payload()` and return the queries verbatim without finalizing, so that raw sentinel (perf_counter floats plus a half-built ms object) leaks into those two responses even with `CHART_DATA_INCLUDE_TIMING` off. That's a bit at odds with the 'opt-in, no default contract change' intent. Simplest fix is a `finalize_timing_payload(q)` on each query in those two consumers... it's idempotent, so it just strips the sentinel when the flag's off. Holler if you want me to push that. -- 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]
