gabotorresruiz commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r3982197652
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -486,28 +635,34 @@ def export_dashboard_excel(
dashboard_title=dashboard_title,
download_url=download_url,
requested_at=requested_at,
- expires_at=expires_at,
+ # Stored naive-local to match is_expired(); the
email
+ # labels its timestamps "UTC", so convert for
display.
+ expires_at=expires_at.astimezone(timezone.utc),
ttl_seconds=ttl,
errored=errored,
),
)
except Exception: # pylint: disable=broad-except
- # The file is already in S3; a send failure should not
trigger
+ # The file is already uploaded; a send failure should not
trigger
# a misleading failure email.
logger.exception("Failed to send export success email")
except SoftTimeLimitExceeded:
logger.warning("Dashboard excel export %s timed out", job_id)
- _send_failure_email(user, dashboard_title, requested_at)
+ _handle_export_failure(user, dashboard_title, requested_at, job_id,
ttl)
raise
except Exception:
logger.exception("Dashboard excel export %s failed", job_id)
- _send_failure_email(user, dashboard_title, requested_at)
+ _handle_export_failure(user, dashboard_title, requested_at, job_id,
ttl)
raise
finally:
try:
ReleaseDistributedLock(
Review Comment:
Fixed in 3d76c017d9. The API now threads its acquisition token into the
task, and the task releases with token=lock_token, so the release is master's
compare-and-delete: a TTL-expired lock reacquired by another export is left
untouched instead of blindly deleted. The enqueue-failure release path passes
the token too. Covered by unit and integration tests.
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -167,6 +210,121 @@ export const useDownloadMenuItems = (
}
};
+ const triggerExportDownload = async (downloadUrl: string) => {
+ // Fetch the file and save it via an anchor instead of navigating: a link
+ // whose object is already gone answers a JSON error, and a navigation
+ // would replace the dashboard (fatal inside an embedded iframe) instead
+ // of surfacing a retryable toast.
+ const response = await SupersetClient.get({
+ endpoint: downloadUrl,
+ parseMethod: 'raw',
+ });
+ const disposition = response.headers.get('Content-Disposition');
+ let fileName = 'dashboard_export.xlsx';
+ if (disposition) {
+ try {
+ const parsed = parseContentDisposition(disposition);
+ if (parsed?.parameters?.filename) {
+ fileName = parsed.parameters.filename;
+ }
+ } catch (error) {
+ logging.warn('Failed to parse Content-Disposition header:', error);
+ }
+ }
+ const blob = await response.blob();
Review Comment:
Fixed in 3d76c017d9. The ready download now streams through a hidden iframe:
the endpoint's Content-Disposition attachment makes the browser save straight
to disk, so the whole workbook is never buffered in tab memory, and the
dashboard is never navigated away (safe in an embed). The status endpoint
already validated the link before ready, so the only residual case is the
narrow race where the object is removed after that check, which loads invisibly
in the iframe and leaves the page untouched.
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -167,6 +210,121 @@ export const useDownloadMenuItems = (
}
};
+ const triggerExportDownload = async (downloadUrl: string) => {
+ // Fetch the file and save it via an anchor instead of navigating: a link
+ // whose object is already gone answers a JSON error, and a navigation
+ // would replace the dashboard (fatal inside an embedded iframe) instead
+ // of surfacing a retryable toast.
+ const response = await SupersetClient.get({
+ endpoint: downloadUrl,
+ parseMethod: 'raw',
+ });
+ const disposition = response.headers.get('Content-Disposition');
+ let fileName = 'dashboard_export.xlsx';
+ if (disposition) {
+ try {
+ const parsed = parseContentDisposition(disposition);
+ if (parsed?.parameters?.filename) {
+ fileName = parsed.parameters.filename;
+ }
+ } catch (error) {
+ logging.warn('Failed to parse Content-Disposition header:', error);
+ }
+ }
+ const blob = await response.blob();
+ const url = window.URL.createObjectURL(blob);
+ try {
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = fileName;
+ a.style.display = 'none';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ } finally {
+ window.URL.revokeObjectURL(url);
+ }
+ };
+
+ const pollExportStatus = (jobId: string, pollState: ExportPollState) => {
+ if (unmountedRef.current) {
+ return;
+ }
+ SupersetClient.get({
+ endpoint: `/api/v1/dashboard/export_xlsx/status/${jobId}/`,
+ })
+ .then(({ json }) => {
+ // A response in flight when the component unmounts must not navigate
+ // (redirect) or toast on whatever page the user moved to.
+ if (unmountedRef.current) {
+ return;
+ }
+ const {
+ status,
+ download_url: downloadUrl,
+ message,
+ } = json as ExportStatusResponse;
+ if (status === 'ready') {
+ if (downloadUrl) {
+ triggerExportDownload(downloadUrl)
+ .then(() =>
+ addSuccessToast(t('Your export is ready and downloading.')),
+ )
+ .catch(error => {
+ logging.error(error);
+ addDangerToast(
+ t(
+ 'Your export could not be downloaded. It may have expired;
please export again.',
+ ),
+ );
+ });
+ } else {
+ addDangerToast(t('Sorry, something went wrong. Try again later.'));
+ }
+ return;
+ }
+ if (status === 'error') {
+ addDangerToast(
+ message || t('Sorry, something went wrong. Try again later.'),
+ );
+ return;
+ }
+ if (status === 'running' && !pollState.sawRunning) {
+ // The task's execution budget only starts when a worker picks it
+ // up; restart the wait window then, so queue delay doesn't eat it.
+ pollState.sawRunning = true;
+ pollState.deadline = Date.now() + EXPORT_STATUS_POLL_TIMEOUT_MS;
+ }
+ if (Date.now() > pollState.deadline) {
+ addDangerToast(
+ t('Your export is taking longer than expected. Try again later.'),
+ );
+ return;
+ }
+ addExportPendingToast();
Review Comment:
Fixed in 3d76c017d9. The pending toast is now announced once at enqueue and
no longer re-emitted on every poll, so a long export no longer spawns a fresh
role=alert every few seconds. Added a test asserting a single info toast across
many poll cycles.
--
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]
