gabotorresruiz commented on code in PR #43340:
URL: https://github.com/apache/superset/pull/43340#discussion_r3867585825
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -83,9 +92,27 @@ export const useDownloadMenuItems = (
canExportImage,
} = props;
- const { addDangerToast, addSuccessToast } = useToasts();
+ const { addDangerToast, addSuccessToast, addInfoToast } = useToasts();
const dataMask = useSelector((state: RootState) => state.dataMask);
- const SCREENSHOT_NODE_SELECTOR = ".dashboard";
+ const user = useSelector((state: RootState) => state.user);
+ // Guests and anonymous sessions have no userId; sessions the backend
+ // cannot email get neutral copy and a poll window outliving the task budget.
+ const isGuestSession = !user?.userId;
+ const canReceiveEmail = Boolean(user?.userId && user?.email);
Review Comment:
Fixed in 2739f7e1fb, taking the simplification: one 12 minute poll window
for every session, since the email is best effort (SMTP optional, send failures
swallowed) and cannot be treated as a fallback. Identity now only selects the
toast copy.
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3526,7 +3544,7 @@ def test_export_xlsx_admitted_with_can_export_only(
db.session.commit()
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
- @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @with_config({"EXPORT_STORAGE": {"bucket": "exports", "backend":
MagicMock()}})
@patch("superset.dashboards.api.AcquireDistributedLock")
@patch("superset.dashboards.api.export_dashboard_excel")
def test_export_xlsx_admitted_without_email(self, mock_task, mock_acquire):
Review Comment:
Added in 2739f7e1fb:
test_export_xlsx_guest_enqueues_with_token_and_no_user_id asserts the API
enqueues user_id None plus the guest token payload and acquires lock slot 0,
covering the API to worker handoff.
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -441,6 +453,59 @@ def _handle_export_failure(
logger.exception("Failed to record export failure status for %s",
job_id)
+def _resolve_export_storage(
+ dashboard_id: int, job_id: str
+) -> tuple[ExportStorage, str, str]:
+ """The configured storage backend, bucket, and this export's object key.
+
+ The API already rejects the request with 501 when either the bucket or
+ the backend is unset, so reaching this unconfigured normally means
+ EXPORT_STORAGE was cleared after the job was enqueued (or the task
+ was invoked directly, bypassing the API). Fail with a clear message
+ instead of an opaque storage-SDK error.
+ """
+ storage_config = current_app.config["EXPORT_STORAGE"]
+ bucket = storage_config.get("bucket")
+ storage_backend = storage_config.get("backend")
+ if not bucket or storage_backend is None:
+ raise SupersetException(
+ "Excel export is not configured on this server: "
+ "EXPORT_STORAGE needs both a 'bucket' and a 'backend' "
+ "(e.g. superset.utils.s3.S3ExportStorage())."
+ )
+ key_prefix = storage_config.get("key_prefix", "dashboard-exports/")
+ if callable(key_prefix):
+ # A callable prefix is resolved per export, for deployments where it
+ # is only known in task context (e.g. a multi-tenant installation
+ # scoping a shared bucket per tenant).
+ key_prefix = key_prefix()
+ return storage_backend, bucket, f"{key_prefix}{dashboard_id}/{job_id}.xlsx"
+
+
+def _mark_running(job_id: str) -> None:
+ """Tell pollers execution has begun (vs. queued); best-effort, the export
+ must not fail over a status write."""
+ try:
+ expires_at = datetime.now(tz=timezone.utc) + timedelta(
+ seconds=EXPORT_HARD_TIME_LIMIT + 300
+ )
+ mark_export_running(uuid.UUID(job_id), expires_at.replace(tzinfo=None))
+ except Exception: # pylint: disable=broad-except
+ logger.exception("Failed to record running status for %s", job_id)
+
+
+def _resolve_requesting_user(
+ user_id: int | None, guest_token: GuestToken | None
+) -> Any:
+ if user_id is not None:
+ return security_manager.get_user_by_id(user_id)
+ if guest_token:
+ return security_manager.get_guest_user_from_token(guest_token)
Review Comment:
Keeping as is, deliberately: this mirrors superset.tasks.async_queries,
which reconstructs the guest from the already validated payload. The access
decision is enforced at request time via raise_for_access, guest tokens are
short lived stateless JWTs with no server side revocation to consult, and
enforcing exp in the worker would fail every legitimately queued export since
tokens routinely expire within a normal queue wait. If delayed execution under
a stale token is a concern worth addressing, I think it belongs to both
consumers together as a follow up rather than this PR.
##########
superset/dashboards/api.py:
##########
@@ -1814,7 +1828,12 @@ def export_xlsx(self, pk: int) -> WerkzeugResponse:
# otherwise) so the guard works across the web server and workers and
is
# not a no-op under the default cache. The task releases it when it
# settles; the TTL is the backstop if that release is ever lost.
- lock_params = export_lock_params(g.user.id, dashboard.id)
+ # A guest/embedded requester has no DB-backed user id (GuestUser
carries
+ # no ``id`` attribute at all), so all guests share lock slot 0 for the
+ # dashboard; the task reconstructs the guest (with the token's RLS
rules
+ # and resource claims) from the token payload passed alongside.
+ user_id = get_user_id()
+ lock_params = export_lock_params(user_id or 0, dashboard.id)
try:
Review Comment:
Real but bounded, and I would defer it: the worst case is a second
concurrent export while the first finishes, and the throttle is best effort by
design. This base branch's release is an unconditioned delete with no extend
primitive, so renewing from the task would be a release plus acquire, which is
itself a race. Master already has ownership checked release (compare and
delete); the clean fix is lease extension plus owned release when this stack
rebases onto master, and I am happy to file that follow up.
--
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]
