gabotorresruiz commented on code in PR #43340:
URL: https://github.com/apache/superset/pull/43340#discussion_r3867584510
##########
superset/dashboards/excel_export/download_link.py:
##########
@@ -98,23 +99,38 @@ def build_download_url(job_id: UUID) -> str:
def create_download_link(
- job_id: UUID, bucket: str, key: str, expires_at: datetime
+ job_id: UUID, bucket: str, key: str, expires_at: datetime, backend: str
) -> str:
"""Record that ``job_id``'s export succeeded and is downloadable from
``key`` in ``bucket`` until ``expires_at``, and return the download URL
(used in the success email).
+ ``backend`` is the dotted path of the ``ExportStorage`` class that
+ uploaded the file; the download redirect refuses to sign with a different
+ backend (see ``download_xlsx``), failing clearly after a storage migration
+ instead of minting a URL for the wrong provider.
+
``expires_at`` should be a naive datetime in the same timezone convention
``KeyValueEntry.is_expired()`` compares against (naive ``datetime.now()``).
"""
_sweep_and_upsert(
job_id,
- {"status": STATUS_READY, "bucket": bucket, "key": key},
+ {"status": STATUS_READY, "bucket": bucket, "key": key, "backend":
backend},
expires_at,
)
return build_download_url(job_id)
+def mark_export_running(job_id: UUID, expires_at: datetime) -> None:
+ """Record that a worker has started executing ``job_id`` (as opposed to
+ still sitting in the queue), so a polling client can wait out broker
+ backlog without racing the task's execution budget, which only starts
+ here. Overwritten by the terminal record; ``expires_at`` is the backstop
+ if the worker dies first.
+ """
+ _sweep_and_upsert(job_id, {"status": STATUS_RUNNING}, expires_at)
Review Comment:
Confirmed and fixed in 2739f7e1fb. I verified it empirically first: write,
session remove, fresh read returned None. _sweep_and_upsert is now transaction
wrapped, which also makes the ready and error records durable at write time
instead of riding the lock release commit in the task's finally. The status
integration test now does a session remove with no explicit commit before
reading through the API.
##########
superset/dashboards/api.py:
##########
@@ -1795,6 +1801,14 @@ def export_xlsx(self, pk: int) -> WerkzeugResponse:
):
return self.response_404()
+ # The webdriver cannot render under a guest identity: the export would
+ # hold the shared guest lock for its whole budget and produce nothing.
+ # The UI hides the option for guests, but hiding is not enforcement.
+ if payload.get("mode") == "images" and
security_manager.is_guest_user():
Review Comment:
Fixed in 2739f7e1fb: the predicate is now get_user_id() is None, covering
anonymous requesters as well as guests and matching the UI's gate exactly. Test
updated to patch get_user_id.
##########
superset/utils/gcs.py:
##########
@@ -0,0 +1,107 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+``ExportStorage`` implementation backed by Google Cloud Storage, for
+deployments where the export bucket is a native GCS bucket rather than S3.
+
+Set ``EXPORT_STORAGE["backend"] = GCSExportStorage()`` in
+``superset_config.py`` (vs ``superset.utils.s3.S3ExportStorage`` for an S3
+bucket). Authentication uses Application Default Credentials (a
+service account key, workload identity, etc.) via the standard
+``google-cloud-storage`` resolution chain -- there is no separate credential
+config here. Signed download URLs from token-only credentials (workload
+identity, GCE metadata, Cloud Run) are routed through the IAM signBlob API,
+which requires ``roles/iam.serviceAccountTokenCreator`` on the service
+account itself.
+"""
+
+from __future__ import annotations
+
+from datetime import timedelta
+from typing import Any
+
+
+def _get_client() -> Any:
+ """Build a GCS client using Application Default Credentials."""
+ # Imported lazily, mirroring superset.utils.s3.S3ExportStorage: importing
+ # this module (which happens at config-load time if EXPORT_STORAGE
+ # is set) should not require google-cloud-storage unless an export
+ # actually runs.
+ try:
+ from google.cloud import storage # pylint:
disable=import-outside-toplevel
+ except ImportError as ex:
+ raise ImportError(
+ "google-cloud-storage is required for GCSExportStorage but is not "
+ "installed. Install it with "
+ "`pip install apache-superset[excel-export-gcs]`."
+ ) from ex
+
+ return storage.Client()
+
+
+class GCSExportStorage:
+ """``ExportStorage`` backed by Google Cloud Storage.
+
+ See ``superset.utils.export_storage.ExportStorage`` for the interface this
+ implements.
+ """
+
+ def upload_file(self, local_path: str, bucket: str, key: str) -> None:
+ """
+ Upload a local file to GCS.
+
+ :param local_path: Path to the file on local disk
+ :param bucket: Destination GCS bucket
+ :param key: Destination GCS blob name
+ """
+ _get_client().bucket(bucket).blob(key).upload_from_filename(local_path)
+
+ def generate_download_url(self, bucket: str, key: str, expires_in: int) ->
str:
+ """
+ Generate a time-limited signed URL for downloading a GCS object.
+
+ Token-only Application Default Credentials (GKE workload identity, GCE
+ metadata, Cloud Run) carry no private key, so local V4 signing raises.
+ For those, route the signature through the IAM signBlob API by passing
+ ``service_account_email`` and ``access_token``; the service account
+ needs ``roles/iam.serviceAccountTokenCreator`` on itself.
+
+ :param bucket: The GCS bucket
+ :param key: The GCS blob name
+ :param expires_in: URL lifetime in seconds
+ :returns: A v4 signed URL
+ """
+ # pylint: disable=import-outside-toplevel
+ import google.auth
+ from google.auth import credentials as auth_credentials
+ from google.auth.transport import requests as auth_requests
+
+ blob = _get_client().bucket(bucket).blob(key)
+ signing_kwargs: dict[str, Any] = {}
+ credentials, _ = google.auth.default()
Review Comment:
Fixed in 2739f7e1fb: the signing path now calls google.auth.default with an
explicit cloud-platform scope, so external account and workload identity
credentials can refresh for signBlob.
##########
UPDATING.md:
##########
@@ -274,19 +274,32 @@ Note that a retried query returns partial data with no
truncation indicator
(e.g. a filter dropdown may list only a subset of values on tables above the
row cap).
-### Dashboard "Export Data to Excel" requires a Celery worker and S3 bucket
+### Dashboard "Export Data to Excel" requires a Celery worker and a storage
bucket
A new dashboard action exports every chart's data to a single multi-sheet
`.xlsx` asynchronously. It is disabled by default and turns on only when
-`EXCEL_EXPORT_S3_BUCKET` is set (the endpoint returns `501` otherwise). It also
-requires a running Celery worker and a configured SMTP transport, since the
task
-emails the requesting user a pre-signed download link. New config keys:
-`EXCEL_EXPORT_S3_BUCKET`, `EXCEL_EXPORT_S3_KEY_PREFIX`,
-`EXCEL_EXPORT_LINK_TTL_SECONDS`, `EXCEL_EXPORT_S3_CLIENT_KWARGS`,
+`EXPORT_STORAGE` is configured with both a `bucket` and a `backend` (the
+endpoint returns `501` otherwise) — there is no implicit storage default:
+
+```python
+from superset.utils.s3 import S3ExportStorage # or
superset.utils.gcs.GCSExportStorage
+
+EXPORT_STORAGE = {
+ "bucket": "my-export-bucket",
+ "backend": S3ExportStorage(),
+}
+```
+
+It also requires a running Celery worker and a configured SMTP transport, since
Review Comment:
Fixed in 2739f7e1fb: UPDATING.md now documents SMTP as optional, used only
to additionally email logged in users, with polling as the delivery path for
every session.
--
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]
