codeant-ai-for-open-source[bot] commented on code in PR #41133:
URL: https://github.com/apache/superset/pull/41133#discussion_r3615693760
##########
superset/dashboards/api.py:
##########
@@ -1575,6 +1595,132 @@ def export_as_example(self, pk: int) -> Response:
response.set_cookie(token, "done", max_age=600)
return response
+ @expose("/<pk>/export_xlsx/", methods=("POST",))
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.export_xlsx",
+ log_to_statsd=False,
+ )
+ def export_xlsx(self, pk: int) -> WerkzeugResponse:
+ """Export all of a dashboard's chart data to an Excel workbook (async).
+ ---
+ post:
+ summary: Export dashboard chart data to Excel
+ description: >-
+ Enqueues an async task that writes each chart's data to its own
+ worksheet, uploads the .xlsx to S3, and emails the requesting user
a
+ pre-signed download link. Returns immediately with a job id.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The dashboard id
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/DashboardExportXlsxPostSchema'
+ responses:
+ 202:
+ description: Export task accepted
+ content:
+ application/json:
+ schema:
+ $ref:
'#/components/schemas/DashboardExportXlsxResponseSchema'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/403'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ 501:
+ description: Excel export is not configured on this server
+ """
+ if not current_app.config["EXCEL_EXPORT_S3_BUCKET"]:
+ return self.response(
+ 501, message="Excel export is not configured on this server."
+ )
+ try:
+ # Tolerate an empty/non-JSON body (e.g. a POST with no
Content-Type);
+ # request.json would otherwise raise 415.
+ payload = DashboardExportXlsxPostSchema().load(
+ request.get_json(silent=True) or {}
+ )
+ except ValidationError as error:
+ return self.response_400(message=error.messages)
+
+ # Image export drives the headless webdriver, so it is only available
+ # when the same screenshot flags the UI checks are enabled. The
decorator
+ # form (``@validate_feature_flags``) can't be used here because it
would
+ # also block ``mode="data"``; mirror its 404 behavior inline instead.
+ if payload.get("mode") == "images" and not (
+ is_feature_enabled("ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS")
+ and
is_feature_enabled("ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT")
+ ):
+ return self.response_404()
+
+ dashboard = cast(Dashboard, self.datamodel.get(pk, self._base_filters))
+ if not dashboard:
+ return self.response_404()
+ try:
+ security_manager.raise_for_access(dashboard=dashboard)
+ except SupersetSecurityException:
+ return self.response_403()
+
+ # Email delivery is the only result channel, so an account with an
email
+ # address is required; embedded guest users are excluded in this
version.
+ if isinstance(g.user, GuestUser) or not getattr(g.user, "email", None):
+ return self.response_400(
+ message="Excel export requires an account with an email
address."
+ )
+ if not dashboard.slices:
+ return self.response_400(message="Dashboard has no charts to
export.")
+
+ # Throttle: one concurrent export per user+dashboard. Acquire a shared,
+ # atomic distributed lock (Redis when configured, the metadata DB
+ # 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)
+ try:
+ AcquireDistributedLock(
+ EXPORT_LOCK_NAMESPACE,
+ lock_params,
+ ttl_seconds=EXPORT_LOCK_TTL_SECONDS,
+ ).run()
+ except LockAlreadyHeldException:
+ return self.response(
+ 202,
+ message="An Excel export for this dashboard is already in
progress.",
+ )
+
+ job_id = str(uuid.uuid4())
+ try:
Review Comment:
**Suggestion:** The lock-held branch returns HTTP 202 without a `job_id`,
but the documented 202 response schema requires a `job_id`; clients that parse
successful responses by schema can fail on this path. Return a status/body
shape consistent with the contract (for example include the existing job id if
available, or use a different status code/schema for “already running”).
[possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Lock-throttle 202 response breaks OpenAPI response contract.
- ⚠️ API clients may mis-handle already-running export responses.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with this PR code and configure `EXCEL_EXPORT_S3_BUCKET` in
`superset/config.py` so `export_xlsx` does not return 501 (see check at
`superset/dashboards/api.py:1646`).
2. As a user with Dashboard `can_export` permission and an email address,
open a dashboard
with at least one chart so `dashboard.slices` is non-empty
(`superset/models/dashboard.py`, used at `superset/dashboards/api.py:1688`).
3. Trigger the Excel export twice for the same dashboard and user (e.g. via
the frontend
`onExportXlsx()` in
`superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:170`,
which
POSTs to `POST /api/v1/dashboard/<pk>/export_xlsx/` implemented at
`superset/dashboards/api.py:1599-1723`).
4. On the first request, `AcquireDistributedLock(...).run()` at
`superset/dashboards/api.py:1696-1701` succeeds and
`export_dashboard_excel.apply_async(...)` is called, then `self.response(202,
job_id=job_id)` is returned at `superset/dashboards/api.py:1723` matching
`DashboardExportXlsxResponseSchema` (declared in the OpenAPI block at
`superset/dashboards/api.py:1608-1633`).
5. While the Celery task is still running and the lock not yet released by
`export_dashboard_excel` (see lock namespace `EXPORT_LOCK_NAMESPACE`
imported at
`superset/dashboards/api.py:145-150` and used in the task), send another
POST to the same
endpoint.
6. On the second request, `AcquireDistributedLock(...).run()` raises
`LockAlreadyHeldException` (imported at `superset/dashboards/api.py:132`),
hitting the
`except LockAlreadyHeldException` branch at lines `1702-1705` which returns
`self.response(202, message="An Excel export for this dashboard is already
in progress.")`
without a `job_id` field.
7. Any client that relies on the OpenAPI contract that 202 responses conform
to
`DashboardExportXlsxResponseSchema` (registered in
`openapi_spec_component_schemas` at
`superset/dashboards/api.py:512-520`) and expects a `job_id` will fail to
deserialize or
treat this successful 202 as schema-invalid.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=27139f58eed24da08c38a4960f5593fe&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=27139f58eed24da08c38a4960f5593fe&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/dashboards/api.py
**Line:** 1702:1705
**Comment:**
*Possible Bug: The lock-held branch returns HTTP 202 without a
`job_id`, but the documented 202 response schema requires a `job_id`; clients
that parse successful responses by schema can fail on this path. Return a
status/body shape consistent with the contract (for example include the
existing job id if available, or use a different status code/schema for
“already running”).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=4baeb9ca4ce0faf3d46a41c5d4e29cdc301d3b08c079c5105aa409ab3d864303&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=4baeb9ca4ce0faf3d46a41c5d4e29cdc301d3b08c079c5105aa409ab3d864303&reaction=dislike'>👎</a>
--
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]
