fitzee commented on PR #42253: URL: https://github.com/apache/superset/pull/42253#issuecomment-5082171416
Thanks for the detailed review @aminghadersohi! The zero-holder warning-only behavior is intentional for this PR. A dashboard can legitimately contain no chart holders, for example, Markdown/dynamic-component-only dashboards; so treating zero holders as an unconditional readiness failure would make valid exports wait until the task budget expires and then fail. The warning makes the readiness-gate gap observable without introducing that regression. Fully closing the timing race requires a stronger signal that distinguishes “charts expected but not mounted yet” from “dashboard legitimately has zero charts,” likely using dashboard/layout metadata rather than DOM holder count alone. I’ll track that separately rather than broadening #42253. On the other CodeAnt threads: - Celery timelimit ordering: confirmed false positive. Celery provides (hard_limit, soft_limit), matching the current destructuring. - Tiled budget awareness: real but explicitly outside this PR’s budget scope; #42118 is the vehicle for that path. - Animation/capture budget awareness: also a valid follow-up, but this PR specifically bounds readiness and preserves cleanup time. - So no further code changes are proposed for #42253 from these threads. -- 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]
