rusackas commented on PR #42561: URL: https://github.com/apache/superset/pull/42561#issuecomment-5187493272
This is a solid improvement over what folks are doing today (globally disabling `DASHBOARD_VIRTUALIZATION`, which breaks more than just export). Couple of non-blocking follow-ups worth tracking though. `downloadUtils.test.ts` covers the dispatch/timeout logic well, but `Row.tsx` is where the actual work happens, tearing down and re-attaching the observers on these events, and it has no test coverage for that. Also, `forceLoadAllCharts` puts every row in view at once, no batching. Fine for tens of charts, but on a dashboard with hundreds it's the same thundering-herd load virtualization exists to prevent, just moved to export time. I'll open a follow-up PR for both, plus maybe some way to signal a large export is in progress. Thanks for chasing this down, @jenwitteng! -- 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]
