You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "innovark37 (via GitHub)" <gi...@apache.org> on 2026/06/09 06:45:43 UTC
[PR] feat: added email delivery of XLSX reports [superset]
innovark37 opened a new pull request, #40885:
URL: https://github.com/apache/superset/pull/40885
### SUMMARY
Adds XLSX as an output format for chart-based email reports.
This change:
- Adds XLSX options to the chart report and Alerts & Reports modals.
- Generates XLSX files using post-processed chart data.
- Preserves the configured Excel export and index behavior.
- Attaches regular exports as `.xlsx` files.
- Attaches server-paginated, multi-file exports as `.zip` archives.
- Sets the appropriate MIME types for XLSX and ZIP attachments.
- Adds backend and frontend unit test coverage.
- Updates the Alerts & Reports documentation.
Dashboard reports remain limited to PDF and PNG formats.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before**
XLSX was not available as an email report format.
<!-- Add a screenshot of the format selector without XLSX. -->
**After**
Chart reports can be delivered as XLSX email attachments.
<!-- Add screenshots of XLSX in:
1. The chart email report modal.
2. The Alerts & Reports modal.
-->
### TESTING INSTRUCTIONS
1. Enable the `ALERT_REPORTS` feature flag and configure email reports.
2. Open a chart and select **Schedule email report**.
3. Select **Excel (XLSX) attached in email** as the report format.
4. Create the report and trigger its execution.
5. Verify that the email contains an `.xlsx` attachment that opens successfully.
6. Verify that the exported data reflects the chart's post-processing.
7. Repeat with a server-paginated chart and verify that the email contains a `.zip` archive with XLSX files.
8. Open **Settings > Alerts & Reports** and verify that XLSX is available for chart reports but not dashboard reports.
9. Verify that the screenshot width field is not displayed when XLSX is selected.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [x] Required feature flags: `ALERT_REPORTS`
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3571207887
##########
superset/commands/report/execute.py:
##########
@@ -897,6 +934,13 @@ def _get_notification_content(self) -> NotificationContent: # noqa: C901
csv_data = self._get_csv_data()
if not csv_data:
error_text = "Unexpected missing csv file"
+ elif (
+ self._report_schedule.chart
+ and self._report_schedule.report_format == ReportDataFormat.XLSX
+ ):
+ xlsx_data = self._get_xlsx_data()
+ if not xlsx_data:
+ error_text = "Unexpected missing xlsx file"
Review Comment:
Good catch.
XLSX is only supported for chart-backed schedules, so I added an explicit server-side guard for unsupported targets. A dashboard schedule configured with XLSX now raises `ReportScheduleXlsxFailedError` instead of falling through and sending a successful notification without an attachment.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4661547513
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601580802
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
Review Comment:
Thanks, this was stale after the merge. `_get_xlsx_data` was removed in favor of `_get_data(ChartDataResultFormat.XLSX)`, so I updated the XLSX tests to use the current API.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4658796972
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601290705
##########
superset/utils/core.py:
##########
@@ -120,6 +120,27 @@
logging.getLogger("MARKDOWN").setLevel(logging.INFO)
logger = logging.getLogger(__name__)
+EMAIL_ATTACHMENT_SUBTYPES: dict[str, str] = {
+ ".pdf": "pdf",
+ ".zip": "zip",
+ ".xlsx": "vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+}
+
+
+def build_email_attachment(name: str, body: bytes | str) -> MIMEApplication:
+ """
+ Create an email attachment part with stable filename metadata.
+ """
+ subtype = EMAIL_ATTACHMENT_SUBTYPES.get(os.path.splitext(name)[1].lower())
+ attachment = MIMEApplication(
+ body,
+ _subtype=subtype or "octet-stream",
+ Name=name,
+ )
Review Comment:
**Suggestion:** `MIMEApplication` expects binary payloads, but `build_email_attachment` now accepts `str` and forwards it directly as `body`. For non-ASCII text attachments this can produce incorrect encoding (or encoding errors) and corrupt delivered files. Normalize `str` bodies to UTF-8 bytes (or route textual payloads through `MIMEText`) before creating the attachment so attachment bytes are stable across character sets. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Email attachments with non-ASCII text may be corrupted.
- ⚠️ Text-attachment reports via send_email_smtp become unreliable.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start a Python shell inside the Superset environment so you can call the existing mail
helper `send_email_smtp` defined in `superset/utils/core.py:9-23` (see lines 848-868 in
the file for its signature and documentation).
2. Import the helper and config: `from superset.utils.core import send_email_smtp` and
obtain a running Flask app context exposing `current_app.config` (the same config used in
`superset/reports/notifications/email.py:117-121` when sending report emails).
3. Invoke `send_email_smtp` with a textual attachment value instead of bytes, for example:
`send_email_smtp("to@example.com", "subject", "content", current_app.config,
data={"report.txt": "café"})`, which flows into the `for name, body in (data or
{}).items()` loop at `superset/utils/core.py:65-66`.
4. Observe that `build_email_attachment` at `superset/utils/core.py:130-139` receives
`body="café"` (a `str`), passes it directly to `MIMEApplication` as the payload, and the
stdlib’s `MIMEApplication` base64 encoder treats this as binary data; for non-ASCII text
this either raises a `UnicodeEncodeError` during ASCII encoding or produces incorrectly
encoded bytes, resulting in a failed send or a corrupted `report.txt` attachment when the
email is delivered.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=311ac4ae032d4acb84200dd7af5bcfd3&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=311ac4ae032d4acb84200dd7af5bcfd3&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/core.py
**Line:** 130:139
**Comment:**
*Api Mismatch: `MIMEApplication` expects binary payloads, but `build_email_attachment` now accepts `str` and forwards it directly as `body`. For non-ASCII text attachments this can produce incorrect encoding (or encoding errors) and corrupt delivered files. Normalize `str` bodies to UTF-8 bytes (or route textual payloads through `MIMEText`) before creating the attachment so attachment bytes are stable across character sets.
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%2F40885&comment_hash=0b225068a73642f0fe39c6a389d3c09b219398abe37529ca4d4617201fde1923&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=0b225068a73642f0fe39c6a389d3c09b219398abe37529ca4d4617201fde1923&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
Review Comment:
**Suggestion:** This test calls a method that does not exist on `BaseReportState`, so it will fail with `AttributeError` before the intended assertion is evaluated. Update the test to call the current API (`_get_data(...)`) or reintroduce the removed method if that was intended. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ JSON-format rejection test crashes with AttributeError.
- ⚠️ Unsupported chart-data format handling remains untested.
- ⚠️ Failing unit test can block CI runs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate function
test_get_chart_data_rejects_non_table_format where line 1368 calls
report_state._get_chart_data(ChartDataResultFormat.JSON) inside a
pytest.raises(ReportScheduleExecuteUnexpectedError, ...) context.
2. Open superset/commands/report/execute.py and inspect BaseReportState (lines 126-792),
confirming it defines _get_data and _get_chart_data_request_payload but no _get_chart_data
method (verified by grep for "def _get_chart_data").
3. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_get_chart_data_rejects_non_table_format
to execute this specific test.
4. During test execution, when Python evaluates
report_state._get_chart_data(ChartDataResultFormat.JSON) at
tests/unit_tests/commands/report/execute_test.py:1368 on a BaseReportState instance, it
raises AttributeError: 'BaseReportState' object has no attribute '_get_chart_data',
causing the test to fail before any ReportScheduleExecuteUnexpectedError can be raised for
the unsupported JSON format.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b3152c66d8294e63a7936217b5e60249&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=b3152c66d8294e63a7936217b5e60249&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/commands/report/execute_test.py
**Line:** 1368:1368
**Comment:**
*Api Mismatch: This test calls a method that does not exist on `BaseReportState`, so it will fail with `AttributeError` before the intended assertion is evaluated. Update the test to call the current API (`_get_data(...)`) or reintroduce the removed method if that was intended.
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%2F40885&comment_hash=008156b28aeab4e1c21851e448ebb5f401c62cb8881dcef0d52ef8cd57ba1f72&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=008156b28aeab4e1c21851e448ebb5f401c62cb8881dcef0d52ef8cd57ba1f72&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
+ get_url.assert_called_once_with(result_format=ChartDataResultFormat.XLSX)
+ get_chart_csv_data.assert_called_once_with(
+ chart_url="/api/v1/chart/1/data/xlsx",
+ auth_cookies=auth_cookies,
+ timeout=app.config["ALERT_REPORTS_CSV_REQUEST_TIMEOUT"],
+ )
+
+
+@pytest.mark.parametrize(
+ ("side_effect", "expected_exception", "expected_message"),
+ [
+ (
+ SoftTimeLimitExceeded(),
+ ReportScheduleXlsxTimeout,
+ "timeout occurred while generating an xlsx",
+ ),
+ (
+ RuntimeError("export failed"),
+ ReportScheduleXlsxFailedError,
+ "Failed generating xlsx export failed",
+ ),
+ ],
+)
+def test_get_xlsx_data_maps_errors(
+ app: SupersetApp,
+ mocker: MockerFixture,
+ side_effect: Exception,
+ expected_exception: type[Exception],
+ expected_message: str,
+) -> None:
+ """XLSX generation errors should use XLSX-specific report exceptions."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ side_effect=side_effect,
+ )
+
+ with pytest.raises(expected_exception, match=expected_message) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is side_effect
+
+
+def test_get_xlsx_data_rejects_empty_result(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """An empty XLSX response should fail report generation."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=None,
+ )
+
+ with pytest.raises(
+ ReportScheduleXlsxFailedError,
+ match="Report Schedule execution failed when generating an xlsx",
+ ) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is None
+
+
+def test_notification_content_contains_xlsx(mocker: MockerFixture) -> None:
+ """XLSX chart reports should populate the XLSX notification field."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.report_format = ReportDataFormat.XLSX
+ report_schedule.force_screenshot = False
+ report_schedule.email_subject = None
+ report_schedule.owners = []
+ report_schedule.recipients = []
+ report_state = BaseReportState(
+ report_schedule,
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ mocker.patch.object(report_state, "_get_url", return_value="/chart/1")
+ mocker.patch.object(report_state, "_get_log_data", return_value={})
+ get_xlsx_data = mocker.patch.object(
+ report_state,
+ "_get_xlsx_data",
+ return_value=b"xlsx-data",
+ )
Review Comment:
**Suggestion:** This patch target is invalid because `_get_xlsx_data` is not defined on `BaseReportState`, so `patch.object` will raise immediately. Patch `_get_data` with the XLSX result format path instead, and assert against the actual method used by `_get_notification_content`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ XLSX notification-content test fails during setup.
- ⚠️ NotificationContent.xlsx population remains unvalidated.
- ⚠️ Test suite stability degraded by setup error.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate
test_notification_content_contains_xlsx around lines 214-239; within this test, lines
1508-1512 define get_xlsx_data = mocker.patch.object(report_state, "_get_xlsx_data",
return_value=b"xlsx-data").
2. Open superset/commands/report/execute.py and inspect BaseReportState (lines 126-792),
confirming that it implements _get_data for CSV/XLSX but does not define a _get_xlsx_data
method (grep for "_get_xlsx_data" shows only references in this test file).
3. Recall that unittest.mock.patch.object by default uses create=False, meaning it first
checks that the target object has the named attribute and raises AttributeError if it does
not.
4. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_notification_content_contains_xlsx;
when pytest executes the test, patch.object(report_state, "_get_xlsx_data", ...) at line
1508 raises AttributeError because BaseReportState lacks _get_xlsx_data, so the test fails
during setup and never reaches state._get_notification_content() or asserts that
NotificationContent.xlsx is populated.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c6b54c91251c4ddb8cf3807273c0755b&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=c6b54c91251c4ddb8cf3807273c0755b&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/commands/report/execute_test.py
**Line:** 1508:1512
**Comment:**
*Api Mismatch: This patch target is invalid because `_get_xlsx_data` is not defined on `BaseReportState`, so `patch.object` will raise immediately. Patch `_get_data` with the XLSX result format path instead, and assert against the actual method used by `_get_notification_content`.
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%2F40885&comment_hash=f63aebb231bbf4832f940fb7ce11766773f37d72ac365c085dc9002bbf1f9479&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=f63aebb231bbf4832f940fb7ce11766773f37d72ac365c085dc9002bbf1f9479&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
Review Comment:
**Suggestion:** These new XLSX tests invoke `_get_xlsx_data()`, but the implementation now routes XLSX through `_get_data(ChartDataResultFormat.XLSX)`. As written, the tests will fail with `AttributeError` instead of validating XLSX behavior; switch these calls to the current method contract. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ XLSX fetch tests crash before exercising export path.
- ⚠️ XLSX timeout and error mapping unverified.
- ⚠️ CI runs fail when XLSX tests are executed.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate
test_get_xlsx_data_fetches_chart_data at lines 124-147, where line 1419 asserts
report_state._get_xlsx_data() == b"xlsx-data"; similar calls appear in
test_get_xlsx_data_maps_errors (line 1463) and test_get_xlsx_data_rejects_empty_result
(line 1488).
2. Open superset/commands/report/execute.py and confirm that BaseReportState implements
_get_data(result_format: ChartDataResultFormat) at lines 713-792, which handles both CSV
and XLSX, but there is no _get_xlsx_data method anywhere in the module (verified by grep
for "_get_xlsx_data").
3. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_get_xlsx_data_fetches_chart_data to
execute the first XLSX test.
4. When the test reaches line 1419 and evaluates report_state._get_xlsx_data() on the
BaseReportState instance, Python raises AttributeError: 'BaseReportState' object has no
attribute '_get_xlsx_data', so the test never exercises the XLSX-through-_get_data path
and fails before validating that XLSX bytes are fetched via the chart data endpoint.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5347de9eca6a408290e6b6a572ee5484&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=5347de9eca6a408290e6b6a572ee5484&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/commands/report/execute_test.py
**Line:** 1419:1419
**Comment:**
*Api Mismatch: These new XLSX tests invoke `_get_xlsx_data()`, but the implementation now routes XLSX through `_get_data(ChartDataResultFormat.XLSX)`. As written, the tests will fail with `AttributeError` instead of validating XLSX behavior; switch these calls to the current method contract.
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%2F40885&comment_hash=019391d04815982664352c24df06ed35600b60d5519eeb2516f1bf0f1be852e5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=019391d04815982664352c24df06ed35600b60d5519eeb2516f1bf0f1be852e5&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3571268482
##########
superset/reports/notifications/email.py:
##########
@@ -72,11 +74,46 @@
class EmailContent:
body: str
header_data: Optional[HeaderDataType] = None
- data: Optional[dict[str, Any]] = None
+ data: Optional[dict[str, bytes | str]] = None
pdf: Optional[dict[str, bytes]] = None
images: Optional[dict[str, bytes]] = None
+def _get_xlsx_attachment_extension(content: bytes) -> str:
+ """
+ Return the attachment extension for bytes returned by the XLSX export endpoint.
+ """
+ try:
+ with ZipFile(BytesIO(content)) as zip_file:
+ names = zip_file.namelist()
+ if _is_xlsx_zip(names):
+ return "xlsx"
+
+ files = [name for name in names if not name.endswith("/")]
+ if files and all(name.lower().endswith(".xlsx") for name in files):
+ for name in files:
+ with zip_file.open(name) as xlsx_file:
+ if not _is_xlsx_zipfile(xlsx_file.read()):
+ return "xlsx"
Review Comment:
Fixed.
The XLSX/ZIP classifier no longer reads each embedded workbook fully into memory. It now only checks the nested file ZIP signature, so detection stays bounded even for paginated exports with many large workbook files.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3570966150
##########
superset/charts/client_processing.py:
##########
@@ -403,5 +410,11 @@ def apply_client_processing( # noqa: C901
index=show_default_index,
**current_app.config["CSV_EXPORT"],
)
+ elif query["result_format"] == ChartDataResultFormat.XLSX:
+ excel.apply_column_types(processed_df, query["coltypes"])
+ query["data"] = excel.df_to_excel(
+ processed_df,
+ **current_app.config["EXCEL_EXPORT"],
+ )
Review Comment:
**Suggestion:** The XLSX serialization path does not pass an explicit `index` flag, so pandas defaults to writing the index. This diverges from the chart export behavior that suppresses default `RangeIndex`, causing post-processed report attachments to gain an unexpected extra index column. Pass the same computed index policy used by the CSV branch (eg, based on whether the index is default). [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ XLSX attachments include unintended default index column.
- ⚠️ XLSX report tables differ from CSV formatting policy.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Construct a pandas DataFrame with a default `RangeIndex` (e.g., `pd.DataFrame({"city":
["Paris", "London"], "value": [10, 20]})`) and wrap it in a chart query result for the
`table` viz type so that `post_processor` in `apply_client_processing` returns a frame
with a `RangeIndex` (superset/charts/client_processing.py:283-305, 313-325).
2. Call `apply_client_processing(result, form_data)` and observe that metadata computation
sets `show_default_index = not isinstance(processed_df.index, pd.RangeIndex)` to `False`
for this frame (client_processing.py:379), meaning default `RangeIndex` should not be
shown in exports.
3. For CSV result format, the code writes `query["data"] =
csv.df_to_escaped_csv(processed_df, index=show_default_index,
**current_app.config["CSV_EXPORT"])` (client_processing.py:405-412), passing `index=False`
when the index is a `RangeIndex`, which omits the index as intended and matches other
export paths like `query_context_processor`
(superset/common/query_context_processor.py:2-23).
4. For XLSX result format, the new branch at lines 413-418 calls
`excel.apply_column_types(processed_df, query["coltypes"])` and then `query["data"] =
excel.df_to_excel(processed_df, **current_app.config["EXCEL_EXPORT"])` without an explicit
`index` argument. With default `EXCEL_EXPORT` (superset/config.py:1436) lacking an
`"index"` key, `df.to_excel` inside `df_to_excel` (superset/utils/excel.py:14-22) defaults
to `index=True`, causing the emitted XLSX bytes to include the `RangeIndex` as an extra
first column even though `show_default_index` was `False` and CSV exports suppress it.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=47e44651449f4fbeaaa4e3c83777baba&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=47e44651449f4fbeaaa4e3c83777baba&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/charts/client_processing.py
**Line:** 413:418
**Comment:**
*Logic Error: The XLSX serialization path does not pass an explicit `index` flag, so pandas defaults to writing the index. This diverges from the chart export behavior that suppresses default `RangeIndex`, causing post-processed report attachments to gain an unexpected extra index column. Pass the same computed index policy used by the CSV branch (eg, based on whether the index is default).
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%2F40885&comment_hash=667c0612dac607d6d4332268e1ad61b1a7f856732a2fad5c7a2131c0ff528131&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=667c0612dac607d6d4332268e1ad61b1a7f856732a2fad5c7a2131c0ff528131&reaction=dislike'>👎</a>
##########
superset/charts/client_processing.py:
##########
@@ -357,6 +357,13 @@ def apply_client_processing( # noqa: C901
sep=sep,
decimal=decimal,
)
+ elif query["result_format"] == ChartDataResultFormat.XLSX:
+ read_excel_kwargs = (
+ {"index_col": 0}
+ if current_app.config["EXCEL_EXPORT"].get("index", True)
+ else {}
+ )
+ df = pd.read_excel(BytesIO(data), **read_excel_kwargs)
Review Comment:
**Suggestion:** The XLSX reader is deciding whether to consume an index column from `EXCEL_EXPORT["index"]`, but exported chart data does not reliably encode index presence from that config (it is based on the actual dataframe index shape). With the default config (`EXCEL_EXPORT` empty), this forces `index_col=0` and can drop the first real data column for files exported without an index. Derive index handling from the payload characteristics (or fallback-read logic), not from config alone. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ XLSX post-processing drops first column when index absent.
- ⚠️ Email chart XLSX attachments show misaligned columns and index.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start with the default Superset configuration where `EXCEL_EXPORT` is an empty dict
(superset/config.py:1436), so `current_app.config["EXCEL_EXPORT"].get("index", True)`
returns `True`.
2. In a test or runtime flow, construct an XLSX payload for a simple table without an
index column by calling `excel.df_to_excel(source_df, index=False)` as in
`_assert_xlsx_client_processing`
(superset/tests/unit_tests/charts/test_client_processing.py:2-18), and build `result =
{"queries": [{"result_format": ChartDataResultFormat.XLSX, "data": xlsx_bytes}]}`.
3. Call `apply_client_processing(result, form_data)` for a table viz
(superset/charts/client_processing.py:313-325), which iterates queries and reaches the
XLSX input branch at lines 360-366: `read_excel_kwargs = {"index_col": 0} if
current_app.config["EXCEL_EXPORT"].get("index", True) else {}; df =
pd.read_excel(BytesIO(data), **read_excel_kwargs)`.
4. Because the XLSX payload was written without an index column, `pd.read_excel(...,
index_col=0)` in `apply_client_processing` (client_processing.py:360-366) consumes the
first real data column as the index, dropping it from `df.columns`. The subsequent
metadata (`colnames`, `indexnames`, `rowcount`) and any XLSX post-processed exports built
from this frame will be based on a corrupted table where the first data column has been
turned into the index.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b2a553bb15d4460b94188f677ed76581&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=b2a553bb15d4460b94188f677ed76581&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/charts/client_processing.py
**Line:** 360:366
**Comment:**
*Logic Error: The XLSX reader is deciding whether to consume an index column from `EXCEL_EXPORT["index"]`, but exported chart data does not reliably encode index presence from that config (it is based on the actual dataframe index shape). With the default config (`EXCEL_EXPORT` empty), this forces `index_col=0` and can drop the first real data column for files exported without an index. Derive index handling from the payload characteristics (or fallback-read logic), not from config alone.
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%2F40885&comment_hash=e3ea4d002ee7846fa51ca302af748161c6135b9fa35218f23549e8e32f19c8e8&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=e3ea4d002ee7846fa51ca302af748161c6135b9fa35218f23549e8e32f19c8e8&reaction=dislike'>👎</a>
##########
superset/commands/report/execute.py:
##########
@@ -897,6 +934,13 @@ def _get_notification_content(self) -> NotificationContent: # noqa: C901
csv_data = self._get_csv_data()
if not csv_data:
error_text = "Unexpected missing csv file"
+ elif (
+ self._report_schedule.chart
+ and self._report_schedule.report_format == ReportDataFormat.XLSX
+ ):
+ xlsx_data = self._get_xlsx_data()
+ if not xlsx_data:
+ error_text = "Unexpected missing xlsx file"
Review Comment:
**Suggestion:** XLSX generation is wired only for chart-backed schedules, but there is no corresponding hard validation to reject dashboard schedules configured with XLSX. Since XLSX is now an accepted report format, a dashboard schedule can pass through content generation without hitting any attachment branch and send a misleading “successful” email with no file. Add server-side validation that restricts XLSX to chart targets (or raise an explicit execution error when target is unsupported). [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard XLSX reports send emails without any attachments.
- ⚠️ Users receive success notifications for unsupported XLSX dashboards.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create or update a `ReportSchedule` via the reports API using
`ReportSchedulePostSchema` or `ReportSchedulePutSchema`, setting `dashboard` to a valid
dashboard ID, leaving `chart` unset, and choosing `report_format="XLSX"`
(superset/reports/schemas.py:9-13, 173-177). The schema only validates that
`report_format` is one of `ReportDataFormat` values and does not restrict formats based on
whether a chart or dashboard is targeted.
2. Persist this schedule; the `ReportSchedule` model stores `report_format` as a string
defaulting to `ReportDataFormat.PNG` but now also allowing `XLSX`
(superset/reports/models.py:82-86, 120), with `dashboard_id` populated and `chart_id` left
`NULL`.
3. Trigger execution of this dashboard schedule so
`BaseReportState._get_notification_content` runs
(superset/commands/report/execute.py:903-123). In the attachment routing block
(execute.py:919-985), PNG and PDF formats are handled unconditionally, CSV and XLSX
formats are only handled when `self._report_schedule.chart` is truthy
(execute.py:931-941), and there is no branch for a dashboard with `ReportDataFormat.XLSX`.
For the dashboard-only schedule, the XLSX branch at lines 937-943 is skipped, leaving
`xlsx_data` as `None` and `error_text` unset.
4. `_get_notification_content` falls through to build a `NotificationContent` with a name
derived from the dashboard title and `xlsx=None` (execute.py:102-121). The email
notification plugin attaches XLSX data only if `content.xlsx` is set
(superset/reports/notifications/email.py:106-118), so the schedule sends a "successful"
dashboard email that advertises XLSX format in `header_data["notification_format"]` but
contains no XLSX attachment and no explicit error explaining that dashboards do not
support XLSX.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bf2d002ee0724a449f3c4ab450f86d14&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=bf2d002ee0724a449f3c4ab450f86d14&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/commands/report/execute.py
**Line:** 937:943
**Comment:**
*Incomplete Implementation: XLSX generation is wired only for chart-backed schedules, but there is no corresponding hard validation to reject dashboard schedules configured with XLSX. Since XLSX is now an accepted report format, a dashboard schedule can pass through content generation without hitting any attachment branch and send a misleading “successful” email with no file. Add server-side validation that restricts XLSX to chart targets (or raise an explicit execution error when target is unsupported).
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%2F40885&comment_hash=8c672e32cf3911b9bc49f007d8bffcb9ffcde319db2e3b964912967e05e7ed81&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=8c672e32cf3911b9bc49f007d8bffcb9ffcde319db2e3b964912967e05e7ed81&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601562002
##########
superset/charts/client_processing.py:
##########
@@ -310,6 +310,33 @@ def table(
}
+def _is_default_index_column(series: pd.Series) -> bool:
+ return series.tolist() == list(range(len(series)))
+
+
+def _read_excel_for_client_processing(
+ data: bytes,
+ form_data: dict[str, Any],
+) -> pd.DataFrame:
+ df = pd.read_excel(BytesIO(data))
+ if len(df.columns) == 0:
+ return df
+
+ first_column = df.columns[0]
+ expected_columns = {
+ *get_column_names(form_data.get("columns")),
+ *get_metric_names(form_data.get("metrics")),
+ }
+
+ if first_column in expected_columns:
+ return df
+
+ if _is_default_index_column(df.iloc[:, 0]):
+ return df.iloc[:, 1:].reset_index(drop=True)
+
+ return df.set_index(first_column)
Review Comment:
Good catch, thanks. This was a valid edge case for pivot_table_v2 exports.
I updated the XLSX re-import expected-column detection to include `groupbyRows` and `groupbyColumns` in addition to `columns` and `metrics`, so real pivot dimensions are not mistaken for an export-added index column. I also added a regression test covering XLSX client processing for pivot tables with row/column groupbys.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3602444755
##########
tests/unit_tests/utils/test_core.py:
##########
@@ -70,6 +71,39 @@
}
+@pytest.mark.parametrize(
+ ("name", "content_type"),
+ [
+ (
+ "report.xlsx",
+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+ ),
+ ("report.zip", "application/zip"),
+ ],
+)
+def test_build_email_attachment(
+ name: str,
+ content_type: str,
+) -> None:
+ """Email attachments should expose the expected MIME type and filename."""
+ attachment = build_email_attachment(name, b"attachment")
+
+ assert attachment.get_content_type() == content_type
+ assert attachment.get_filename() == name
+ assert attachment.get_payload(decode=True) == b"attachment"
+
+
+def test_build_email_attachment_encodes_text_payload() -> None:
+ """Text attachment bodies should be encoded to stable UTF-8 bytes."""
+ attachment = build_email_attachment("report.csv", "город,value\nМосква,1")
+
+ assert attachment.get_content_type() == "application/octet-stream"
+ assert attachment.get_filename() == "report.csv"
+ assert attachment.get_payload(decode=True) == "город,value\nМосква,1".encode(
+ "utf-8"
+ )
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Semantic test duplication</b></div>
<div id="fix">
This test duplicates existing assertion at line 93 that also checks `get_payload(decode=True)`. Both tests verify identical email library behavior from different input types. Consider parameterizing the existing `test_build_email_attachment` to cover all three cases (bytes ASCII, bytes UTF-8, str UTF-8) instead of maintaining separate test functions with overlapping coverage.
</div>
</div>
<small><i>Code Review Run #2b27dd</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3570933364
##########
superset/reports/notifications/email.py:
##########
@@ -72,11 +74,46 @@
class EmailContent:
body: str
header_data: Optional[HeaderDataType] = None
- data: Optional[dict[str, Any]] = None
+ data: Optional[dict[str, bytes | str]] = None
pdf: Optional[dict[str, bytes]] = None
images: Optional[dict[str, bytes]] = None
+def _get_xlsx_attachment_extension(content: bytes) -> str:
+ """
+ Return the attachment extension for bytes returned by the XLSX export endpoint.
+ """
+ try:
+ with ZipFile(BytesIO(content)) as zip_file:
+ names = zip_file.namelist()
+ if _is_xlsx_zip(names):
+ return "xlsx"
+
+ files = [name for name in names if not name.endswith("/")]
+ if files and all(name.lower().endswith(".xlsx") for name in files):
+ for name in files:
+ with zip_file.open(name) as xlsx_file:
+ if not _is_xlsx_zipfile(xlsx_file.read()):
+ return "xlsx"
Review Comment:
**Suggestion:** The XLSX/ZIP type detection reads every embedded XLSX file fully into memory before deciding the extension. For server-paginated exports with many large workbooks, this can cause high memory spikes and slow email delivery. Avoid fully reading each nested file; classify using lightweight metadata (for example, trust the outer archive structure/filenames or inspect only minimal signatures) so extension detection remains bounded. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Email report worker allocates extra memory per attachment.
- ⚠️ Large server-paginated XLSX reports see slower emails.
- ⚠️ Increased memory pressure on workers under heavy workloads.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Generate a large server-paginated XLSX export where each page is written as a separate
workbook and bundled into a ZIP, using `create_zip` in `superset/utils/core.py:2128-2134`
with many sizable XLSX byte blobs (e.g. produced via `df_to_excel` in
`superset/utils/excel.py:70`).
2. Construct a `NotificationContent` instance with its `xlsx` field set to this ZIP
archive, as shown in the helper `_make_notification` in
`superset/tests/unit_tests/reports/notifications/email_tests.py:5-27`, and pass it to
`EmailNotification(recipient, content)`.
3. Call `EmailNotification._get_content()` in
`superset/reports/notifications/email.py:167-259`; this method detects the XLSX attachment
extension by invoking `_get_xlsx_attachment_extension(self._content.xlsx)` at
`superset/reports/notifications/email.py:249-251`.
4. Inside `_get_xlsx_attachment_extension`
(`superset/reports/notifications/email.py:82-102`), for ZIPs where all entries end with
`.xlsx`, the loop at lines 94-97 opens each embedded workbook and calls
`_is_xlsx_zipfile(xlsx_file.read())`, fully reading and decompressing every XLSX file into
memory and then re-wrapping it in another `ZipFile`, causing memory usage and latency to
scale with the total uncompressed size of all workbooks in the server-paginated export.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=635c2314ec2040b58d014ed0570e1798&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=635c2314ec2040b58d014ed0570e1798&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/reports/notifications/email.py
**Line:** 94:97
**Comment:**
*Performance: The XLSX/ZIP type detection reads every embedded XLSX file fully into memory before deciding the extension. For server-paginated exports with many large workbooks, this can cause high memory spikes and slow email delivery. Avoid fully reading each nested file; classify using lightweight metadata (for example, trust the outer archive structure/filenames or inspect only minimal signatures) so extension detection remains bounded.
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%2F40885&comment_hash=ade0d9ab97d29952051e8eb39280cb023b2e266def8e14107ec2847d24861a78&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=ade0d9ab97d29952051e8eb39280cb023b2e266def8e14107ec2847d24861a78&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3379998213
##########
superset/translations/th/LC_MESSAGES/messages.po:
##########
@@ -1456,6 +1462,11 @@ msgstr "เกิดการหมดเวลาขณะสร้างไฟ
msgid "A timeout occurred while generating a dataframe."
msgstr "เกิดการหมดเวลาขณะสร้าง dataframe"
+# Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
+# de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
+msgid "A timeout occurred while generating an xlsx."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation after fuzzy removal</b></div>
<div id="fix">
Removing the `fuzzy` flag from "A timeout occurred while generating an xlsx." leaves `msgstr` empty (line 1468). Per the documented workflow in `howtos.md` (lines 401-410), the `fuzzy` flag signals that a machine-generated translation needs human review before use. Removing it with an empty msgstr causes the English string to display to Thai users instead of a translated message.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/translations/th/LC_MESSAGES/messages.po (lines 1465-1469) ---
1465: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
1466: # de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
1467: -#, fuzzy
1467: +#, fuzzy
1468: msgid "A timeout occurred while generating an xlsx."
1469: -msgstr ""
1469: +msgstr "เกิดการหมดเวลาขณะสร้างไฟล์ xlsx"
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/870adf4/docs/developer_docs/contributing/howtos.md">howtos.md:403-408</a>
</li>
</ul>
</details>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/th/LC_MESSAGES/messages.po:
##########
@@ -21486,6 +21517,11 @@ msgstr ""
msgid "Send as CSV"
msgstr "ส่งเป็น CSV"
+# Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
+# de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
+msgid "Send as Excel"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation after fuzzy removal</b></div>
<div id="fix">
Removing the `fuzzy` flag from "Send as Excel" leaves `msgstr` empty (line 21524). Per the documented workflow in `howtos.md` (lines 401-410), the `fuzzy` flag signals that a machine-generated translation needs human review before use. Removing it with an empty msgstr causes the English string to display to Thai users instead of a translated message.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/translations/th/LC_MESSAGES/messages.po (lines 21520-21527) ---
21520: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
21521: # de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
21522: msgid "Send as Excel"
21523: +#, fuzzy
21524: msgstr ""
21525: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ca, cs, de,
21526: # es, fa, fr, ja, lv, mi, ru, sk, sl, uk]
21527: -msgstr "ส่งเป็นข้อความ"
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/870adf4/docs/developer_docs/contributing/howtos.md">howtos.md:403-408</a>
</li>
</ul>
</details>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/lv/LC_MESSAGES/messages.po:
##########
@@ -6219,6 +6226,9 @@ msgstr "Piemērs"
msgid "Examples"
msgstr "Piemēri"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation for UI label</b></div>
<div id="fix">
The msgstr is now empty, causing English text to display in the UI. This is used in ReportModal/index.tsx:286 and AlertReportModal.tsx. Suggest: 'Excel (XLSX) pielikums e-pastā'
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/lv/LC_MESSAGES/messages.po:
##########
@@ -965,6 +969,9 @@ msgstr "Ģenerējot CSV, iestājās noildze."
msgid "A timeout occurred while generating a dataframe."
msgstr "Ģenerējot datu rāmi, iestājās noildze."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation for user message</b></div>
<div id="fix">
The msgstr is now empty, causing English text to display to Latvian users. This is an actively used error message in superset/commands/report/exceptions.py:284. Suggest: 'Ģenerējot xlsx, iestājās noildze.'
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/lv/LC_MESSAGES/messages.po:
##########
@@ -11948,6 +11971,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "Sūtīt kā CSV"
+msgid "Send as Excel"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation for menu item</b></div>
<div id="fix">
The msgstr is now empty, causing English text to display in the dropdown menu. This is used in AlertReportModal.tsx:225. Suggest: 'Sūtīt kā Excel'
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/th/LC_MESSAGES/messages.po:
##########
@@ -10926,6 +10937,11 @@ msgstr "ตัวอย่าง"
msgid "Examples"
msgstr "ตัวอย่าง"
+# Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
+# de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
+msgid "Excel (XLSX) attached in email"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation after fuzzy removal</b></div>
<div id="fix">
Removing the `fuzzy` flag from "Excel (XLSX) attached in email" leaves `msgstr` empty (line 10944). Per the documented workflow in `howtos.md` (lines 401-410), the `fuzzy` flag signals that a machine-generated translation needs human review before use. Removing it with an empty msgstr causes the English string to display to Thai users instead of a translated message.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/translations/th/LC_MESSAGES/messages.po (lines 10940-10946) ---
10940: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
10941: # de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
10942: msgid "Excel (XLSX) attached in email"
10943: -#, fuzzy
10943: +#, fuzzy
10944: msgstr ""
10945: -msgstr "แนบ CSV ที่จัดรูปแบบแล้วในอีเมล"
10946: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: fr, ja, lv,
10947: # ru, uk]
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/870adf4/docs/developer_docs/contributing/howtos.md">howtos.md:403-408</a>
</li>
</ul>
</details>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/th/LC_MESSAGES/messages.po:
##########
@@ -19153,6 +19175,11 @@ msgstr "การดำเนินการกำหนดการรายง
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "การดำเนินการกำหนดการรายงานล้มเหลวขณะสร้างภาพหน้าจอ"
+# Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
+# de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation after fuzzy removal</b></div>
<div id="fix">
Removing the `fuzzy` flag from "Report Schedule execution failed when generating an xlsx." leaves `msgstr` empty (line 19182). Per the documented workflow in `howtos.md` (lines 401-410), the `fuzzy` flag signals that a machine-generated translation needs human review before use. Removing it with an empty msgstr causes the English string to display to Thai users instead of a translated message.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/translations/th/LC_MESSAGES/messages.po (lines 19178-19184) ---
19178: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
19179: # de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
19180: msgid "Report Schedule execution failed when generating an xlsx."
19181: +#, fuzzy
19182: msgstr ""
19183: # Machine-translated via backfill_po.py (claude-sonnet-4-6) [refs: ar, ca, cs,
19184: -msgstr "การดำเนินการกำหนดการรายงานล้มเหลวขณะสร้างไฟล์ CSV"
19184: +#, fuzzy
19185: # de, es, fa, fr, ja, lv, mi, nl, pl, pt_BR, ru, sk, sl, uk, zh, zh_TW]
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/870adf4/docs/developer_docs/contributing/howtos.md">howtos.md:403-408</a>
</li>
</ul>
</details>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/ja/LC_MESSAGES/messages.po:
##########
@@ -5837,6 +5844,9 @@ msgstr "例"
msgid "Examples"
msgstr "サンプル"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Japanese translation</b></div>
<div id="fix">
Empty `msgstr` at line 5848 causes English fallback. This label is used in `AlertReportModal.tsx` and `ReportModal.tsx` as a user-selectable option, so Japanese users will see the English label in dropdowns.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/ja/LC_MESSAGES/messages.po:
##########
@@ -882,6 +886,9 @@ msgstr "CSVの生成中にタイムアウトが発生しました。"
msgid "A timeout occurred while generating a dataframe."
msgstr "データフレームの生成中にタイムアウトが発生しました。"
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Japanese translation</b></div>
<div id="fix">
Empty `msgstr` at line 890 causes English fallback in Japanese UI. This message is used in `superset/commands/report/exceptions.py` and will display the English string to Japanese users instead of a translated error message.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/ja/LC_MESSAGES/messages.po:
##########
@@ -10096,6 +10112,9 @@ msgstr "PDF生成時にレポートスケジュールの実行が失敗しまし
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "スクリーンショット生成時にレポートスケジュールの実行が失敗しました。"
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+
msgid "Report Schedule execution got an unexpected error."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Japanese translation</b></div>
<div id="fix">
Empty `msgstr` at line 10116 causes English fallback. This error is thrown by `ReportScheduleXlsxFailedError` in `exceptions.py` and displayed to users when XLSX report generation fails.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/ja/LC_MESSAGES/messages.po:
##########
@@ -11327,6 +11350,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "CSVとして送信"
+msgid "Send as Excel"
+msgstr ""
+
msgid "Send as PDF"
msgstr "PDFとして送信"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Japanese translation</b></div>
<div id="fix">
Empty `msgstr` at line 11354 causes English fallback. This label is used in `AlertReportModal.tsx:225` and `AlertReportModal.test.tsx:770` as a dropdown option, so Japanese users will see 'Send as Excel' in English.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -206,6 +206,10 @@ msgstr "%(dialect)s به دلایل امنیتی نمیتواند به عنو
msgid "%(label)s file"
msgstr "فایل %(type)s"
+#, fuzzy, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "ابعاد"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect translation placeholder</b></div>
<div id="fix">
The translation 'ابعاد' (dimensions) has no semantic relation to the msgid '%(name)s.%(extension)s' which represents a filename with extension. Users will see nonsensical output.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -12203,6 +12229,10 @@ msgstr ""
msgid "Send as CSV"
msgstr "بهعنوان CSV ارسال کنید"
+#, fuzzy
+msgid "Send as Excel"
+msgstr "متن را ارسال کنید"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Fuzzy translation with imprecise msgstr</b></div>
<div id="fix">
The `#, fuzzy` flag indicates this translation is uncertain and needs review. Combined with an imprecise msgstr that drops the 'Excel' format specification, Persian users will see misleading text. Either provide an accurate translation and remove the fuzzy flag, or leave msgstr empty with a clear comment explaining the pending translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -6312,6 +6320,10 @@ msgstr "مثال"
msgid "Examples"
msgstr "نمونهها"
+#, fuzzy
+msgid "Excel (XLSX) attached in email"
+msgstr "فرمتشده CSV در ایمیل پیوست شده است"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong file format in translation</b></div>
<div id="fix">
The translation incorrectly says 'CSV' instead of 'XLSX'. The msgid specifically mentions 'Excel (XLSX)' not 'CSV'.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -10978,6 +11000,10 @@ msgstr "مقادیر کنترل مورد نیاز حذف شدهاند"
msgid "Resample"
msgstr "نمونهبرداری مجدد"
+#, python-format
+msgid "Resample method '%(method)s' is not supported."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty msgstr for user-facing error messages</b></div>
<div id="fix">
Three new translation entries have empty msgstr values (lines 11005, 15397, 15403). These error messages from `superset/viz.py` will display in English to Persian users at runtime unless translated. Consider providing translations or adding translator notes explaining the status.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -932,6 +936,10 @@ msgstr "در حین ساخت csv، یک تایم اوت رخ داد."
msgid "A timeout occurred while generating a dataframe."
msgstr "در حین تولید یک دیتافریم، یک تایم اوت رخ داد."
+#, fuzzy
+msgid "A timeout occurred while generating an xlsx."
+msgstr "در حین ساخت csv، یک تایم اوت رخ داد."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong file format in translation</b></div>
<div id="fix">
The translation incorrectly says 'csv' instead of 'xlsx'. This message should say 'A timeout occurred while generating an xlsx' not 'csv'.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/cs/LC_MESSAGES/messages.po:
##########
@@ -6346,6 +6353,9 @@ msgstr "Příklad"
msgid "Examples"
msgstr "Příklady"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
#, fuzzy
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation introduced</b></div>
<div id="fix">
Removing `fuzzy` flag while leaving `msgstr` empty marks this as translated with an empty string. Users will see a blank email attachment description instead of the English source text or a proper Czech translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/cs/LC_MESSAGES/messages.po:
##########
@@ -965,6 +969,9 @@ msgstr "Při generování CSV došlo k vypršení časového limitu."
msgid "A timeout occurred while generating a dataframe."
msgstr "Při generování datového rámce došlo k vypršení časového limitu."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation introduced</b></div>
<div id="fix">
Removing `fuzzy` flag while leaving `msgstr` empty causes this entry to be marked as translated with an empty string. Per `build_translation_index.py` logic, empty msgstr without fuzzy flag = 'translated' (indexed as empty string). Czech users will see a blank instead of either the English fallback or a proper translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/cs/LC_MESSAGES/messages.po:
##########
@@ -10899,6 +10915,9 @@ msgstr "Provedení plánu reportu selhalo při generování PDF."
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "Provedení plánu reportu selhalo při generování snímku obrazovky."
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+
msgid "Report Schedule execution got an unexpected error."
msgstr "Při provádění plánu reportu došlo k neočekávané chybě."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation introduced</b></div>
<div id="fix">
Adding empty `msgstr` without fuzzy flag marks this as translated with an empty string. Users will see a blank error message when XLSX report generation times out instead of the English source text or proper Czech translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/cs/LC_MESSAGES/messages.po:
##########
@@ -12215,6 +12238,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "Odeslat jako CSV"
+msgid "Send as Excel"
+msgstr ""
+
msgid "Send as PDF"
msgstr "Odeslat jako PDF"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation introduced</b></div>
<div id="fix">
Adding empty `msgstr` without fuzzy flag marks this as translated with an empty string. Users will see a blank label in the report delivery options UI instead of the English source text or proper Czech translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/tr/LC_MESSAGES/messages.po:
##########
@@ -180,6 +180,10 @@ msgstr ""
msgid "%(label)s file"
msgstr ""
+#, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "%(name)s.%(extension)s"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation regression: valid Turkish lost</b></div>
<div id="fix">
This change regresses a valid Turkish translation. The previous value "son kullanılanlar" (meaning "recently used") was a proper Turkish translation that is now lost. Replacing it with the English msgid copy means users see untranslated text instead of Turkish. Either restore the original translation with the fuzzy flag, or provide a proper Turkish translation.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/translations/tr/LC_MESSAGES/messages.po (lines 183-185) ---
183: #, fuzzy, python-format
184: msgid "%(name)s.%(extension)s"
185: -msgstr "%(name)s.%(extension)s"
185: +msgstr "son kullanılanlar"
```
</div>
</details>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/it/LC_MESSAGES/messages.po:
##########
@@ -182,6 +182,10 @@ msgstr ""
msgid "%(label)s file"
msgstr ""
+#, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "%(name)s.%(extension)s"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Placeholder translation for file name</b></div>
<div id="fix">
The `msgstr` for `%(name)s.%(extension)s` uses the English placeholder instead of an Italian translation. While this is a valid fallback pattern in gettext, Italian users will see English text. Consider providing a proper Italian translation or at minimum prefixing with "file" to indicate context.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/it/LC_MESSAGES/messages.po:
##########
@@ -875,6 +879,9 @@ msgstr "Errore nel recupero dei metadati della tabella"
msgid "A timeout occurred while generating a dataframe."
msgstr "Errore nel creare il datasource"
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Untranslated xlsx timeout error</b></div>
<div id="fix">
The `msgstr` for the xlsx timeout error is empty, leaving Italian users with untranslated English text. Similar timeout messages in this file have proper Italian translations (e.g., 'Errore nel recupero dei metadati della tabella' for csv, 'Errore nel creare il datasource' for dataframe). This inconsistency should be addressed.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/nl/LC_MESSAGES/messages.po:
##########
@@ -6416,6 +6423,9 @@ msgstr "Voorbeeld"
msgid "Examples"
msgstr "Voorbeelden"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
#, fuzzy
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Dutch translation</b></div>
<div id="fix">
Removing the fuzzy flag exposes an empty msgstr, causing 'Excel (XLSX) attached in email' to display untranslated to Dutch users. Line 6427 should contain the Dutch equivalent.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/nl/LC_MESSAGES/messages.po:
##########
@@ -919,6 +923,9 @@ msgstr "Er is een time-out opgetreden tijdens het genereren van een csv."
msgid "A timeout occurred while generating a dataframe."
msgstr "Er is een time-out opgetreden tijdens het genereren van een dataframe."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Dutch translation</b></div>
<div id="fix">
Removing the fuzzy flag exposes an empty msgstr, causing the English text to display to Dutch users. Line 927 should contain a Dutch translation following the established pattern from lines 918-924 (e.g., "Er is een time-out opgetreden tijdens het genereren van een xlsx-bestand.").
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/nl/LC_MESSAGES/messages.po:
##########
@@ -11042,6 +11058,9 @@ msgstr ""
"Rapportage planning uitvoering mislukt bij het genereren van een "
"screenshot."
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+
msgid "Report Schedule execution got an unexpected error."
msgstr "Rapportage planning uitvoering kreeg een onverwachte fout."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Dutch translation</b></div>
<div id="fix">
Removing the fuzzy flag exposes an empty msgstr at line 11062, causing the English error message to display to Dutch users. The translation should follow the established pattern from line 11057-11059 (e.g., "Rapportage planning uitvoering mislukt bij het genereren van een xlsx-bestand.").
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/nl/LC_MESSAGES/messages.po:
##########
@@ -12382,6 +12405,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "Verstuur als CSV"
+msgid "Send as Excel"
+msgstr ""
+
#, fuzzy
msgid "Send as PDF"
msgstr "Verstuur als PNG"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Dutch translation</b></div>
<div id="fix">
Removing the fuzzy flag exposes an empty msgstr at line 12409, causing 'Send as Excel' to display untranslated. Following the established pattern from line 12405-12406 ("Verstuur als CSV"), line 12409 should be "Verstuur als Excel".
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -188,6 +188,10 @@ msgstr "出於安全原因，%(dialect)s SQLite 資料庫不能用作數據源
msgid "%(label)s file"
msgstr ""
+#, fuzzy, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "维度"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation: Incorrect msgstr</b></div>
<div id="fix">
The translation '维度' (dimension) is semantically incorrect for a filename pattern like '%(name)s.%(extension)s'. This will display confusing output to users. The translation should reflect a filename pattern.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -885,6 +889,10 @@ msgstr "生成 CSV 時超時。"
msgid "A timeout occurred while generating a dataframe."
msgstr "生成數據超時"
+#, fuzzy
+msgid "A timeout occurred while generating an xlsx."
+msgstr "生成 CSV 時超時。"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation: Copy-paste error</b></div>
<div id="fix">
Copy-paste error: the translation references CSV but the msgid is for xlsx. Users will see incorrect message '生成 CSV 時超時' when an xlsx timeout occurs.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -6290,6 +6298,10 @@ msgstr "例子"
msgid "Examples"
msgstr "示例"
+#, fuzzy
+msgid "Excel (XLSX) attached in email"
+msgstr "郵件中附上格式化好的CSV"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation: Copy-paste error</b></div>
<div id="fix">
Copy-paste error: the translation references CSV but the msgid is 'Excel (XLSX) attached in email'. Users will see incorrect message about CSV when Excel is attached.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4656914330
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | a3acdce84f001c89cb39cfaaa2c7deeb8926c1e7 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a27b697e8a36900080d255d |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40885--superset-docs-preview.netlify.app](https://deploy-preview-40885--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8y6UAp_3aBjjGkUmcokYUR3z91bvBcypTMXmAA6sIRx2Zr3GNONdLDVY2E1XLUnnxOVlpzPIT2za03ioKw0ymscpeGca-4GIRx8Pw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40885--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-5002179513
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4736585140
Really nice work here, @innovark37. The build_email_attachment extraction and reusing the existing CSV fetch path for XLSX both look good.
That said, it's (significantly) conflicting with master and needs a rebase. Looks like all a bunch of messages.po files got regenerated. Can you drop the per-language .po churn and just leave the source string changes? A rebase may clear some of that out, I'm not sure...
Also, in email.py, `_get_xlsx_attachment_extension` falls back to .zip when the bytes aren't a valid xlsx zip. When does the XLSX export endpoint actually hand back a zip rather than a single workbook? Just want to make sure we're not covering over an error payload by renaming it .zip.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3571237167
##########
superset/charts/client_processing.py:
##########
@@ -403,5 +410,11 @@ def apply_client_processing( # noqa: C901
index=show_default_index,
**current_app.config["CSV_EXPORT"],
)
+ elif query["result_format"] == ChartDataResultFormat.XLSX:
+ excel.apply_column_types(processed_df, query["coltypes"])
+ query["data"] = excel.df_to_excel(
+ processed_df,
+ **current_app.config["EXCEL_EXPORT"],
+ )
Review Comment:
Fixed.
The XLSX serialization path now passes the same computed index policy used by the CSV branch (`show_default_index`). This suppresses the default `RangeIndex` for XLSX attachments while still preserving a meaningful/non-default index.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601316015
##########
superset/charts/client_processing.py:
##########
@@ -310,6 +310,33 @@ def table(
}
+def _is_default_index_column(series: pd.Series) -> bool:
+ return series.tolist() == list(range(len(series)))
+
+
+def _read_excel_for_client_processing(
+ data: bytes,
+ form_data: dict[str, Any],
+) -> pd.DataFrame:
+ df = pd.read_excel(BytesIO(data))
+ if len(df.columns) == 0:
+ return df
+
+ first_column = df.columns[0]
+ expected_columns = {
+ *get_column_names(form_data.get("columns")),
+ *get_metric_names(form_data.get("metrics")),
+ }
+
+ if first_column in expected_columns:
+ return df
+
+ if _is_default_index_column(df.iloc[:, 0]):
+ return df.iloc[:, 1:].reset_index(drop=True)
+
+ return df.set_index(first_column)
Review Comment:
**Suggestion:** The XLSX re-import logic infers expected columns only from `columns` and `metrics`, but pivot-table charts use `groupbyRows`/`groupbyColumns`; this causes real dimension columns to be misclassified as index columns and removed before post-processing, which can trigger missing-column failures or incorrect output. Build the expected-column set with the pivot-table field names as well (and only strip an index column when it is explicitly recognized as an export-added index). [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Pivot-table XLSX reports fail with missing-column errors.
❌ Excel attachments omit pivot dimensions or crash generation.
⚠️ XLSX exports inconsistent with CSV and JSON pivot output.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a chart with viz_type "pivot_table_v2" using non-empty groupbyRows /
groupbyColumns and metrics, then schedule an XLSX email report so
BaseReportState._get_notification_content() runs the tabular chart path
(superset/commands/report/execute.py:26-68).
2. When report_format is ReportDataFormat.XLSX and a chart is present,
_get_notification_content() calls self._get_data(ChartDataResultFormat.XLSX)
(superset/commands/report/execute.py:57-60), which builds a POST_PROCESSED request payload
and sends it to ChartDataRestApi.get_data (verified in tests at
tests/unit_tests/commands/report/execute_test.py:124-37).
3. In ChartDataRestApi._send_chart_response() (superset/charts/data/api.py:11-18), when
result_type == ChartDataResultType.POST_PROCESSED it invokes
apply_client_processing(result, form_data, datasource), and for XLSX result_format
apply_client_processing() (superset/charts/client_processing.py:81-137) passes the Excel
bytes into _read_excel_for_client_processing(data, form_data) (lines 58-78).
4. _read_excel_for_client_processing() builds expected_columns only from columns and
metrics (superset/charts/client_processing.py:67-70), so for pivot_table_v2 where
dimensions live in groupbyRows/groupbyColumns the first dimension column is not in
expected_columns, causing df.set_index(first_column) (line 78); pivot_table_v2() then
calls pivot_df() with rows from groupbyRows (lines 40-55), but those row labels no longer
exist as DataFrame columns, triggering a KeyError during df.pivot_table(...) and causing
the XLSX report generation request to fail.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=39fe7022c0db41e9a548b73cbb98b10c&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=39fe7022c0db41e9a548b73cbb98b10c&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/charts/client_processing.py
**Line:** 326:337
**Comment:**
*Incorrect Condition Logic: The XLSX re-import logic infers expected columns only from `columns` and `metrics`, but pivot-table charts use `groupbyRows`/`groupbyColumns`; this causes real dimension columns to be misclassified as index columns and removed before post-processing, which can trigger missing-column failures or incorrect output. Build the expected-column set with the pivot-table field names as well (and only strip an index column when it is explicitly recognized as an export-added index).
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%2F40885&comment_hash=efbfce6c8da73a95cbf1261fb8a0fb652f401ebbcec0e9200569b853d2d90e6a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=efbfce6c8da73a95cbf1261fb8a0fb652f401ebbcec0e9200569b853d2d90e6a&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4657263467
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40885?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `52.17391%` with `33 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 63.34%. Comparing base ([`c58408d`](https://app.codecov.io/gh/apache/superset/commit/c58408d76c43194a7f0de0f613e9615234dd89c4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5e8cc55`](https://app.codecov.io/gh/apache/superset/commit/5e8cc55cd3aec87015aebeea3e940f29953b0df8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40885?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/commands/report/execute.py](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&filepath=superset%2Fcommands%2Freport%2Fexecute.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvcmVwb3J0L2V4ZWN1dGUucHk=) | 44.44% | [13 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/reports/notifications/email.py](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&filepath=superset%2Freports%2Fnotifications%2Femail.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2VtYWlsLnB5) | 38.88% | [10 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/charts/client\_processing.py](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&filepath=superset%2Fcharts%2Fclient_processing.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY2hhcnRzL2NsaWVudF9wcm9jZXNzaW5nLnB5) | 25.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/utils/core.py](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&filepath=superset%2Futils%2Fcore.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | 88.88% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40885?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40885 +/- ##
==========================================
- Coverage 64.13% 63.34% -0.80%
==========================================
Files 2653 2653
Lines 143569 143623 +54
Branches 33126 33132 +6
==========================================
- Hits 92084 90971 -1113
- Misses 49873 51030 +1157
- Partials 1612 1622 +10
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.49% <27.53%> (-0.02%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.09% <27.53%> (-0.02%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.09% <52.17%> (-1.71%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.92% <52.17%> (-0.02%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40885/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/40885?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4671299399
Hi @innovark37 , I wanted to say thanks for your contributions to Superset! They have been great. If you ever want to discuss how you fit into the project, your goals or interests in contributing, etc. feel free to connect with me or @rusackas on Slack, LinkedIn, email etc.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3570895764
##########
superset/charts/client_processing.py:
##########
@@ -357,6 +357,13 @@ def apply_client_processing( # noqa: C901
sep=sep,
decimal=decimal,
)
+ elif query["result_format"] == ChartDataResultFormat.XLSX:
+ read_excel_kwargs = (
+ {"index_col": 0}
+ if current_app.config["EXCEL_EXPORT"].get("index", True)
+ else {}
Review Comment:
**Suggestion:** Add an explicit type annotation to `read_excel_kwargs` so this newly introduced variable is type-hinted in line with the typing rule. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The new local variable `read_excel_kwargs` is introduced without any type annotation, and it is a variable that can reasonably be annotated (e.g. as a mapping/dict). This matches the Python type-hint requirement for newly added 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=dd3f8f4f11874c47801ce152bc02d127&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=dd3f8f4f11874c47801ce152bc02d127&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/charts/client_processing.py
**Line:** 360:364
**Comment:**
*Custom Rule: Add an explicit type annotation to `read_excel_kwargs` so this newly introduced variable is type-hinted in line with the typing rule.
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%2F40885&comment_hash=d478f6b302f860b7921cb5d77a2dd2405b2d1315d4ca5cb974bfd3fd4fe1d46e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=d478f6b302f860b7921cb5d77a2dd2405b2d1315d4ca5cb974bfd3fd4fe1d46e&reaction=dislike'>👎</a>
##########
superset/commands/report/execute.py:
##########
@@ -871,6 +907,7 @@ def _get_notification_content(self) -> NotificationContent: # noqa: C901
:raises: ReportScheduleScreenshotFailedError
"""
csv_data = None
+ xlsx_data = None
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced optional attachment variable to comply with the type-hint requirement. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The newly introduced local variable is assigned `None` without a type annotation in Python code that otherwise uses type hints. This matches the custom rule requiring type hints on new or modified Python code where relevant variables can be annotated.
</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=36fb6d189944411ca47652cff1a05a1a&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=36fb6d189944411ca47652cff1a05a1a&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/commands/report/execute.py
**Line:** 910:910
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced optional attachment variable to comply with 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%2F40885&comment_hash=913b6861570a9f46f247295fbd935a70ba271a3d5fb8bc72fa225cf64d00610f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=913b6861570a9f46f247295fbd935a70ba271a3d5fb8bc72fa225cf64d00610f&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3571249998
##########
superset/charts/client_processing.py:
##########
@@ -357,6 +357,13 @@ def apply_client_processing( # noqa: C901
sep=sep,
decimal=decimal,
)
+ elif query["result_format"] == ChartDataResultFormat.XLSX:
+ read_excel_kwargs = (
+ {"index_col": 0}
+ if current_app.config["EXCEL_EXPORT"].get("index", True)
+ else {}
+ )
+ df = pd.read_excel(BytesIO(data), **read_excel_kwargs)
Review Comment:
Fixed.
The XLSX reader no longer uses `EXCEL_EXPORT["index"]` to decide whether to consume the first column as an index. It now reads the payload first and derives index handling from the XLSX structure plus the expected columns/metrics from `form_data`, so XLSX files exported without an index keep their first real data column.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3378610007
##########
superset/commands/report/execute.py:
##########
@@ -552,6 +567,19 @@ def _get_csv_data(self) -> bytes:
raise ReportScheduleCsvFailedError()
return csv_data
+ def _get_xlsx_data(self) -> bytes:
+ try:
+ xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
+ except SoftTimeLimitExceeded as ex:
+ raise ReportScheduleXlsxTimeout() from ex
+ except Exception as ex:
+ raise ReportScheduleXlsxFailedError(
+ f"Failed generating xlsx {str(ex)}"
+ ) from ex
+ if not xlsx_data:
+ raise ReportScheduleXlsxFailedError()
+ return xlsx_data
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing timeout diagnostic logging</b></div>
<div id="fix">
The `_get_xlsx_data` handler for `SoftTimeLimitExceeded` (line 573) lacks the elapsed-time logging that `_get_csv_data` provides. Without this, timeouts are harder to diagnose in production since operators won't know how long the operation ran.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -567,8 +567,13 @@ class BaseReportState:
def _get_xlsx_data(self) -> bytes:
try:
+ start_time = datetime.utcnow()
xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
except SoftTimeLimitExceeded as ex:
+ elapsed_seconds = (datetime.utcnow() - start_time).total_seconds()
+ logger.warning(
+ "XLSX generation timeout after %.2fs - execution_id: %s",
+ elapsed_seconds,
+ self._execution_id,
+ )
raise ReportScheduleXlsxTimeout() from ex
except Exception as ex:
raise ReportScheduleXlsxFailedError(
```
</div>
</details>
</div>
<small><i>Code Review Run #077cf0</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3382195656
##########
superset/translations/es/LC_MESSAGES/messages.po:
##########
@@ -969,6 +973,9 @@ msgstr "Se ha agotado el tiempo de espera al generar un CSV."
msgid "A timeout occurred while generating a dataframe."
msgstr "Se ha agotado el tiempo de espera al generar un marco de datos."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Spanish translation</b></div>
<div id="fix">
The `#, fuzzy` flag was removed, indicating this translation is approved for production, but `msgstr` remains empty. Spanish users will see the untranslated English string in the UI. Compare with the CSV variant at line 970 which has a proper translation.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -12573,6 +12596,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "Enviar como CSV"
+msgid "Send as Excel"
+msgstr ""
+
#, fuzzy
msgid "Send as PDF"
msgstr "Enviar como PNG"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent fuzzy flag and translation state</b></div>
<div id="fix">
The diff claims to remove #, fuzzy from 'Send as Excel' (line 12602), but the actual file shows #, fuzzy is on 'Send as PDF' (line 12602-12603). The fuzzy flag was not removed from the intended entry. Additionally, 'Send as Excel' remains untranslated while adjacent entries ('Send as CSV', 'Send as PNG', 'Send as text') are fully translated, creating inconsistency.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -6290,6 +6297,9 @@ msgstr "例子"
msgid "Examples"
msgstr "示例"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Quality: Fuzzy Flag Removal</b></div>
<div id="fix">
Removing the `#, fuzzy` flag without providing a proper translation causes this entry to be treated as untranslated. The original fuzzy translation '郵件中附上格式化好的CSV' was incorrect (mentions CSV instead of Excel) but at least provided a translation.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pl/LC_MESSAGES/messages.po:
##########
@@ -6595,6 +6602,9 @@ msgstr "Przykład"
msgid "Examples"
msgstr "Przykłady"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect CSV translation replaced with empty</b></div>
<div id="fix">
Empty translation for 'Excel (XLSX) attached in email' will display English text. The old fuzzy translation incorrectly mentioned CSV format.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -234,6 +234,10 @@ msgstr ""
msgid "%(label)s file"
msgstr "%(type)s do Arquivo"
+#, fuzzy, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "Dimensões"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Missing Format Variables</b></div>
<div id="fix">
The translation 'Dimensões' (Dimensions) ignores both `%(name)s` and `%(extension)s` placeholders. When rendered, users will see 'Dimensões' instead of the actual file name and extension. The translation must preserve both format variables.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div><div>
<div id="suggestion">
<div id="issue"><b>Empty translation for format string</b></div>
<div id="fix">
The `msgstr` for `%(name)s.%(extension)s` was changed from 'Dimensões' to empty. 'Dimensões' was an incorrect translation from a fuzzy entry, but replacing it with empty string removes any user-facing text. Provide a proper Portuguese translation or copy the msgid if no translation is available.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -885,6 +889,9 @@ msgstr "生成 CSV 時超時。"
msgid "A timeout occurred while generating a dataframe."
msgstr "生成數據超時"
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Quality: Fuzzy Flag Removal</b></div>
<div id="fix">
Removing the `#, fuzzy` flag without providing a proper translation causes this entry to be treated as untranslated. The original fuzzy translation '生成 CSV 時超時。' was incorrect (mentions CSV instead of xlsx) but at least provided a translation. The new empty state provides no translation at all.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pl/LC_MESSAGES/messages.po:
##########
@@ -960,6 +964,9 @@ msgstr "Wystąpiło przekroczenie czasu podczas generowania pliku CSV."
msgid "A timeout occurred while generating a dataframe."
msgstr "Wystąpiło przekroczenie czasu podczas generowania ramki danych."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect translation replaced with empty</b></div>
<div id="fix">
Empty translation for XLSX timeout message will display English text to Polish users. The old translation incorrectly mentioned CSV; provide proper Polish translation for XLSX format.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -12302,6 +12325,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "發送為CSV"
+msgid "Send as Excel"
+msgstr ""
+
#, fuzzy
msgid "Send as PDF"
msgstr "發送PNG"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Quality: Fuzzy Flag Removal</b></div>
<div id="fix">
Removing the `#, fuzzy` flag without providing a proper translation causes this entry to be treated as untranslated. The original fuzzy translation '發送文本' was incorrect (mentions text instead of PDF) but at least provided a translation.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -188,6 +188,10 @@ msgstr "出於安全原因，%(dialect)s SQLite 資料庫不能用作數據源
msgid "%(label)s file"
msgstr ""
+#, python-format
+msgid "%(name)s.%(extension)s"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Quality: Fuzzy Flag Removal</b></div>
<div id="fix">
Removing the `#, fuzzy` flag without providing a proper translation causes this entry to be treated as untranslated (null in the translation index). The fuzzy flag signals that the translation needs review; removing it without adding a reviewed translation degrades translation quality. Either provide a proper translation or retain the fuzzy marker.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -10952,6 +10968,9 @@ msgstr "在生成 CSV 文件時，報告計劃執行失敗。"
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "在生成螢幕截圖時，報告計劃執行失敗。"
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+
msgid "Report Schedule execution got an unexpected error."
msgstr "報表計劃執行遇到意外錯誤。"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Quality: Fuzzy Flag Removal</b></div>
<div id="fix">
Removing the `#, fuzzy` flag without providing a proper translation causes this entry to be treated as untranslated. The original fuzzy translation '在生成 CSV 文件時，報告計劃執行失敗。' was incorrect (mentions CSV instead of xlsx) but at least provided a translation.
</div>
</div>
<small><i>Code Review Run #561edf</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601577448
##########
superset/utils/core.py:
##########
@@ -120,6 +120,27 @@
logging.getLogger("MARKDOWN").setLevel(logging.INFO)
logger = logging.getLogger(__name__)
+EMAIL_ATTACHMENT_SUBTYPES: dict[str, str] = {
+ ".pdf": "pdf",
+ ".zip": "zip",
+ ".xlsx": "vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+}
+
+
+def build_email_attachment(name: str, body: bytes | str) -> MIMEApplication:
+ """
+ Create an email attachment part with stable filename metadata.
+ """
+ subtype = EMAIL_ATTACHMENT_SUBTYPES.get(os.path.splitext(name)[1].lower())
+ attachment = MIMEApplication(
+ body,
+ _subtype=subtype or "octet-stream",
+ Name=name,
+ )
Review Comment:
Thanks, this is a valid concern. I updated `build_email_attachment` to normalize `str` bodies to UTF-8 bytes before constructing `MIMEApplication`, so text attachments have stable bytes regardless of character set. I also added a regression test for a non-ASCII text attachment payload.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601245432
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2836,6 +2838,64 @@ def test_apply_client_processing_csv_format_default_na_behavior():
) # Second data row should have empty last_name (NA converted to null)
+def _assert_xlsx_client_processing(index: bool) -> None:
+ """Assert XLSX post-processing preserves columns and configured index."""
+ source_df = pd.DataFrame(
Review Comment:
**Suggestion:** Add an explicit type annotation to `source_df` so static type checking can validate DataFrame usage in this helper. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The new helper introduces `source_df` without any type annotation, even though it is a clearly annotatable pandas DataFrame variable. This matches the rule requiring type hints on new or modified Python variables that can be annotated.
</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=24a82b52aa514e3aa43829d29a2ad094&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=24a82b52aa514e3aa43829d29a2ad094&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/charts/test_client_processing.py
**Line:** 2843:2843
**Comment:**
*Custom Rule: Add an explicit type annotation to `source_df` so static type checking can validate DataFrame usage in this helper.
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%2F40885&comment_hash=1176b3d4f9bca2219ba00251be6ba84ad0defb8ecd0996f90e85a9245e3b4f44&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=1176b3d4f9bca2219ba00251be6ba84ad0defb8ecd0996f90e85a9245e3b4f44&reaction=dislike'>👎</a>
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2836,6 +2838,64 @@ def test_apply_client_processing_csv_format_default_na_behavior():
) # Second data row should have empty last_name (NA converted to null)
+def _assert_xlsx_client_processing(index: bool) -> None:
+ """Assert XLSX post-processing preserves columns and configured index."""
+ source_df = pd.DataFrame(
+ {
+ "city": ["Paris", "London"],
+ "value": [10, 20],
+ },
+ index=pd.Index(["row-1", "row-2"], name="row"),
+ )
+ result = {
+ "queries": [
+ {
+ "result_format": ChartDataResultFormat.XLSX,
+ "data": excel.df_to_excel(source_df, index=index),
+ }
+ ]
+ }
+ form_data = {
Review Comment:
**Suggestion:** Add an explicit type annotation to `form_data` to document and enforce the expected shape of this newly added request payload. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The newly introduced `form_data` dictionary is unannotated in the added helper, and it is a relevant local variable that could be typed to improve clarity and static checking.
</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=d6ed68ee6ac24bedb72885aee5b96cf6&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=d6ed68ee6ac24bedb72885aee5b96cf6&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/charts/test_client_processing.py
**Line:** 2858:2858
**Comment:**
*Custom Rule: Add an explicit type annotation to `form_data` to document and enforce the expected shape of this newly added request payload.
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%2F40885&comment_hash=15e222cf42f41ca3581d0f71ea044f51f1f2c56a7dae0905937bd97d849c2752&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=15e222cf42f41ca3581d0f71ea044f51f1f2c56a7dae0905937bd97d849c2752&reaction=dislike'>👎</a>
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2836,6 +2838,64 @@ def test_apply_client_processing_csv_format_default_na_behavior():
) # Second data row should have empty last_name (NA converted to null)
+def _assert_xlsx_client_processing(index: bool) -> None:
+ """Assert XLSX post-processing preserves columns and configured index."""
+ source_df = pd.DataFrame(
+ {
+ "city": ["Paris", "London"],
+ "value": [10, 20],
+ },
+ index=pd.Index(["row-1", "row-2"], name="row"),
+ )
+ result = {
Review Comment:
**Suggestion:** Add a concrete type annotation for `result` (for example, a typed mapping structure) to satisfy the type-hint requirement for newly introduced variables. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The `result` payload is newly added in the helper and has no type hint. Since it is a Python variable with a definable structure, it falls under the type-hint rule.
</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=48588e2307844f7985e6360891f88323&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=48588e2307844f7985e6360891f88323&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/charts/test_client_processing.py
**Line:** 2850:2850
**Comment:**
*Custom Rule: Add a concrete type annotation for `result` (for example, a typed mapping structure) 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%2F40885&comment_hash=ae9bfcaa6d66fb2546afa9b22676cbe3b799dc3c70248768409333a095d745f6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=ae9bfcaa6d66fb2546afa9b22676cbe3b799dc3c70248768409333a095d745f6&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1956,6 +2148,22 @@ def test_get_notification_content_xlsx_format(
assert content.csv is None
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_xlsx_rejects_dashboard(
+ mock_ff, mocker: MockerFixture
+) -> None:
Review Comment:
**Suggestion:** Add a concrete type annotation for the untyped patched argument so the new test function has fully typed parameters. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The new test function adds an untyped parameter `mock_ff` even though it can be annotated like `Mock` or `MagicMock`, so this is a real violation of the Python type-hint requirement.
</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=19bcc22f77cb4581b86471deb3a2fe2b&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=19bcc22f77cb4581b86471deb3a2fe2b&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/commands/report/execute_test.py
**Line:** 2151:2154
**Comment:**
*Custom Rule: Add a concrete type annotation for the untyped patched argument so the new test function has fully typed parameters.
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%2F40885&comment_hash=f470e2dc3638a3a2c1787eb06803d78fb54c0e810e046e4bdc53c15e3bb06c63&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=f470e2dc3638a3a2c1787eb06803d78fb54c0e810e046e4bdc53c15e3bb06c63&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601585917
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
+ get_url.assert_called_once_with(result_format=ChartDataResultFormat.XLSX)
+ get_chart_csv_data.assert_called_once_with(
+ chart_url="/api/v1/chart/1/data/xlsx",
+ auth_cookies=auth_cookies,
+ timeout=app.config["ALERT_REPORTS_CSV_REQUEST_TIMEOUT"],
+ )
+
+
+@pytest.mark.parametrize(
+ ("side_effect", "expected_exception", "expected_message"),
+ [
+ (
+ SoftTimeLimitExceeded(),
+ ReportScheduleXlsxTimeout,
+ "timeout occurred while generating an xlsx",
+ ),
+ (
+ RuntimeError("export failed"),
+ ReportScheduleXlsxFailedError,
+ "Failed generating xlsx export failed",
+ ),
+ ],
+)
+def test_get_xlsx_data_maps_errors(
+ app: SupersetApp,
+ mocker: MockerFixture,
+ side_effect: Exception,
+ expected_exception: type[Exception],
+ expected_message: str,
+) -> None:
+ """XLSX generation errors should use XLSX-specific report exceptions."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ side_effect=side_effect,
+ )
+
+ with pytest.raises(expected_exception, match=expected_message) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is side_effect
+
+
+def test_get_xlsx_data_rejects_empty_result(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """An empty XLSX response should fail report generation."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=None,
+ )
+
+ with pytest.raises(
+ ReportScheduleXlsxFailedError,
+ match="Report Schedule execution failed when generating an xlsx",
+ ) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is None
+
+
+def test_notification_content_contains_xlsx(mocker: MockerFixture) -> None:
+ """XLSX chart reports should populate the XLSX notification field."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.report_format = ReportDataFormat.XLSX
+ report_schedule.force_screenshot = False
+ report_schedule.email_subject = None
+ report_schedule.owners = []
+ report_schedule.recipients = []
+ report_state = BaseReportState(
+ report_schedule,
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ mocker.patch.object(report_state, "_get_url", return_value="/chart/1")
+ mocker.patch.object(report_state, "_get_log_data", return_value={})
+ get_xlsx_data = mocker.patch.object(
+ report_state,
+ "_get_xlsx_data",
+ return_value=b"xlsx-data",
+ )
Review Comment:
Good catch. `_get_xlsx_data` was removed during the merge in favor of `_get_data(result_format)`, so this test was patching a stale method. I updated the XLSX tests to call/patch `_get_data(ChartDataResultFormat.XLSX)` and assert the actual path used by `_get_notification_content`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601583175
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) -> None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
Review Comment:
Good catch. `_get_chart_data` no longer exists after the merge, so I updated the test to call `_get_data(...)`. I also restored the table-like format guard in `_get_data`, so unsupported formats like JSON still raise `ReportScheduleExecuteUnexpectedError` as intended.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3602680973
##########
tests/unit_tests/utils/test_core.py:
##########
@@ -70,6 +71,39 @@
}
+@pytest.mark.parametrize(
+ ("name", "content_type"),
+ [
+ (
+ "report.xlsx",
+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+ ),
+ ("report.zip", "application/zip"),
+ ],
+)
+def test_build_email_attachment(
+ name: str,
+ content_type: str,
+) -> None:
+ """Email attachments should expose the expected MIME type and filename."""
+ attachment = build_email_attachment(name, b"attachment")
+
+ assert attachment.get_content_type() == content_type
+ assert attachment.get_filename() == name
+ assert attachment.get_payload(decode=True) == b"attachment"
+
+
+def test_build_email_attachment_encodes_text_payload() -> None:
+ """Text attachment bodies should be encoded to stable UTF-8 bytes."""
+ attachment = build_email_attachment("report.csv", "город,value\nМосква,1")
+
+ assert attachment.get_content_type() == "application/octet-stream"
+ assert attachment.get_filename() == "report.csv"
+ assert attachment.get_payload(decode=True) == "город,value\nМосква,1".encode(
+ "utf-8"
+ )
Review Comment:
Thanks, agreed. I folded the new text-payload case into the existing `test_build_email_attachment` parametrization, covering bytes ASCII, bytes UTF-8, and str UTF-8 without a separate overlapping test.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3570896929
##########
tests/unit_tests/reports/notifications/email_tests.py:
##########
@@ -144,3 +149,95 @@ def test_email_subject_with_datetime() -> None:
subject = notification._get_subject()
assert datetime_pattern not in subject
assert frozen_now.strftime(datetime_pattern) in subject
+
+
+def _make_notification(xlsx: bytes) -> "EmailNotification":
+ """Build an email notification for attachment tests."""
+ from superset.reports.models import ReportRecipients, ReportRecipientType
+ from superset.reports.notifications.base import NotificationContent
+ from superset.reports.notifications.email import EmailNotification
+
+ recipient = ReportRecipients(type=ReportRecipientType.EMAIL)
Review Comment:
**Suggestion:** Add an explicit type annotation for this local variable to comply with the type-hint requirement for relevant variables. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This new local variable is introduced without a type annotation in a Python file covered by the type-hint rule. It can be annotated, so the suggestion matches the rule 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=bf3f5bec103f46259073de9b3a6a378d&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=bf3f5bec103f46259073de9b3a6a378d&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/reports/notifications/email_tests.py
**Line:** 160:160
**Comment:**
*Custom Rule: Add an explicit type annotation for this local variable to comply with the type-hint requirement for 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%2F40885&comment_hash=51d3042e9df0276c253b86a6a8874db03b59f57e179687bc9907d0bb1fadd0b6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=51d3042e9df0276c253b86a6a8874db03b59f57e179687bc9907d0bb1fadd0b6&reaction=dislike'>👎</a>
##########
tests/unit_tests/reports/notifications/email_tests.py:
##########
@@ -144,3 +149,95 @@ def test_email_subject_with_datetime() -> None:
subject = notification._get_subject()
assert datetime_pattern not in subject
assert frozen_now.strftime(datetime_pattern) in subject
+
+
+def _make_notification(xlsx: bytes) -> "EmailNotification":
+ """Build an email notification for attachment tests."""
+ from superset.reports.models import ReportRecipients, ReportRecipientType
+ from superset.reports.notifications.base import NotificationContent
+ from superset.reports.notifications.email import EmailNotification
+
+ recipient = ReportRecipients(type=ReportRecipientType.EMAIL)
+ content = NotificationContent(
+ name="test report",
+ text=None,
+ xlsx=xlsx,
+ header_data={
+ "notification_format": "XLSX",
+ "notification_type": "Report",
+ "editors": [1],
+ "notification_source": None,
+ "chart_id": None,
+ "dashboard_id": None,
+ "slack_channels": None,
+ "execution_id": "test-execution-id",
+ },
+ )
+ return EmailNotification(recipient=recipient, content=content)
+
+
+def test_get_xlsx_attachment_extension_for_non_zip_content() -> None:
+ """Non-ZIP responses retain the XLSX extension."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+
+ assert _get_xlsx_attachment_extension(b"xlsx-response") == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_xlsx_content() -> None:
+ """An OOXML workbook is identified as XLSX."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils import excel
+
+ xlsx = excel.df_to_excel(pd.DataFrame({"value": [1, 2]}), index=False)
+
+ assert _get_xlsx_attachment_extension(xlsx) == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_invalid_xlsx_zip_content() -> None:
+ """A ZIP with invalid XLSX entries is not identified as a report archive."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils.core import create_zip
+
+ archive = create_zip({"query_1.xlsx": b"xlsx-response"}).getvalue()
Review Comment:
**Suggestion:** Annotate this archive variable with its concrete type to satisfy the mandatory typing rule for new variables. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The variable is newly introduced in Python code and lacks a type annotation even though it can be annotated, so this is a real violation of the type-hint rule.
</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=40d2863f115a42f5b16e55e9f998dce7&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=40d2863f115a42f5b16e55e9f998dce7&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/reports/notifications/email_tests.py
**Line:** 207:207
**Comment:**
*Custom Rule: Annotate this archive variable with its concrete type to satisfy the mandatory typing rule for new 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%2F40885&comment_hash=098113fcac91bf177a62f6f163df430d9a797325ac859db25d6e48a2fb410e32&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=098113fcac91bf177a62f6f163df430d9a797325ac859db25d6e48a2fb410e32&reaction=dislike'>👎</a>
##########
tests/unit_tests/reports/notifications/email_tests.py:
##########
@@ -144,3 +149,95 @@ def test_email_subject_with_datetime() -> None:
subject = notification._get_subject()
assert datetime_pattern not in subject
assert frozen_now.strftime(datetime_pattern) in subject
+
+
+def _make_notification(xlsx: bytes) -> "EmailNotification":
+ """Build an email notification for attachment tests."""
+ from superset.reports.models import ReportRecipients, ReportRecipientType
+ from superset.reports.notifications.base import NotificationContent
+ from superset.reports.notifications.email import EmailNotification
+
+ recipient = ReportRecipients(type=ReportRecipientType.EMAIL)
+ content = NotificationContent(
+ name="test report",
+ text=None,
+ xlsx=xlsx,
+ header_data={
+ "notification_format": "XLSX",
+ "notification_type": "Report",
+ "editors": [1],
+ "notification_source": None,
+ "chart_id": None,
+ "dashboard_id": None,
+ "slack_channels": None,
+ "execution_id": "test-execution-id",
+ },
+ )
+ return EmailNotification(recipient=recipient, content=content)
+
+
+def test_get_xlsx_attachment_extension_for_non_zip_content() -> None:
+ """Non-ZIP responses retain the XLSX extension."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+
+ assert _get_xlsx_attachment_extension(b"xlsx-response") == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_xlsx_content() -> None:
+ """An OOXML workbook is identified as XLSX."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils import excel
+
+ xlsx = excel.df_to_excel(pd.DataFrame({"value": [1, 2]}), index=False)
+
+ assert _get_xlsx_attachment_extension(xlsx) == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_invalid_xlsx_zip_content() -> None:
+ """A ZIP with invalid XLSX entries is not identified as a report archive."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils.core import create_zip
+
+ archive = create_zip({"query_1.xlsx": b"xlsx-response"}).getvalue()
+
+ assert _get_xlsx_attachment_extension(archive) == "xlsx"
+
+
+@pytest.mark.parametrize(
+ ("server_pagination", "expected_extension"),
+ [
+ (False, "xlsx"),
+ (True, "zip"),
+ ],
+)
+def test_xlsx_report_attachment_extension(
+ server_pagination: bool,
+ expected_extension: str,
+) -> None:
+ """Server-paginated XLSX reports should be attached as ZIP archives."""
+ from superset.utils import excel
+ from superset.utils.core import create_zip
+
+ xlsx = excel.df_to_excel(pd.DataFrame({"value": [1, 2]}), index=False)
Review Comment:
**Suggestion:** Add an explicit type annotation to this variable assignment so the new test code remains fully typed. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This added local variable is unannotated and clearly annotatable, so it violates the Python type-hint requirement for new 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=302ad9d342a6436f988ad3d65bee69f9&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=302ad9d342a6436f988ad3d65bee69f9&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/reports/notifications/email_tests.py
**Line:** 227:227
**Comment:**
*Custom Rule: Add an explicit type annotation to this variable assignment so the new test code remains 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%2F40885&comment_hash=e8a73fb6a4a723b31d638abedafbfaafef720c1adabfc89bdbf0ff86ba0872d7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=e8a73fb6a4a723b31d638abedafbfaafef720c1adabfc89bdbf0ff86ba0872d7&reaction=dislike'>👎</a>
##########
tests/unit_tests/reports/notifications/email_tests.py:
##########
@@ -144,3 +149,95 @@ def test_email_subject_with_datetime() -> None:
subject = notification._get_subject()
assert datetime_pattern not in subject
assert frozen_now.strftime(datetime_pattern) in subject
+
+
+def _make_notification(xlsx: bytes) -> "EmailNotification":
+ """Build an email notification for attachment tests."""
+ from superset.reports.models import ReportRecipients, ReportRecipientType
+ from superset.reports.notifications.base import NotificationContent
+ from superset.reports.notifications.email import EmailNotification
+
+ recipient = ReportRecipients(type=ReportRecipientType.EMAIL)
+ content = NotificationContent(
+ name="test report",
+ text=None,
+ xlsx=xlsx,
+ header_data={
+ "notification_format": "XLSX",
+ "notification_type": "Report",
+ "editors": [1],
+ "notification_source": None,
+ "chart_id": None,
+ "dashboard_id": None,
+ "slack_channels": None,
+ "execution_id": "test-execution-id",
+ },
+ )
+ return EmailNotification(recipient=recipient, content=content)
+
+
+def test_get_xlsx_attachment_extension_for_non_zip_content() -> None:
+ """Non-ZIP responses retain the XLSX extension."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+
+ assert _get_xlsx_attachment_extension(b"xlsx-response") == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_xlsx_content() -> None:
+ """An OOXML workbook is identified as XLSX."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils import excel
+
+ xlsx = excel.df_to_excel(pd.DataFrame({"value": [1, 2]}), index=False)
+
+ assert _get_xlsx_attachment_extension(xlsx) == "xlsx"
+
+
+def test_get_xlsx_attachment_extension_for_invalid_xlsx_zip_content() -> None:
+ """A ZIP with invalid XLSX entries is not identified as a report archive."""
+ from superset.reports.notifications.email import (
+ _get_xlsx_attachment_extension,
+ )
+ from superset.utils.core import create_zip
+
+ archive = create_zip({"query_1.xlsx": b"xlsx-response"}).getvalue()
+
+ assert _get_xlsx_attachment_extension(archive) == "xlsx"
+
+
+@pytest.mark.parametrize(
+ ("server_pagination", "expected_extension"),
+ [
+ (False, "xlsx"),
+ (True, "zip"),
+ ],
+)
+def test_xlsx_report_attachment_extension(
+ server_pagination: bool,
+ expected_extension: str,
+) -> None:
+ """Server-paginated XLSX reports should be attached as ZIP archives."""
+ from superset.utils import excel
+ from superset.utils.core import create_zip
+
+ xlsx = excel.df_to_excel(pd.DataFrame({"value": [1, 2]}), index=False)
+ attachment = (
+ create_zip(
+ {
+ "query_1.xlsx": xlsx,
+ "query_2.xlsx": xlsx,
+ }
+ ).getvalue()
+ if server_pagination
+ else xlsx
+ )
+
+ email_content = _make_notification(xlsx=attachment)._get_content()
Review Comment:
**Suggestion:** Add a type annotation for this computed content variable to keep the new code compliant with typing requirements. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This new local variable is not type-annotated and could be annotated, so it fits the custom Python type-hint requirement.
</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=03f11caa5582419689a52a4ff9a6432c&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=03f11caa5582419689a52a4ff9a6432c&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/reports/notifications/email_tests.py
**Line:** 239:239
**Comment:**
*Custom Rule: Add a type annotation for this computed content variable to keep the new code compliant with typing requirements.
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%2F40885&comment_hash=e6783b321470036fc4dab5ade85a74fc190ae41787fb04bf0ad45386bd07ffee&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=e6783b321470036fc4dab5ade85a74fc190ae41787fb04bf0ad45386bd07ffee&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4960055773
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4657198227
<!-- translation-regression-bot -->
## ⚠️ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `sk`, `sl`, `th`, `tr`, `uk`. Please resolve the affected `.po` files before merging.
_Note: intentionally **deleting** a translatable string is not a regression and is not flagged here — only translations invalidated by a renamed/reworded source string are._
| Language | Fuzzy before | Fuzzy after | New |
|----------|-------------:|------------:|----:|
| `ar` | 1136 | 1141 | +5 |
| `ca` | 815 | 820 | +5 |
| `cs` | 787 | 792 | +5 |
| `de` | 997 | 1002 | +5 |
| `fi` | 4823 | 4828 | +5 |
| `fr` | 1014 | 1019 | +5 |
| `it` | 1667 | 1669 | +2 |
| `ja` | 322 | 327 | +5 |
| `ko` | 1521 | 1522 | +1 |
| `lv` | 295 | 300 | +5 |
| `mi` | 807 | 812 | +5 |
| `nl` | 1143 | 1148 | +5 |
| `pt` | 1839 | 1841 | +2 |
| `sk` | 774 | 779 | +5 |
| `sl` | 963 | 968 | +5 |
| `th` | 4823 | 4828 | +5 |
| `tr` | 786 | 788 | +2 |
| `uk` | 293 | 298 | +5 |
### How to fix
**1. Install dependencies** (if not already set up):
```bash
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext
```
**2. Re-extract strings and sync `.po` files:**
```bash
./scripts/translations/babel_update.sh
```
This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
**3. Resolve the fuzzy entries** in the affected language files (`ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `sk`, `sl`, `th`, `tr`, `uk`):
```bash
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
```
For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
**4. Commit your changes to the `.po` files.**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3378999419
##########
superset/commands/report/execute.py:
##########
@@ -552,6 +567,19 @@ def _get_csv_data(self) -> bytes:
raise ReportScheduleCsvFailedError()
return csv_data
+ def _get_xlsx_data(self) -> bytes:
+ try:
+ xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
+ except SoftTimeLimitExceeded as ex:
+ raise ReportScheduleXlsxTimeout() from ex
+ except Exception as ex:
+ raise ReportScheduleXlsxFailedError(
+ f"Failed generating xlsx {str(ex)}"
+ ) from ex
+ if not xlsx_data:
+ raise ReportScheduleXlsxFailedError()
+ return xlsx_data
Review Comment:
Fixed
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3379000821
##########
superset/commands/report/execute.py:
##########
@@ -552,6 +567,19 @@ def _get_csv_data(self) -> bytes:
raise ReportScheduleCsvFailedError()
return csv_data
+ def _get_xlsx_data(self) -> bytes:
+ try:
+ xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
+ except SoftTimeLimitExceeded as ex:
+ raise ReportScheduleXlsxTimeout() from ex
+ except Exception as ex:
+ raise ReportScheduleXlsxFailedError(
+ f"Failed generating xlsx {str(ex)}"
+ ) from ex
+ if not xlsx_data:
+ raise ReportScheduleXlsxFailedError()
+ return xlsx_data
Review Comment:
<!-- Bito Reply -->
The suggestion is appropriate and improves the code by adding diagnostic logging for timeouts, which aligns with the existing logging pattern in the codebase. Applying this change will help in troubleshooting production issues by providing visibility into the duration of XLSX generation timeouts.
**superset/commands/report/execute.py**
```
start_time = datetime.utcnow()
xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
except SoftTimeLimitExceeded as ex:
elapsed_seconds = (datetime.utcnow() - start_time).total_seconds()
logger.warning(
"XLSX generation timeout after %.2fs - execution_id: %s",
elapsed_seconds,
self._execution_id,
)
raise ReportScheduleXlsxTimeout() from ex
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3379999833
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -12302,6 +12328,10 @@ msgstr ""
msgid "Send as CSV"
msgstr "發送為CSV"
+#, fuzzy
+msgid "Send as Excel"
+msgstr "發送文本"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation: Copy-paste error</b></div>
<div id="fix">
Copy-paste error: the translation says '發送文本' (send as text) but the msgid is 'Send as Excel'. Users will see incorrect message when selecting Excel export.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh_TW/LC_MESSAGES/messages.po:
##########
@@ -10952,6 +10970,10 @@ msgstr "在生成 CSV 文件時，報告計劃執行失敗。"
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "在生成螢幕截圖時，報告計劃執行失敗。"
+#, fuzzy
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr "在生成 CSV 文件時，報告計劃執行失敗。"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation: Copy-paste error</b></div>
<div id="fix">
Copy-paste error: the translation references CSV but the msgid is 'Report Schedule execution failed when generating an xlsx.' Users will see incorrect message about CSV when xlsx generation fails.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/es/LC_MESSAGES/messages.po:
##########
@@ -12692,6 +12720,10 @@ msgstr ""
msgid "Send as CSV"
msgstr "Enviar como CSV"
+#, fuzzy
+msgid "Send as Excel"
+msgstr "Enviar como texto"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation mismatch</b></div>
<div id="fix">
Mistranslation: 'Send as Excel' UI action incorrectly displays as 'Send as text' in Spanish. Users may not find Excel export option.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/es/LC_MESSAGES/messages.po:
##########
@@ -6584,6 +6592,10 @@ msgstr "Ejemplo"
msgid "Examples"
msgstr "Ejemplos"
+#, fuzzy
+msgid "Excel (XLSX) attached in email"
+msgstr "CSV formateado adjunto en el correo electrónico"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation mismatch</b></div>
<div id="fix">
Mistranslation: 'Excel (XLSX)' message incorrectly says 'CSV' in Spanish. Email attachment description will be misleading.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/es/LC_MESSAGES/messages.po:
##########
@@ -969,6 +973,10 @@ msgstr "Se ha agotado el tiempo de espera al generar un CSV."
msgid "A timeout occurred while generating a dataframe."
msgstr "Se ha agotado el tiempo de espera al generar un marco de datos."
+#, fuzzy
+msgid "A timeout occurred while generating an xlsx."
+msgstr "Se ha agotado el tiempo de espera al generar un CSV."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation mismatch</b></div>
<div id="fix">
Mistranslation: msgid references 'xlsx' but msgstr incorrectly says 'CSV'. Users will see wrong file type in error message.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -6286,6 +6294,10 @@ msgstr "例子"
msgid "Examples"
msgstr "示例"
+#, fuzzy
+msgid "Excel (XLSX) attached in email"
+msgstr "邮件中附上格式化好的CSV"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CSV/XLSX format mismatch in translation</b></div>
<div id="fix">
Translation mismatch: msgid mentions 'Excel (XLSX)' but msgstr says 'CSV' (逗号分割文件). Should use 'XLSX文件' (XLSX file) instead to match the original message.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/es/LC_MESSAGES/messages.po:
##########
@@ -11322,6 +11340,12 @@ msgstr ""
"Se ha producido un error en la ejecución de la programación de informes "
"al generar una captura de pantalla."
+#, fuzzy
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+"Se ha producido un error en la ejecución de la programación de informes "
+"al generar un CSV."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation mismatch</b></div>
<div id="fix">
Mistranslation: Report schedule failure message incorrectly references 'CSV' instead of 'xlsx'. Users see wrong format in error.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -6726,6 +6738,12 @@ msgstr ""
msgid "File settings"
msgstr "过滤器设置"
+#, python-format
+msgid ""
+"File size %(size)s bytes exceeds the maximum allowed upload size of "
+"%(max_size)s bytes"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation string for error message</b></div>
<div id="fix">
Empty translation string: msgid 'File size %(size)s bytes exceeds the maximum allowed upload size of %(max_size)s bytes' has empty msgstr. Users will see the raw English error message. Add appropriate Chinese translation.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -10939,6 +10957,10 @@ msgstr "在生成 CSV 文件时，报告计划执行失败。"
msgid "Report Schedule execution failed when generating a screenshot."
msgstr "在生成屏幕截图时，报告计划执行失败。"
+#, fuzzy
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr "在生成 CSV 文件时，报告计划执行失败。"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect file format in error message</b></div>
<div id="fix">
Translation mismatch: msgid specifies 'xlsx' but msgstr incorrectly says 'CSV 文件'. Compare with line 10947-10948 where 'csv' correctly translates to 'CSV 文件'. This entry should use 'XLSX 文件'.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -886,6 +890,10 @@ msgstr "生成CSV时超时。"
msgid "A timeout occurred while generating a dataframe."
msgstr "生成数据超时"
+#, fuzzy
+msgid "A timeout occurred while generating an xlsx."
+msgstr "生成CSV时超时。"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect file format translation</b></div>
<div id="fix">
Translation mismatch: msgid references 'xlsx' but msgstr incorrectly uses 'CSV'. Following the existing pattern 'A timeout occurred while generating a csv.' → '生成CSV时超时。', this entry should read '生成XLSX时超时。' for consistency and correctness.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -11033,6 +11055,10 @@ msgstr "必需的控件值已被移除"
msgid "Resample"
msgstr "重新采样"
+#, python-format
+msgid "Resample method '%(method)s' is not supported."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty translation string for validation error</b></div>
<div id="fix">
Empty translation string: msgid 'Resample method \'%(method)s\' is not supported.' has empty msgstr. Users will see raw English error message. Add appropriate Chinese translation following the pattern of similar error messages in the file.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -234,6 +234,10 @@ msgstr ""
msgid "%(label)s file"
msgstr "%(type)s do Arquivo"
+#, fuzzy, python-format
+msgid "%(name)s.%(extension)s"
+msgstr "Dimensões"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Missing Format Variables</b></div>
<div id="fix">
The translation 'Dimensões' (Dimensions) ignores both `%(name)s` and `%(extension)s` placeholders. When rendered, users will see 'Dimensões' instead of the actual file name and extension. The translation must preserve both format variables.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -967,6 +971,10 @@ msgstr "Ocorreu um tempo limite ao gerar um arquivo csv."
msgid "A timeout occurred while generating a dataframe."
msgstr "Ocorreu um timeout durante a geração de um dataframe."
+#, fuzzy
+msgid "A timeout occurred while generating an xlsx."
+msgstr "Ocorreu um tempo limite ao gerar um arquivo csv."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Type Mismatch</b></div>
<div id="fix">
The translation says 'arquivo csv' but the source message is about 'xlsx' files. This mismatch will confuse users seeing an xlsx-related error message that mentions csv.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/zh/LC_MESSAGES/messages.po:
##########
@@ -12289,6 +12315,10 @@ msgstr ""
msgid "Send as CSV"
msgstr "发送为CSV"
+#, fuzzy
+msgid "Send as Excel"
+msgstr "发送文本"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect action verb in translation</b></div>
<div id="fix">
Translation mismatch: msgid 'Send as Excel' is translated as '发送文本' (Send text). Compare with 'Send as CSV' at line 12315 which correctly translates to '发送为CSV'. This entry should use '发送为Excel' for consistency.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -11217,6 +11235,10 @@ msgstr ""
"A execução do agendamento do relatório falhou ao gerar uma captura de "
"tela."
+#, fuzzy
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr "A execução do Report Schedule falhou ao gerar um arquivo csv."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Type Mismatch</b></div>
<div id="fix">
The translation says 'arquivo csv' but the source is about 'xlsx' files. Users will see incorrect format type in report schedule error messages.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -6984,6 +6996,12 @@ msgstr ""
msgid "File settings"
msgstr "Configurações de filtro"
+#, python-format
+msgid ""
+"File size %(size)s bytes exceeds the maximum allowed upload size of "
+"%(max_size)s bytes"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty Translation String</b></div>
<div id="fix">
The translation is empty (msgstr ""), so users will see the English error message instead of Portuguese. This is a user-facing error message that should be translated.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -11309,6 +11331,10 @@ msgstr "Os valores de controle necessários foram eliminados"
msgid "Resample"
msgstr "Reamostragem"
+#, python-format
+msgid "Resample method '%(method)s' is not supported."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty Translation String</b></div>
<div id="fix">
The translation is empty (msgstr ""), so users will see the English error message instead of Portuguese. This is a user-facing error message that should be translated.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -12573,6 +12599,10 @@ msgstr ""
msgid "Send as CSV"
msgstr "Enviar como CSV"
+#, fuzzy
+msgid "Send as Excel"
+msgstr "Enviar como texto"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Semantic Mismatch</b></div>
<div id="fix">
The translation says 'Enviar como texto' (Send as text) but the source is 'Send as Excel'. Users will see incorrect format type in export options.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -6540,6 +6548,10 @@ msgstr "Exemplo"
msgid "Examples"
msgstr "Exemplos"
+#, fuzzy
+msgid "Excel (XLSX) attached in email"
+msgstr "CSV formatado anexado no e-mail"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Translation Type Mismatch</b></div>
<div id="fix">
The translation says 'CSV formatado anexado no e-mail' but the source is 'Excel (XLSX) attached in email'. Users will see incorrect format type in email notification settings.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -15795,6 +15825,18 @@ msgstr "Para obter um URL legível para seu painel"
msgid "Token Request URI"
msgstr ""
+#, python-format
+msgid ""
+"Too many sub-slices requested. The maximum allowed is %(max)s, but "
+"%(count)s were requested."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty Translation String</b></div>
<div id="fix">
The translation is empty (msgstr ""), so users will see the English error message instead of Portuguese. This is a user-facing error message that should be translated.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/de/LC_MESSAGES/messages.po:
##########
@@ -956,6 +960,9 @@ msgstr "Beim Generieren einer CSV-Datei ist ein Timeout aufgetreten."
msgid "A timeout occurred while generating a dataframe."
msgstr "Beim Generieren eines Datenextrakts ist ein Timeout aufgetreten."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect semantic translation</b></div>
<div id="fix">
The old fuzzy entry for 'xlsx' contained the wrong translation text (CSV message). The removed line 965 'Beim Generieren einer CSV-Datei ist ein Timeout aufgetreten.' is semantically wrong for xlsx and creates incorrect German user output when xlsx timeout occurs.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/pt_BR/LC_MESSAGES/messages.po:
##########
@@ -15795,6 +15825,18 @@ msgstr "Para obter um URL legível para seu painel"
msgid "Token Request URI"
msgstr ""
+#, python-format
+msgid ""
+"Too many sub-slices requested. The maximum allowed is %(max)s, but "
+"%(count)s were requested."
+msgstr ""
+
+#, python-format
+msgid ""
+"Too many time comparisons requested. The maximum allowed is %(max)s, but "
+"%(count)s were requested."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty Translation String</b></div>
<div id="fix">
The translation is empty (msgstr ""), so users will see the English error message instead of Portuguese. This is a user-facing error message that should be translated.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/de/LC_MESSAGES/messages.po:
##########
@@ -6460,6 +6467,9 @@ msgstr "Beispiel"
msgid "Examples"
msgstr "Beispiele"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
+
#, fuzzy
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing translation after defuzzy</b></div>
<div id="fix">
Removing the fuzzy flag for 'Excel (XLSX) attached in email' without providing a German translation creates empty output. The removed 'Formatierte CSV-Datei in E-Mail angehängt' was incorrectly applied to xlsx.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/sk/LC_MESSAGES/messages.po:
##########
@@ -958,6 +962,9 @@ msgstr "Pri generovanie CSV došlo k vypršenie časového limitu."
msgid "A timeout occurred while generating a dataframe."
msgstr "Pri generovanie dátového rámce došlo k vypršenie časového limitu."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing XLSX translation</b></div>
<div id="fix">
Empty `msgstr` for XLSX timeout message will display as blank text to Slovak users. Add proper translation following the pattern of adjacent entries (lines 962-963, 968-969).
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fr/LC_MESSAGES/messages.po:
##########
@@ -6387,6 +6394,9 @@ msgstr "Exemple"
msgid "Examples"
msgstr "Exemples"
+msgid "Excel (XLSX) attached in email"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation fix</b></div>
<div id="fix">
The `fuzzy` flag was removed but `msgstr` was left empty. Users will see English 'Excel (XLSX) attached in email' instead of French. This is a UI label used in ReportModal.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/translations/fr/LC_MESSAGES/messages.po
+++ b/superset/translations/fr/LC_MESSAGES/messages.po
@@ -6394,7 +6394,8 @@ msgstr "Exemples"
msgid "Excel (XLSX) attached in email"
-msgstr ""
+msgstr "Fichier Excel (XLSX) joint dans le courriel"
msgid "Excel Export"
```
</div>
</details>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/commands/report/execute.py:
##########
@@ -552,6 +567,19 @@ def _get_csv_data(self) -> bytes:
raise ReportScheduleCsvFailedError()
return csv_data
+ def _get_xlsx_data(self) -> bytes:
+ try:
+ xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
+ except SoftTimeLimitExceeded as ex:
+ raise ReportScheduleXlsxTimeout() from ex
+ except Exception as ex:
+ raise ReportScheduleXlsxFailedError(
+ f"Failed generating xlsx {str(ex)}"
+ ) from ex
+ if not xlsx_data:
+ raise ReportScheduleXlsxFailedError()
+ return xlsx_data
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing timeout diagnostic logging</b></div>
<div id="fix">
The `_get_xlsx_data` handler for `SoftTimeLimitExceeded` (line 573) lacks the elapsed-time logging that `_get_csv_data` provides. Without this, timeouts are harder to diagnose in production since operators won't know how long the operation ran.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -567,8 +567,13 @@ class BaseReportState:
def _get_xlsx_data(self) -> bytes:
try:
+ start_time = datetime.utcnow()
xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX)
except SoftTimeLimitExceeded as ex:
+ elapsed_seconds = (datetime.utcnow() - start_time).total_seconds()
+ logger.warning(
+ "XLSX generation timeout after %.2fs - execution_id: %s",
+ elapsed_seconds,
+ self._execution_id,
+ )
raise ReportScheduleXlsxTimeout() from ex
except Exception as ex:
raise ReportScheduleXlsxFailedError(
```
</div>
</details>
</div>
<small><i>Code Review Run #077cf0</i></small>
</div><div>
<div id="suggestion">
<div id="issue"><b>Semantic duplication in diff</b></div>
<div id="fix">
The new timing instrumentation in `_get_xlsx_data` (lines 575-580, 583-588) duplicates the exact pattern already present in `_get_csv_data` (lines 549-562). This creates maintenance risk where logging format changes require updates in two places.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/de/LC_MESSAGES/messages.po:
##########
@@ -11127,6 +11143,9 @@ msgstr ""
"Bericht-Ausführungsplan Die Ausführung des Zeitplans ist beim Generieren "
"eines Screenshots fehlgeschlagen."
+msgid "Report Schedule execution failed when generating an xlsx."
+msgstr ""
+
msgid "Report Schedule execution got an unexpected error."
msgstr "Unerwarteter Fehler bei der Ausführung des geplanten Berichts."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing xlsx report failure translation</b></div>
<div id="fix">
Empty translation for xlsx report failure leaves users without error context. The CSV translation was incorrectly applied to xlsx in the old fuzzy entry.
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fr/LC_MESSAGES/messages.po:
##########
@@ -950,6 +954,9 @@ msgstr "Dépassement de délai lors de la génération d'un CSV."
msgid "A timeout occurred while generating a dataframe."
msgstr "Dépassement de délai lors de la génération d'une trame de données."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation fix</b></div>
<div id="fix">
The `fuzzy` flag was removed but `msgstr` was left empty. Users will see English 'A timeout occurred while generating an xlsx.' instead of French. Compare with the correctly translated CSV variant at line 951-952 for the proper pattern.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/translations/fr/LC_MESSAGES/messages.po
+++ b/superset/translations/fr/LC_MESSAGES/messages.po
@@ -954,7 +954,7 @@ msgstr "Dépassement de délai lors de la génération d'un CSV."
msgid "A timeout occurred while generating a dataframe."
msgstr "Dépassement de délai lors de la génération d'une trame de données."
-msgid "A timeout occurred while generating an xlsx."
+msgid "A timeout occurred while generating an xlsx."
msgstr ""
+msgstr "Dépassement de délai lors de la génération d'un fichier Excel."
```
</div>
</details>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/fr/LC_MESSAGES/messages.po:
##########
@@ -12506,6 +12529,9 @@ msgstr ""
msgid "Send as CSV"
msgstr "Envoyer comme CSV"
+msgid "Send as Excel"
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete translation fix</b></div>
<div id="fix">
The `fuzzy` flag was removed but `msgstr` was left empty. Users will see English 'Send as Excel' instead of French. This is a UI label used in AlertReportModal.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/translations/fr/LC_MESSAGES/messages.po
+++ b/superset/translations/fr/LC_MESSAGES/messages.po
@@ -12529,7 +12529,8 @@ msgstr ""
msgid "Send as Excel"
-msgstr ""
+msgstr "Envoyer comme Excel"
msgid "Send as PDF"
```
</div>
</details>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/translations/ko/LC_MESSAGES/messages.po:
##########
@@ -870,6 +874,9 @@ msgstr "데이터 베이스 목록을 가져오는 도중 에러가 발생하였
msgid "A timeout occurred while generating a dataframe."
msgstr "데이터 베이스 목록을 가져오는 도중 에러가 발생하였습니다."
+msgid "A timeout occurred while generating an xlsx."
+msgstr ""
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty Translation String Regression</b></div>
<div id="fix">
Please restore the Korean translation for 'A timeout occurred while generating an xlsx.' by replacing the empty msgstr with "데이터 베이스 목록을 가져오는 도중 에러가 발생하였습니다.".
</div>
</div>
<small><i>Code Review Run #64f414</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40885:
URL: https://github.com/apache/superset/pull/40885
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3602682284
##########
tests/unit_tests/utils/test_core.py:
##########
@@ -70,6 +71,39 @@
}
+@pytest.mark.parametrize(
+ ("name", "content_type"),
+ [
+ (
+ "report.xlsx",
+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+ ),
+ ("report.zip", "application/zip"),
+ ],
+)
+def test_build_email_attachment(
+ name: str,
+ content_type: str,
+) -> None:
+ """Email attachments should expose the expected MIME type and filename."""
+ attachment = build_email_attachment(name, b"attachment")
+
+ assert attachment.get_content_type() == content_type
+ assert attachment.get_filename() == name
+ assert attachment.get_payload(decode=True) == b"attachment"
+
+
+def test_build_email_attachment_encodes_text_payload() -> None:
+ """Text attachment bodies should be encoded to stable UTF-8 bytes."""
+ attachment = build_email_attachment("report.csv", "город,value\nМосква,1")
+
+ assert attachment.get_content_type() == "application/octet-stream"
+ assert attachment.get_filename() == "report.csv"
+ assert attachment.get_payload(decode=True) == "город,value\nМосква,1".encode(
+ "utf-8"
+ )
Review Comment:
<!-- Bito Reply -->
The approach of folding the text-payload case into the existing parametrization is appropriate. It effectively consolidates the test logic while maintaining coverage for bytes (ASCII and UTF-8) and string (UTF-8) inputs, which aligns with the reviewer's suggestion to avoid test duplication.
**tests/unit_tests/utils/test_core.py**
```
@pytest.mark.parametrize(
("name", "content_type"),
[
(
"report.xlsx",
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
),
("report.zip", "application/zip"),
],
)
def test_build_email_attachment(
name: str,
content_type: str,
) -> None:
"""Email attachments should expose the expected MIME type and filename."""
attachment = build_email_attachment(name, b"attachment")
assert attachment.get_content_type() == content_type
assert attachment.get_filename() == name
assert attachment.get_payload(decode=True) == b"attachment"
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-5004242867
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: added email delivery of XLSX reports [superset]
Posted by "innovark37 (via GitHub)" <gi...@apache.org>.
innovark37 commented on PR #40885:
URL: https://github.com/apache/superset/pull/40885#issuecomment-4958336033
@rusackas
Thanks for the review. I addressed both points:
- merged master into the branch and removed the broad per-language `.po` churn
- kept only the intended RU translations for the new XLSX strings
- narrowed the `.zip` handling so it only applies to server-paginated XLSX exports containing valid workbook entries
That should avoid masking an error payload as a ZIP attachment.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org