gabotorresruiz commented on PR #43805: URL: https://github.com/apache/superset/pull/43805#issuecomment-5529074302
Thank you Elizabeth, outstanding review. All three blockers plus the minors are addressed in 7bb2ce6f40: Blockers: expiries are now naive local time at all three sites (with the two lying comments corrected); the polled download_url is origin relative with APPLICATION_ROOT preserved and only the email builds an absolute URL via the user friendly base; guests get a stable lock slot derived from the token identity (username plus resources fingerprint), mirrored on the worker release side, so concurrent guests no longer collide on slot 0. On the explicit decision: clamping to the token exp would break guest exports outright, since embed tokens routinely expire in about five minutes while the task can run ten. What I did instead: guest initiated links are clamped to one hour (they retrieve the file through the polling window and have no email to revisit a link later), and the possession model keeps its documented rationale. Happy to tighten further if you disagree. Minors: failure status is recorded before the failure email; status never reports ready for a backend mismatched link (and reports error rather than polling forever); ready with no download_url is an error toast, not a fake success; poll timers are cancelled on unmount; S3 closes the body on abandoned downloads; SDK install hints fire before the exceptions import; the key_prefix contract now says task context; the CSRF exemption carries its rationale; and the signing era comments are swept, including dropping the impossible legacy record claim. Docs gained the web tier SDK note and s3:ListBucket. One respectful decline: mapping S3 AccessDenied to 410 would disguise genuine misconfiguration as an expired link, so I kept the NoSuchKey/404 mapping and documented ListBucket as the way to get clean 410s under least privilege. New coverage: guest slot determinism, guest TTL clamp, naive local expiry assertion, partial failure summary sheet, S3 body close, status mismatch error, ready without URL, and unmount cancellation. 79 unit, 26 integration, 26 jest, all green. -- 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]
