gabotorresruiz commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r4065641615
##########
pyproject.toml:
##########
@@ -192,10 +192,17 @@ elasticsearch = ["elasticsearch-dbapi>=0.2.13, <0.3.0"]
# SQLAlchemy 2.0 core bump has landed (discussion #40273).
exasol = ["sqlalchemy-exasol>=6.0.0, <8.0"]
excel = ["xlrd>=2.0.2, <2.1"]
-# Async dashboard "Export Data/Images to Excel": uploads the workbook to S3 and
-# emails a pre-signed link. boto3 is imported lazily by superset.utils.s3, so
-# installing this extra is only required to actually run exports.
+# S3 storage backend for the async dashboard "Export Data/Images to Excel"
+# feature (EXPORT_STORAGE["backend"] = S3ExportStorage()). boto3 is
+# imported lazily by superset.utils.s3, so installing this extra is only
+# required to actually run exports.
excel-export = ["boto3"]
+# Google Cloud Storage backend for the same feature (EXPORT_STORAGE
+# ["backend"] = GCSExportStorage()), for a deployment whose export bucket is a
+# native GCS bucket rather than S3. google-cloud-storage is imported lazily by
+# superset.utils.gcs, so this extra is an alternative to excel-export, not an
+# addition to it -- pick whichever matches your bucket's provider.
+excel-export-gcs = ["google-cloud-storage"]
Review Comment:
Good catch, fixed in fe329085289a08cf746fa931b991ffb38fa3ebfc: the extra now
pins `google-cloud-storage>=1.37`.
I checked the boundary against the published wheels rather than the release
notes alone. 1.36.2 carries neither `Blob.open` nor
`google/cloud/storage/fileio.py` at all, and 1.37.0 has `open(self, mode="r",
chunk_size=None, encoding=None, errors=None, newline=None, **kwargs)`, which is
the exact call `superset.utils.gcs` makes. I verified `chunk_size`
specifically, since we pass it, and `BlobReader.__init__` accepts it at that
version too.
You are right about the failure mode as well: a fresh install resolves to
3.x, so this only bites where the version is already constrained lower, and
then it installs cleanly, validates its configuration, and raises on every
download. Silent until someone clicks.
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -175,16 +313,21 @@ export const useDownloadMenuItems = (
});
// The throttle response (an export is already running) returns 202 with
a
// message but no job_id; only a freshly enqueued job carries a job_id.
- if ((json as { job_id?: string })?.job_id) {
- addSuccessToast(
- t(
- "Your export is being prepared. You'll receive an email when it's
ready.",
- ),
+ const jobId = (json as { job_id?: string })?.job_id;
+ if (jobId) {
Review Comment:
Fixed in 2e3142ff49e083154e619e837f8504bf54081830.
The continuation now bails on `unmountedRef` as soon as the POST settles,
which is the same check `pollExportStatus` and its `.then` already make. I put
the same guard on the error path too, so a failed export cannot toast onto an
unrelated page either.
One refinement to what the guard actually buys, since I traced it: the timer
is not an unbounded leak, because `pollExportStatus` returns early on
`unmountedRef`, so it fires once and no-ops. And the export staying unretrieved
follows from unmounting itself rather than from this path, so the early return
does not rescue it. What it does remove is `addExportPendingToast` firing onto
whatever page the user navigated to, plus the untracked timer.
Covered by `unmounting while the export POST is in flight suppresses its
follow-up`, which resolves the POST after `unmount()` and asserts no toast and
no polling. It fails without the guard.
--
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]
