gabotorresruiz commented on PR #44082: URL: https://github.com/apache/superset/pull/44082#issuecomment-5638725796
Thanks for putting this together, Enzo, and for the clear write up. Took a pass at the approach and it looks solid: the backend choosing the path from storage config, the frontend handling either a file `200` or an async job `202`, the `EXCEL_EXPORT_SYNC_MAX_ROWS` budget checked before any query runs, images staying async, and the shared `workbook.py` builder. It lines up well with the `EXPORT_STORAGE` work in #43805. I agree with the sequencing you proposed: let's land #43805 first and then rebase this on top. This PR restructures `export_dashboard_excel.py` and the export endpoint and `DownloadMenuItems` frontend that #43805 also rewrites, so rebasing after keeps us from a large conflict on the approved change and lets both paths share one workbook builder. I'll do a full review right after the rebase. Two things that will shift in that rebase: the availability check moves from `EXCEL_EXPORT_S3_BUCKET` to `EXPORT_STORAGE` (both `bucket` and `backend`), and the guest restriction is worth revisiting, since #43805 brings guest authentication, token revocation, and the streaming download path that the sync flow can reuse. -- 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]
