codeant-ai-for-open-source[bot] commented on code in PR #41133:
URL: https://github.com/apache/superset/pull/41133#discussion_r3545166764
##########
superset/config.py:
##########
@@ -1407,6 +1407,27 @@ def sync_theme_logo_href(
# note: index option should not be overridden
EXCEL_EXPORT: dict[str, Any] = {}
+# ---------------------------------------------------
+# Dashboard "Export Data to Excel" (async, S3-backed)
+# ---------------------------------------------------
+# Destination S3 bucket for generated dashboard .xlsx exports. The feature is
+# disabled until this is set: the export endpoint returns 501 when it is None.
+EXCEL_EXPORT_S3_BUCKET: str | None = None
+# Key prefix for export objects: {prefix}{dashboard_id}/{job_id}.xlsx
+EXCEL_EXPORT_S3_KEY_PREFIX = "dashboard-exports/"
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
configuration constant. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly added configuration constant in the new hunk and it has no
type annotation. The custom rule requires type hints for new or modified Python
variables that can be annotated, so the suggestion correctly identifies a real
violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=323c8ba3ecce419e8dc1ce66d0ef166d&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=323c8ba3ecce419e8dc1ce66d0ef166d&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/config.py
**Line:** 1417:1417
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
configuration constant.
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=a1dac1f919bea55cf7519a3917164b2e066cece8670c4e2d959c628d7c6b0470&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=a1dac1f919bea55cf7519a3917164b2e066cece8670c4e2d959c628d7c6b0470&reaction=dislike'>π</a>
##########
superset/config.py:
##########
@@ -1407,6 +1407,27 @@ def sync_theme_logo_href(
# note: index option should not be overridden
EXCEL_EXPORT: dict[str, Any] = {}
+# ---------------------------------------------------
+# Dashboard "Export Data to Excel" (async, S3-backed)
+# ---------------------------------------------------
+# Destination S3 bucket for generated dashboard .xlsx exports. The feature is
+# disabled until this is set: the export endpoint returns 501 when it is None.
+EXCEL_EXPORT_S3_BUCKET: str | None = None
+# Key prefix for export objects: {prefix}{dashboard_id}/{job_id}.xlsx
+EXCEL_EXPORT_S3_KEY_PREFIX = "dashboard-exports/"
+# Lifetime (seconds) of the pre-signed download URL emailed to the user (24h).
+# Note: AWS S3 caps pre-signed URL lifetime at 7 days (604800 seconds); larger
+# values are rejected by S3, so keep this at or below that when using AWS.
+EXCEL_EXPORT_LINK_TTL_SECONDS = 86400
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
numeric configuration constant. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is also a newly introduced configuration constant in the new hunk and
it lacks an explicit type hint. Since the rule requires type hints on relevant
new Python variables, this is a real violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ba06f3f7ccf342d589d59bdfdaa88bec&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=ba06f3f7ccf342d589d59bdfdaa88bec&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/config.py
**Line:** 1421:1421
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
numeric configuration constant.
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=cda17bafde7a151462fbb088d6710befdac2296040fd988a9f3b2b27e06c54ef&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=cda17bafde7a151462fbb088d6710befdac2296040fd988a9f3b2b27e06c54ef&reaction=dislike'>π</a>
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -0,0 +1,345 @@
+# 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
+#
Review Comment:
**Suggestion:** Add an explicit type annotation to this exported mode
constant since its type is known and stable. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This exported module constant has a clear static type and is unannotated,
which fits the custom rule requiring type hints on annotatable variables in new
or modified Python code.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b6fad055296b4cdb983ebf2055d51c81&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=b6fad055296b4cdb983ebf2055d51c81&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/tasks/export_dashboard_excel.py
**Line:** 8:8
**Comment:**
*Custom Rule: Add an explicit type annotation to this exported mode
constant since its type is known and stable.
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=c743b5534c17adb5f34593af84b78996df971b93e92ed0522158b5dc8e4276a7&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=c743b5534c17adb5f34593af84b78996df971b93e92ed0522158b5dc8e4276a7&reaction=dislike'>π</a>
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -0,0 +1,345 @@
+# 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
Review Comment:
**Suggestion:** Add an explicit type annotation to this module-level
constant to satisfy the type-hint requirement for annotatable variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This module-level constant is assignable a clear static type and is left
unannotated in the new code. That matches the type-hint rule for annotatable
variables, so the suggestion identifies a real violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f406ae83e9274f0997025f69a3f2543e&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=f406ae83e9274f0997025f69a3f2543e&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/tasks/export_dashboard_excel.py
**Line:** 13:13
**Comment:**
*Custom Rule: Add an explicit type annotation to this module-level
constant to satisfy the type-hint requirement for annotatable variables.
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=2c66de0359a58ad74992a21c4d73db960fbb25b14d1316a4bc2fc4be5e425602&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=2c66de0359a58ad74992a21c4d73db960fbb25b14d1316a4bc2fc4be5e425602&reaction=dislike'>π</a>
##########
superset/utils/excel_streaming.py:
##########
@@ -0,0 +1,254 @@
+# 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.
+"""
+Streaming XLSX writer for multi-sheet dashboard exports.
+
+Unlike :mod:`superset.utils.excel`, which builds an in-memory DataFrame per
+sheet and hands the whole thing to ``xlsxwriter`` at once, this writer opens
the
+workbook in ``constant_memory`` mode and writes rows one at a time, so
+``xlsxwriter`` keeps at most one row per sheet buffered on the writer side. The
+source records may still be materialized upstream (e.g. by the chart query
+response); this bounds only the writer's own footprint, not the caller's.
+"""
+
+from __future__ import annotations
+
+import math
+import numbers
+import re
+from collections.abc import Iterable, Sequence
+from datetime import date, datetime
+from decimal import Decimal
+from io import BytesIO
+from typing import Any
+
+import xlsxwriter
+
+from superset.utils.excel import NEUTRAL_DOCUMENT_PROPERTIES
+
+# Excel limits a sheet name to 31 characters and forbids these characters.
+MAX_SHEET_NAME_LEN = 31
+_INVALID_SHEET_CHARS_RE = re.compile(r"[\[\]:*?/\\]")
+# Excel reserves the sheet name "History" (case-insensitive).
+_RESERVED_SHEET_NAME = "history"
+
+# A worksheet holds at most 1,048,576 rows; one is reserved for the header.
+MAX_DATA_ROWS_PER_SHEET = 1_048_576 - 1
+
+# Leading characters that turn a cell into a formula in spreadsheet apps.
Mirrors
+# superset.utils.excel.quote_formulas so streamed exports get the same guard.
+_FORMULA_PREFIXES = {"=", "+", "-", "@"}
+
+# Excel cannot represent integers beyond 10**15 without precision loss.
+_MAX_EXCEL_INT = 10**15
+
+
+def _quote_if_formula(text: str) -> str:
+ """
+ Prefix formula-like text with an apostrophe so spreadsheet apps treat it as
+ literal text (defense against formula injection).
+
+ Leading whitespace is ignored when detecting a formula, because spreadsheet
+ apps still evaluate a cell whose formula prefix is preceded by spaces or
+ tabs (e.g. ``" =cmd"`` or ``"\\t=cmd"``).
+ """
+ stripped = text.lstrip()
+ return f"'{text}" if stripped and stripped[0] in _FORMULA_PREFIXES else
text
+
+
+def _coerce_float_cell(value: Any) -> Any:
+ """
+ Convert a ``Decimal``/real value to something ``xlsxwriter`` accepts.
+
+ ``float()`` on a non-finite ``Decimal`` ("NaN"/"Infinity") yields a value
+ xlsxwriter rejects, and an over-large value can raise ``OverflowError``;
+ blank the former and stringify the latter, and stringify magnitudes Excel
+ cannot represent precisely.
+ """
+ try:
+ number = float(value)
+ except (OverflowError, ValueError):
+ return str(value)
+ if not math.isfinite(number):
+ return ""
+ return str(number) if abs(number) > _MAX_EXCEL_INT else number
+
+
+def sanitize_sheet_name(raw: str, used: set[str]) -> str:
+ """
+ Produce a valid, unique Excel sheet name from ``raw``.
+
+ Replaces forbidden characters, strips surrounding apostrophes/whitespace,
+ avoids the reserved name "History", truncates to 31 characters, and
+ disambiguates case-insensitive collisions with ``~2``/``~3`` suffixes.
+ The chosen name (lower-cased) is added to ``used``.
+
+ :param raw: The desired sheet name (e.g. ``"42 - Sales by Region"``)
+ :param used: Lower-cased names already taken; mutated with the result
+ :returns: A sanitized, unique sheet name no longer than 31 characters
+ """
+ name = _INVALID_SHEET_CHARS_RE.sub("_", raw or "")
+ name = name.strip().strip("'").strip()
+ if not name:
+ name = "Sheet"
+ if name.lower() == _RESERVED_SHEET_NAME:
+ name = f"{name}_"
+ name = name[:MAX_SHEET_NAME_LEN]
+
+ if name.lower() not in used:
+ used.add(name.lower())
+ return name
+
+ suffix = 2
+ while True:
+ marker = f"~{suffix}"
+ candidate = name[: MAX_SHEET_NAME_LEN - len(marker)] + marker
+ if candidate.lower() not in used:
+ used.add(candidate.lower())
+ return candidate
+ suffix += 1
+
+
+def _sanitize_cell(value: Any) -> Any:
+ """
+ Coerce a single cell value into something safe for ``xlsxwriter``.
+
+ Quotes formula-like strings (defense against formula injection),
stringifies
+ integers/floats Excel cannot represent precisely, renders temporal values
as
+ ISO strings (timezones are not natively supported), and blanks out ``None``
+ and non-finite floats.
+ """
+ if value is None:
+ return ""
+ # bool is a subclass of int; preserve it before the numeric branches.
+ if isinstance(value, bool):
+ return value
+ if isinstance(value, str):
+ return _quote_if_formula(value)
+ if isinstance(value, (datetime, date)):
+ return value.isoformat()
+ if isinstance(value, Decimal):
+ return _coerce_float_cell(value)
+ if isinstance(value, numbers.Integral):
+ number = int(value)
+ return str(number) if abs(number) > _MAX_EXCEL_INT else number
+ if isinstance(value, numbers.Real):
+ return _coerce_float_cell(value)
+ # Anything else (lists, dicts, custom objects) is stringified, still
guarding
+ # against formula injection on the resulting text.
+ return _quote_if_formula(str(value))
+
+
+class StreamingXlsxWriter:
+ """
+ A thin wrapper over ``xlsxwriter`` in constant-memory mode that writes one
+ sheet per chart, row by row.
+
+ Sheet names are sanitized and de-duplicated, cell values are sanitized for
+ safety/compatibility, and per-sheet row counts are capped at Excel's limit.
+ Always call :meth:`close` (e.g. in a ``finally`` block) to finalize the
file.
+ """
+
+ def __init__(self, path: str) -> None:
+ self._workbook = xlsxwriter.Workbook(path, {"constant_memory": True})
+ # Reset document properties so the file carries no identifying details.
+ self._workbook.set_properties(NEUTRAL_DOCUMENT_PROPERTIES)
+ self._used_sheet_names: set[str] = set()
+ self.sheet_count = 0
Review Comment:
**Suggestion:** Add an explicit type annotation for this instance attribute
initialization so the class state is fully typed. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new code initializes an instance attribute in __init__ without an
explicit type annotation. Since the attribute is clearly an integer counter and
can be annotated, this matches the rule requiring type hints on relevant
variables in new or modified Python code.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=23d715c070f8473c8307aaa2701d16c5&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=23d715c070f8473c8307aaa2701d16c5&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/utils/excel_streaming.py
**Line:** 171:171
**Comment:**
*Custom Rule: Add an explicit type annotation for this instance
attribute initialization so the class state is fully typed.
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=9f3fe0d7cc04c9e3c3c7ccd6cd7d7960182a21eff145131610e87bf3ecb10154&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=9f3fe0d7cc04c9e3c3c7ccd6cd7d7960182a21eff145131610e87bf3ecb10154&reaction=dislike'>π</a>
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -0,0 +1,345 @@
+# 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.
+"""
+Celery task that exports every chart on a dashboard to a single multi-sheet
+``.xlsx`` file, uploads it to S3, and emails the requesting user a pre-signed
+download link.
+
+In ``"data"`` mode the task re-runs each chart's saved query context under the
+requesting user, applies the live dashboard filter state, and streams the
results
+row-by-row into a constant-memory workbook so large dashboards never load all
+data at once. In ``"images"`` mode non-table charts are instead rendered to
+images (through the same headless path as scheduled reports, reflecting the
live
+filters) and embedded, while table-like charts stay tabular.
+"""
+
+from __future__ import annotations
+
+import logging
+import os
+import tempfile
+from datetime import datetime, timedelta, timezone
+from typing import Any
+
+from celery.exceptions import SoftTimeLimitExceeded
+from flask import current_app, g
+
+from superset import db, security_manager
+from superset.charts.data.dashboard_filter_context import (
+ apply_dashboard_filter_context,
+ get_dashboard_filter_context,
+)
+from superset.charts.schemas import ChartDataQueryContextSchema
+from superset.commands.chart.data.get_data_command import ChartDataCommand
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.dashboards.excel_export import email
+from superset.dashboards.excel_export.layout import get_charts_in_layout_order
+from superset.dashboards.excel_export.screenshot import render_chart_image
+from superset.extensions import cache_manager, celery_app
+from superset.utils import json, s3
+from superset.utils.core import override_user
+from superset.utils.excel_streaming import StreamingXlsxWriter
+
+logger = logging.getLogger(__name__)
+
+# Export modes: "data" streams every chart's tabular result (the default,
+# unchanged behavior); "images" embeds non-table charts as rendered images and
+# keeps only table-like charts tabular.
+EXPORT_MODE_DATA = "data"
+EXPORT_MODE_IMAGES = "images"
+
+# Viz types kept as tabular data in image mode; everything else is rendered as
an
+# image. Operators can override the set via ``EXCEL_EXPORT_TABLE_VIZ_TYPES``.
+TABLE_VIZ_TYPES = {"table", "pivot_table_v2", "pivot_table"}
+
+EXPORT_SOFT_TIME_LIMIT = 600
+EXPORT_HARD_TIME_LIMIT = 660
+# TTL for the per-user+dashboard in-flight lock set by the API before enqueue.
+# It outlives the hard time limit so a worker killed at that limit (which skips
+# the ``finally`` cleanup) cannot hold the lock forever; the delete in
+# ``finally`` is the fast path that frees it as soon as the task settles.
+EXPORT_INFLIGHT_CACHE_TTL = EXPORT_HARD_TIME_LIMIT + 60
+
+
+class _ChartSkippedError(Exception):
+ """Signals a chart that could not be exported and should be listed as
skipped."""
+
+
+def _chart_label(chart: Any) -> str:
+ """Human-readable label for a chart in the skipped-charts list."""
+ return f"{chart.id} - {chart.slice_name or ''}".strip()
+
+
+def _record_to_row(record: dict[str, Any], colnames: list[str]) -> list[Any]:
+ return [record.get(col) for col in colnames]
+
+
+def _table_viz_types() -> set[str]:
+ """Viz types kept tabular in image mode (config override or built-in
default)."""
+ return current_app.config.get("EXCEL_EXPORT_TABLE_VIZ_TYPES") or
TABLE_VIZ_TYPES
+
+
+def _renders_as_image(chart: Any, mode: str) -> bool:
+ """Whether this chart is embedded as an image rather than streamed as
data."""
+ return mode == EXPORT_MODE_IMAGES and chart.viz_type not in
_table_viz_types()
+
+
+def _write_chart_image_sheet(
+ writer: StreamingXlsxWriter,
+ chart: Any,
+ dashboard_id: int,
+ active_data_mask: dict[str, Any],
+ user: Any,
+) -> None:
+ """
+ Render a single chart to an image and embed it as its own sheet.
+
+ :raises _ChartSkippedError: if the chart could not be rendered
+ """
+ image = render_chart_image(chart, dashboard_id, active_data_mask, user)
+ if image is None:
+ raise _ChartSkippedError
+ writer.add_image_sheet(_chart_label(chart), image)
+
+
+def _write_chart_sheets(
+ writer: StreamingXlsxWriter,
+ chart: Any,
+ dashboard_id: int,
+ active_data_mask: dict[str, Any],
+) -> None:
+ """
+ Run a single chart's query and stream its result(s) into the workbook.
+
+ Charts may yield more than one query (e.g. mixed-series charts); each
becomes
+ its own sheet. Raises if the chart cannot be exported, so the caller can
skip
+ it and note it in the email.
+ """
+ json_body = json.loads(chart.query_context)
+ # Override any stale saved values: we always want full JSON results.
+ json_body["result_format"] = ChartDataResultFormat.JSON
+ json_body["result_type"] = ChartDataResultType.FULL
+ json_body.pop("force", None)
+
+ filter_context = get_dashboard_filter_context(
+ dashboard_id=dashboard_id,
+ chart_id=chart.id,
+ active_data_mask=active_data_mask,
+ )
+ if filter_context.extra_form_data:
+ apply_dashboard_filter_context(json_body,
filter_context.extra_form_data)
+
+ # Jinja macros resolve form data from g.form_data; expose the saved
context.
+ g.form_data = json_body
+
+ query_context = ChartDataQueryContextSchema().load(json_body)
+ command = ChartDataCommand(query_context)
+ command.validate()
+ result = command.run()
+
+ for index, query in enumerate(result["queries"]):
+ colnames = query.get("colnames") or []
+ data = query.get("data") or []
+ if index == 0:
+ name = f"{chart.id} - {chart.slice_name or ''}"
+ else:
+ name = f"{chart.id}.{index} - {chart.slice_name or ''}"
+ writer.add_sheet(
+ name,
+ colnames,
+ (_record_to_row(record, colnames) for record in data),
+ )
+
+
+def _build_workbook(
+ path: str,
+ dashboard: Any,
+ active_data_mask: dict[str, Any],
+ job_id: str,
+ mode: str,
+ user: Any,
+) -> dict[str, list[str]]:
+ """Build the workbook on disk.
+
+ Return the charts that could not be exported, grouped by the reason they
+ were omitted (see the ``email.ERROR_*`` reason keys), so the notification
+ can explain each group separately.
+ """
+ errored: dict[str, list[str]] = {}
+ writer = StreamingXlsxWriter(path)
+ try:
+ for chart in get_charts_in_layout_order(dashboard):
+ label = _chart_label(chart)
+ as_image = _renders_as_image(chart, mode)
+ # Image charts render from their saved params and don't need a
query
+ # context; data (and table) charts still do.
+ if not as_image and not chart.query_context:
+ errored.setdefault(email.ERROR_NO_QUERY_CONTEXT,
[]).append(label)
+ continue
+ try:
+ if as_image:
+ _write_chart_image_sheet(
+ writer, chart, dashboard.id, active_data_mask, user
+ )
+ else:
+ _write_chart_sheets(writer, chart, dashboard.id,
active_data_mask)
+ except SoftTimeLimitExceeded:
+ logger.warning(
+ "Chart %s timed out in dashboard export %s", chart.id,
job_id
+ )
+ errored.setdefault(email.ERROR_TIMEOUT, []).append(label)
+ except _ChartSkippedError:
+ logger.warning(
+ "Skipping chart %s in dashboard export %s (could not
render)",
+ chart.id,
+ job_id,
+ )
+ errored.setdefault(email.ERROR_GENERAL, []).append(label)
+ except Exception: # pylint: disable=broad-except
+ logger.exception(
+ "Skipping chart %s in dashboard export %s", chart.id,
job_id
+ )
+ errored.setdefault(email.ERROR_GENERAL, []).append(label)
+
+ if writer.sheet_count == 0:
+ flat = [label for labels in errored.values() for label in labels]
+ writer.add_summary_sheet(
+ "Export Summary",
+ ["No chart data could be exported.", *flat],
+ )
+ finally:
+ writer.close()
+ return errored
+
+
+def _send_failure_email(
+ user: Any, dashboard_title: str, requested_at: datetime
+) -> None:
+ if not (user and getattr(user, "email", None)):
+ return
+ try:
+ email.send_export_email(
+ user.email,
+ email.build_subject(dashboard_title, success=False),
+ email.build_failure_email(dashboard_title, requested_at),
+ )
+ except Exception: # pylint: disable=broad-except
+ logger.exception("Failed to send export failure email")
+
+
+@celery_app.task(
+ name="export_dashboard_excel",
+ bind=True,
+ soft_time_limit=EXPORT_SOFT_TIME_LIMIT,
+ time_limit=EXPORT_HARD_TIME_LIMIT,
+ max_retries=0,
+)
+def export_dashboard_excel(
+ self: Any, # pylint: disable=unused-argument
+ dashboard_id: int,
+ user_id: int,
+ active_data_mask: dict[str, Any],
+ job_id: str,
+ mode: str = EXPORT_MODE_DATA,
+ inflight_key: str | None = None,
+) -> None:
+ """
+ Export a dashboard's charts to an ``.xlsx`` and email a download link.
+
+ :param dashboard_id: The dashboard to export
+ :param user_id: The requesting user (the task runs with their permissions)
+ :param active_data_mask: Live dashboard filter state keyed by native
filter id
+ :param job_id: Correlation id, also the Celery task id and S3 object name
+ :param mode: ``"data"`` streams every chart's tabular result; ``"images"``
+ embeds non-table charts as rendered images and keeps tables tabular
+ :param inflight_key: Cache key of the per-user+dashboard throttle lock,
freed
+ when the task settles (the lock's TTL is the backstop)
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset.models.dashboard import Dashboard
+
+ requested_at = datetime.now(tz=timezone.utc)
+ user = security_manager.get_user_by_id(user_id)
+ dashboard_title = ""
+ tmp_path: str | None = None
Review Comment:
**Suggestion:** Add a type annotation for this local variable initialization
so it is explicitly typed. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The local variable is initialized with an empty string and later assigned
string values, so it can be explicitly annotated as `str`. Since the code omits
that annotation, this is a real type-hint omission.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bcc2fb506b3e4fa2adfda2c97b9d96d9&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=bcc2fb506b3e4fa2adfda2c97b9d96d9&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/tasks/export_dashboard_excel.py
**Line:** 278:278
**Comment:**
*Custom Rule: Add a type annotation for this local variable
initialization so it is explicitly typed.
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=86f3ac165e24d663a529af6cc638adac23efbe57bc1ad86fcb2c9f955104b13a&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=86f3ac165e24d663a529af6cc638adac23efbe57bc1ad86fcb2c9f955104b13a&reaction=dislike'>π</a>
##########
superset/dashboards/api.py:
##########
@@ -1471,6 +1484,109 @@ 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
+ @permission_name("export")
+ @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 {}
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation to this loaded request
payload variable to satisfy the type-hint requirement for newly introduced
variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new local variable `payload` is introduced without a type annotation,
and it is a value that can be annotated. This matches the Python type-hint rule
for newly added variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=62fa39a4888f437386d77a57e526fba6&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=62fa39a4888f437386d77a57e526fba6&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:** 1543:1545
**Comment:**
*Custom Rule: Add an explicit type annotation to this loaded request
payload variable to satisfy the type-hint requirement for newly introduced
variables.
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=731d988aab74ad95fb6101e4da107fab5e3284256640892c0c0839420dc46ab4&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=731d988aab74ad95fb6101e4da107fab5e3284256640892c0c0839420dc46ab4&reaction=dislike'>π</a>
##########
superset/dashboards/excel_export/screenshot.py:
##########
@@ -0,0 +1,98 @@
+# 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.
+"""
+Render a single dashboard chart to a PNG for the image-mode Excel export.
+
+This reuses the same headless render path scheduled reports use
+(:class:`~superset.utils.screenshots.ChartScreenshot`), but points it at an
+Explore URL whose ``form_data`` carries the live dashboard filter state β so an
+embedded image reflects the same filters the data path applies, rather than the
+chart's default saved state.
+"""
+
+from __future__ import annotations
+
+import logging
+from typing import Any
+
+from flask import current_app
+
+from superset.charts.data.dashboard_filter_context import (
+ get_dashboard_filter_context,
+)
+from superset.utils import json
+from superset.utils.screenshots import ChartScreenshot
+from superset.utils.urls import get_url_path
+
+logger = logging.getLogger(__name__)
Review Comment:
**Suggestion:** Add an explicit type annotation to the module-level logger
variable to satisfy required Python type hints on relevant variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The module-level logger is a relevant variable that can be annotated, and
the new file omits a type hint for it. This matches the rule requiring Python
type hints on relevant variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff2687c8b6eb433fa459eb41954e6b89&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=ff2687c8b6eb433fa459eb41954e6b89&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/excel_export/screenshot.py
**Line:** 41:41
**Comment:**
*Custom Rule: Add an explicit type annotation to the module-level
logger variable to satisfy required Python type hints on relevant variables.
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=d6aa3abbfb6b20870ae5478e6638aa0f1fa3fbef5c9b7a837a661cabd4d6840a&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=d6aa3abbfb6b20870ae5478e6638aa0f1fa3fbef5c9b7a837a661cabd4d6840a&reaction=dislike'>π</a>
##########
superset/dashboards/schemas.py:
##########
@@ -633,3 +633,30 @@ class CacheScreenshotSchema(Schema):
fields.List(fields.Str(), validate=lambda x: len(x) == 2),
required=False
)
permalinkKey = fields.Str(required=False) # noqa: N815
+
+
+class DashboardExportXlsxPostSchema(Schema):
+ active_data_mask = fields.Dict(
+ keys=fields.Str(),
+ values=fields.Dict(),
+ load_default=dict,
+ metadata={
+ "description": "Live dashboard filter state keyed by native filter
id, "
+ "each carrying an extraFormData object."
+ },
+ )
+ mode = fields.String(
+ load_default="data",
+ validate=OneOf(["data", "images"]),
+ metadata={
+ "description": "Export mode: 'data' streams each chart's tabular
result "
+ "(default); 'images' embeds non-table charts as rendered images
and "
+ "keeps table charts tabular."
+ },
+ )
+
+
+class DashboardExportXlsxResponseSchema(Schema):
+ job_id = fields.String(
+ metadata={"description": "Correlation id for the async export task"}
+ )
Review Comment:
**Suggestion:** Annotate this newly added response field attribute with an
explicit type to align with the rule requiring type hints on relevant
variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a new Python class-level field declaration and it is not
type-annotated. That matches the custom ruleβs condition for flagging newly
added relevant variables lacking type hints.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=11940f723544446c922389f703a30eea&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=11940f723544446c922389f703a30eea&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/schemas.py
**Line:** 660:662
**Comment:**
*Custom Rule: Annotate this newly added response field attribute with
an explicit type to align with the rule requiring type hints on relevant
variables.
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=0ebae3a42a213156b1d23acde556b1dafd54b2b74a1830d0a21e1af86f45a636&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=0ebae3a42a213156b1d23acde556b1dafd54b2b74a1830d0a21e1af86f45a636&reaction=dislike'>π</a>
##########
superset/dashboards/schemas.py:
##########
@@ -633,3 +633,30 @@ class CacheScreenshotSchema(Schema):
fields.List(fields.Str(), validate=lambda x: len(x) == 2),
required=False
)
permalinkKey = fields.Str(required=False) # noqa: N815
+
+
+class DashboardExportXlsxPostSchema(Schema):
+ active_data_mask = fields.Dict(
+ keys=fields.Str(),
+ values=fields.Dict(),
+ load_default=dict,
+ metadata={
+ "description": "Live dashboard filter state keyed by native filter
id, "
+ "each carrying an extraFormData object."
+ },
+ )
+ mode = fields.String(
+ load_default="data",
+ validate=OneOf(["data", "images"]),
+ metadata={
+ "description": "Export mode: 'data' streams each chart's tabular
result "
+ "(default); 'images' embeds non-table charts as rendered images
and "
+ "keeps table charts tabular."
+ },
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation to this new schema field
variable so the added code complies with required typing coverage. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is another newly introduced class attribute without a type annotation.
Since the rule requires type hints for relevant newly added variables, the
omission is a real violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6b41ca44ba7d464ab80c1fc5c0d1aa88&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=6b41ca44ba7d464ab80c1fc5c0d1aa88&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/schemas.py
**Line:** 648:656
**Comment:**
*Custom Rule: Add an explicit type annotation to this new schema field
variable so the added code complies with required typing coverage.
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=163581b1110535ae9bd6991afec3bd3ec3f3b5b2ea0301446ee3f2b10a3732c2&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=163581b1110535ae9bd6991afec3bd3ec3f3b5b2ea0301446ee3f2b10a3732c2&reaction=dislike'>π</a>
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3279,6 +3281,100 @@ def _base_filter(query):
response_roles = [result["text"] for result in response["result"]]
assert response_roles == ["Alpha"]
+ def test_export_xlsx_501_when_bucket_unset(self):
Review Comment:
**Suggestion:** Add type hints to this new test method signature, including
an explicit return type annotation. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This new test method has no type annotations on its signature and no
explicit return type, which violates the Python type-hint requirement for newly
added or modified code.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5defe3534d6c4189bceaf9ce15c8f2bd&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=5defe3534d6c4189bceaf9ce15c8f2bd&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:** tests/integration_tests/dashboards/api_tests.py
**Line:** 3284:3284
**Comment:**
*Custom Rule: Add type hints to this new test method signature,
including an explicit return type annotation.
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=ae6d313dce6f59d67df06d90283d016a3a9548e4946ad9bd2c38f42c938df52e&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=ae6d313dce6f59d67df06d90283d016a3a9548e4946ad9bd2c38f42c938df52e&reaction=dislike'>π</a>
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3279,6 +3281,100 @@ def _base_filter(query):
response_roles = [result["text"] for result in response["result"]]
assert response_roles == ["Alpha"]
+ def test_export_xlsx_501_when_bucket_unset(self):
+ """Dashboard API: export_xlsx returns 501 when the S3 bucket is
unset."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-501", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 501
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_404_for_missing_dashboard(self, mock_task):
Review Comment:
**Suggestion:** Annotate this method parameters and return type so the new
test function complies with required Python type hints. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This newly added test method omits type annotations for the mocked argument
and return type, so it matches the stated type-hint violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=222e0214ea974bdeb4da56a9ff7b0c1f&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=222e0214ea974bdeb4da56a9ff7b0c1f&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:** tests/integration_tests/dashboards/api_tests.py
**Line:** 3298:3298
**Comment:**
*Custom Rule: Annotate this method parameters and return type so the
new test function complies with required Python type hints.
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=b401560f19f945bdd1d7bf86d6bf36dda78d5ced2ae0599326065a5a3d19b0ee&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=b401560f19f945bdd1d7bf86d6bf36dda78d5ced2ae0599326065a5a3d19b0ee&reaction=dislike'>π</a>
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3279,6 +3281,100 @@ def _base_filter(query):
response_roles = [result["text"] for result in response["result"]]
assert response_roles == ["Alpha"]
+ def test_export_xlsx_501_when_bucket_unset(self):
+ """Dashboard API: export_xlsx returns 501 when the S3 bucket is
unset."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-501", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 501
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_404_for_missing_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 404 for an unknown dashboard."""
+ self.login(ADMIN_USERNAME)
+ rv = self.client.post("api/v1/dashboard/99999999/export_xlsx/")
+ assert rv.status_code == 404
+ mock_task.apply_async.assert_not_called()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_400_for_empty_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 400 for a dashboard with no
charts."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-empty", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 400
+ mock_task.apply_async.assert_not_called()
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_202_enqueues_task(self, mock_task):
Review Comment:
**Suggestion:** Add type hints to this newly added method signature for both
the mock argument and the return type. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This new test function lacks type annotations on both the mock argument and
the return value, which is exactly the type-hint omission the rule flags.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=08e47a951aa14b5a85d4e7a19e2a1227&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=08e47a951aa14b5a85d4e7a19e2a1227&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:** tests/integration_tests/dashboards/api_tests.py
**Line:** 3323:3323
**Comment:**
*Custom Rule: Add type hints to this newly added method signature for
both the mock argument and the return type.
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=af49f4df1e9466b0d7844dc60bdffe24a000af8b86ca5ad0ebed235397c8a9ed&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=af49f4df1e9466b0d7844dc60bdffe24a000af8b86ca5ad0ebed235397c8a9ed&reaction=dislike'>π</a>
##########
tests/unit_tests/dashboards/test_excel_export_screenshot.py:
##########
@@ -0,0 +1,146 @@
+# 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.
+from __future__ import annotations
+
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+from superset.charts.data.dashboard_filter_context import
DashboardFilterContext
+from superset.dashboards.excel_export import screenshot as screenshot_module
+from superset.dashboards.excel_export.screenshot import render_chart_image
+from superset.utils import json
+
+MODULE = "superset.dashboards.excel_export.screenshot"
Review Comment:
**Suggestion:** Add an explicit type annotation to this module-level
constant to satisfy the type-hint requirement for annotatable variables.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This new Python module defines a module-level constant without a type
annotation, and `str` is a clear annotatable type. That matches the custom rule
requiring type hints on relevant variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8b9ba44e9ccb4fe1be1d488436f92092&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=8b9ba44e9ccb4fe1be1d488436f92092&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:** tests/unit_tests/dashboards/test_excel_export_screenshot.py
**Line:** 27:27
**Comment:**
*Custom Rule: Add an explicit type annotation to this module-level
constant to satisfy the type-hint requirement for annotatable variables.
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=f199cfab3ab276ea15ea6137787fd445185db940fe3174a24f3e8914a551310b&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=f199cfab3ab276ea15ea6137787fd445185db940fe3174a24f3e8914a551310b&reaction=dislike'>π</a>
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3279,6 +3281,100 @@ def _base_filter(query):
response_roles = [result["text"] for result in response["result"]]
assert response_roles == ["Alpha"]
+ def test_export_xlsx_501_when_bucket_unset(self):
+ """Dashboard API: export_xlsx returns 501 when the S3 bucket is
unset."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-501", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 501
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_404_for_missing_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 404 for an unknown dashboard."""
+ self.login(ADMIN_USERNAME)
+ rv = self.client.post("api/v1/dashboard/99999999/export_xlsx/")
+ assert rv.status_code == 404
+ mock_task.apply_async.assert_not_called()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_400_for_empty_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 400 for a dashboard with no
charts."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-empty", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 400
+ mock_task.apply_async.assert_not_called()
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_202_enqueues_task(self, mock_task):
+ """Dashboard API: export_xlsx enqueues the task and returns 202 +
job_id."""
+ self.login(ADMIN_USERNAME)
+ dashboard =
db.session.query(Dashboard).filter_by(slug="world_health").first()
+ rv = self.client.post(
+ f"api/v1/dashboard/{dashboard.id}/export_xlsx/",
+ json={"active_data_mask": {}},
+ )
+ assert rv.status_code == 202
+ body = json.loads(rv.data.decode("utf-8"))
+ job_id = body["job_id"]
+ assert job_id
+ mock_task.apply_async.assert_called_once()
+ _, kwargs = mock_task.apply_async.call_args
+ assert kwargs["task_id"] == job_id
+ assert kwargs["kwargs"]["dashboard_id"] == dashboard.id
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.cache_manager")
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_202_when_export_already_in_progress(
+ self, mock_task, mock_cache_manager
+ ):
Review Comment:
**Suggestion:** Annotate all parameters and the return type in this
multiline function signature to satisfy the type-hint requirement. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The multiline test signature introduces new Python code without annotations
for either mocked parameter or the return type, so this is a genuine type-hint
violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=310d2ebe846e43ad8159d1b518078a10&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=310d2ebe846e43ad8159d1b518078a10&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:** tests/integration_tests/dashboards/api_tests.py
**Line:** 3344:3346
**Comment:**
*Custom Rule: Annotate all parameters and the return type in this
multiline function signature to satisfy the type-hint requirement.
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=5200f4cf8219559dbf43d8f1a28f7b38e76c9963d7dbd0a42db9fec2a7af3aa0&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=5200f4cf8219559dbf43d8f1a28f7b38e76c9963d7dbd0a42db9fec2a7af3aa0&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]
