rusackas commented on PR #44499: URL: https://github.com/apache/superset/pull/44499#issuecomment-5770014844
Hey @seunggabi, thanks for this one. Nice, well-scoped opt-in feature, and I like that the reason rides along in the existing event log instead of a new table. However, it looks like `_download_reason()` in `report/execute.py` gets tacked onto the URL for scheduled csv/xlsx reports without checking `REQUIRE_DOWNLOAD_REASON` first, so the generated URL changes even with the flag off. That's what's breaking `test_get_url_for_xlsx_report` (and its csv sibling) in CI. Also needs a rebase, since `master`'s moved on. The bot threads look genuinely addressed from the commits, just never got flipped to resolved in the UI, so nothing to chase there. Happy to take another look once CI's green and that's sorted! -- 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]
