villebro commented on issue #30816: URL: https://github.com/apache/superset/issues/30816#issuecomment-2460751601
I think the simplification makes sense. However, the issue I see with this approach is the fact that the request won't be rendered by the chart plugin, which was the intention of [SIP-6](https://github.com/apache/superset/issues/5692). While we do currently store the query context in the chart table as part of the chart metadata, this is in a sense a cached version of what the plugin rendered when the chart was saved. Relying on this cached value makes it impossible to roll out changes to a viz plugin's `buildQuery` that may have been introduced during a version upgrade, and could lead to dashboards breaking post upgrade. In fact, it can be argued that the introduction of the `query_context` field to the chart metadata was in fact technical debt that contradicts SIP-6. So as part of this SIP, I feel we should revisit that SIP first, and think carefully of how we can facilitate an architecture, where - viz plugins can remain decoupled from the backend (we're still wrestling with decommissioning `viz.py`, and I'd hate to see us go back there) - the backend can confidently pull up query contexts for any given chart without having to to these types of expensive duplicated API calls CC @michael-s-molina @rusackas as this touches on the plugin architecture discussion we've been having. -- 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]
