feat: added email delivery of XLSX reports#40885
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
# Conflicts: # tests/unit_tests/charts/test_client_processing.py
There was a problem hiding this comment.
Code Review Agent Run #077cf0
Actionable Suggestions - 1
-
superset/commands/report/execute.py - 1
- Missing timeout diagnostic logging · Line 570-581
Additional Suggestions - 3
-
tests/unit_tests/utils/test_core.py - 1
-
Missing test coverage for PDF attachments · Line 72-91The test covers `.xlsx` and `.zip` but misses `.pdf` extension, which is defined in `EMAIL_ATTACHMENT_SUBTYPES` (line 126 in `superset/utils/core.py`). This gap means potential regressions in PDF email attachments would go undetected. Additionally, consider testing the fallback behavior for unsupported extensions like `.txt` to verify it correctly defaults to `application/octet-stream`.
-
-
superset/commands/report/execute.py - 2
-
Inconsistent timeout logging · Line 573-574The XLSX timeout handler at line 573-574 does not log elapsed_seconds before raising ReportScheduleXlsxTimeout, unlike _get_csv_data (line 549), _get_screenshots (line 481), and _get_embedded_data (line 612) which all log timing before their respective timeout exceptions. This inconsistency reduces debuggability for XLSX timeout incidents.
Code suggestion
--- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -570,6 +570,11 @@ class BaseReportState: def _get_xlsx_data(self) -> bytes: start_time = datetime.utcnow() try: 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(
-
Inconsistent error logging · Line 575-578The XLSX error handler at line 575-578 raises ReportScheduleXlsxFailedError without calling logger.exception, unlike _get_csv_data (line 558) and _get_screenshots (line 489) which both log the full exception trace before raising their respective errors. This inconsistency may cause XLSX error details to be lost in logs.
Code suggestion
--- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -572,6 +572,11 @@ class BaseReportState: xlsx_data = self._get_chart_data(ChartDataResultFormat.XLSX) except SoftTimeLimitExceeded as ex: raise ReportScheduleXlsxTimeout() from ex except Exception as ex: + elapsed_seconds = (datetime.utcnow() - start_time).total_seconds() + logger.exception( + "XLSX generation failed after %.2fs - execution_id: %s", + elapsed_seconds, + self._execution_id, + ) raise ReportScheduleXlsxFailedError( f"Failed generating xlsx {str(ex)}" ) from ex
-
Review Details
-
Files reviewed - 19 · Commit Range:
a3acdce..a3acdce- docs/admin_docs/configuration/alerts-reports.mdx
- docs/user_docs_versioned_docs/version-6.0.0/configuration/alerts-reports.mdx
- superset-frontend/src/features/alerts/AlertReportModal.test.tsx
- superset-frontend/src/features/alerts/AlertReportModal.tsx
- superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx
- superset-frontend/src/features/reports/ReportModal/index.tsx
- superset-frontend/src/features/reports/types.ts
- superset/charts/client_processing.py
- superset/commands/report/exceptions.py
- superset/commands/report/execute.py
- superset/reports/models.py
- superset/reports/notifications/base.py
- superset/reports/notifications/email.py
- superset/translations/ru/LC_MESSAGES/messages.po
- superset/utils/core.py
- tests/unit_tests/charts/test_client_processing.py
- tests/unit_tests/commands/report/execute_test.py
- tests/unit_tests/reports/notifications/email_tests.py
- tests/unit_tests/utils/test_core.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
|
| 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):
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext2. Re-extract strings and sync .po files:
./scripts/translations/babel_update.shThis 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):
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.poFor 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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40885 +/- ##
==========================================
- Coverage 64.13% 64.12% -0.02%
==========================================
Files 2653 2653
Lines 143569 143643 +74
Branches 33126 33135 +9
==========================================
+ Hits 92084 92110 +26
- Misses 49873 49915 +42
- Partials 1612 1618 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review Agent Run #64f414
Actionable Suggestions - 60
-
superset/commands/report/execute.py - 1
- Semantic duplication in diff · Line 570-594
-
superset/translations/th/LC_MESSAGES/messages.po - 4
- Empty translation after fuzzy removal · Line 1467-1467
- Empty translation after fuzzy removal · Line 10943-10943
- Empty translation after fuzzy removal · Line 19181-19181
- Empty translation after fuzzy removal · Line 21523-21523
-
superset/translations/lv/LC_MESSAGES/messages.po - 3
- Empty translation for user message · Line 972-974
- Empty translation for UI label · Line 6229-6231
- Empty translation for menu item · Line 11974-11975
-
superset/translations/ja/LC_MESSAGES/messages.po - 4
- Missing Japanese translation · Line 889-891
- Missing Japanese translation · Line 5847-5849
- Missing Japanese translation · Line 10115-10118
- Missing Japanese translation · Line 11353-11358
-
superset/translations/fa/LC_MESSAGES/messages.po - 5
- Incorrect translation placeholder · Line 210-211
- Wrong file format in translation · Line 940-941
- Wrong file format in translation · Line 6324-6325
- Empty msgstr for user-facing error messages · Line 11003-11005
- Fuzzy translation with imprecise msgstr · Line 12232-12234
-
superset/translations/cs/LC_MESSAGES/messages.po - 4
- Incomplete translation introduced · Line 972-974
- Incomplete translation introduced · Line 6356-6359
- Incomplete translation introduced · Line 10918-10922
- Incomplete translation introduced · Line 12241-12246
-
superset/translations/tr/LC_MESSAGES/messages.po - 1
- Translation regression: valid Turkish lost · Line 183-185
-
superset/translations/it/LC_MESSAGES/messages.po - 2
- Placeholder translation for file name · Line 187-187
- Untranslated xlsx timeout error · Line 883-883
-
superset/translations/nl/LC_MESSAGES/messages.po - 4
- Missing Dutch translation · Line 926-928
- Missing Dutch translation · Line 6426-6429
- Missing Dutch translation · Line 11061-11065
- Missing Dutch translation · Line 12408-12413
-
superset/translations/zh_TW/LC_MESSAGES/messages.po - 5
- Translation: Incorrect msgstr · Line 193-193
- Translation: Copy-paste error · Line 894-894
- Translation: Copy-paste error · Line 6303-6303
- Translation: Copy-paste error · Line 10975-10975
- Translation: Copy-paste error · Line 12333-12333
-
superset/translations/es/LC_MESSAGES/messages.po - 4
- Translation mismatch · Line 977-978
- Translation mismatch · Line 6595-6597
- Translation mismatch · Line 11343-11347
- Translation mismatch · Line 12723-12725
-
superset/translations/zh/LC_MESSAGES/messages.po - 6
- Incorrect file format translation · Line 893-895
- CSV/XLSX format mismatch in translation · Line 6297-6299
- Empty translation string for error message · Line 6741-6745
- Incorrect file format in error message · Line 10960-10962
- Empty translation string for validation error · Line 11058-11060
- Incorrect action verb in translation · Line 12318-12320
-
superset/translations/pt_BR/LC_MESSAGES/messages.po - 9
- Translation Missing Format Variables · Line 237-239
- Translation Type Mismatch · Line 974-976
- Translation Type Mismatch · Line 6551-6553
- Empty Translation String · Line 6999-7003
- Translation Type Mismatch · Line 11238-11240
- Empty Translation String · Line 11334-11336
- Translation Semantic Mismatch · Line 12602-12604
- Empty Translation String · Line 15828-15832
- Empty Translation String · Line 15834-15838
-
superset/translations/de/LC_MESSAGES/messages.po - 3
- Incorrect semantic translation · Line 963-965
- Missing translation after defuzzy · Line 6470-6473
- Missing xlsx report failure translation · Line 11146-11150
-
superset/translations/sk/LC_MESSAGES/messages.po - 1
- Missing XLSX translation · Line 966-966
-
superset/translations/fr/LC_MESSAGES/messages.po - 3
- Incomplete translation fix · Line 957-958
- Incomplete translation fix · Line 6397-6398
- Incomplete translation fix · Line 12532-12533
-
superset/translations/ko/LC_MESSAGES/messages.po - 1
- Empty Translation String Regression · Line 878-878
Additional Suggestions - 12
-
superset/translations/zh/LC_MESSAGES/messages.po - 1
-
Template string translated incorrectly · Line 192-194Template translation appears incorrect: '%(name)s.%(extension)s' is a filename template (e.g., 'report.xlsx') but msgstr '维度' means 'dimension', which is semantically unrelated. The translation may cause confusing output or be unused.
-
-
superset/translations/sk/LC_MESSAGES/messages.po - 1
-
Lost meaningful translation · Line 229-230Changed from meaningful Slovak word 'Rozšírenie' (Extension) to untranslated placeholder `%(name)s.%(extension)s`. The original translation was semantically appropriate for a file extension placeholder.
-
-
superset/translations/pl/LC_MESSAGES/messages.po - 4
-
Incorrect CSV reference in XLSX translation · Line 969-969The translation at line 969 incorrectly references 'CSV' instead of 'XLSX'. The English msgid is 'A timeout occurred while generating an xlsx.' but the Polish msgstr uses 'pliku CSV'. Compare with the existing correct translation at line 962 for 'A timeout occurred while generating a csv.' which uses 'pliku CSV'.
Code suggestion
--- superset/translations/pl/LC_MESSAGES/messages.po +++ superset/translations/pl/LC_MESSAGES/messages.po @@ -966,7 +966,7 @@ #, fuzzy msgid "A timeout occurred while generating an xlsx." -msgstr "Wystąpiło przekroczenie czasu podczas generowania pliku CSV." +msgstr "Wystąpiło przekroczenie czasu podczas generowania pliku XLSX." msgid "A timeout occurred while taking a screenshot." msgstr "Wystąpiło przekroczenie czasu podczas wykonywania zrzutu ekranu."
-
Incorrect CSV reference in Excel translation · Line 6608-6608The translation at line 6608 incorrectly describes the feature as 'Sformatowany CSV' (Formatted CSV) when the English says 'Excel (XLSX) attached in email'. Polish users will see misleading CSV reference instead of Excel.
Code suggestion
--- superset/translations/pl/LC_MESSAGES/messages.po +++ superset/translations/pl/LC_MESSAGES/messages.po @@ -6605,7 +6605,7 @@ #, fuzzy msgid "Excel (XLSX) attached in email" -msgstr "Sformatowany CSV załączony w e-mailu" +msgstr "Arkusz Excel załączony w e-mailu" #, fuzzy msgid "Excel Export"
-
Incorrect CSV reference in XLSX translation · Line 11390-11392The translation at lines 11390-11392 incorrectly references 'pliku CSV' instead of 'pliku XLSX'. The English msgid at line 11389 is 'Report Schedule execution failed when generating an xlsx.' Polish users will see wrong file format in error message.
Code suggestion
--- superset/translations/pl/LC_MESSAGES/messages.po +++ superset/translations/pl/LC_MESSAGES/messages.po @@ -11387,7 +11387,7 @@ #, fuzzy msgid "Report Schedule execution failed when generating an xlsx." -msgstr "" -"Wykonanie harmonogramu raportu nie powiodło się podczas generowania pliku" -" CSV." +msgstr "" +"Wykonanie harmonogramu raportu nie powiodło się podczas generowania pliku" +" XLSX."
-
Incorrect translation for 'Send as Excel' · Line 12806-12806The translation at line 12806 uses 'Wyślij jako tekst' (Send as text) for 'Send as Excel'. This is incorrect and misleading. Compare with adjacent correct translations: 'Send as CSV' → 'Wyślij jako CSV' (line 12802), 'Send as PDF' → 'Wyślij jako PDF' (line 12810).
Code suggestion
--- superset/translations/pl/LC_MESSAGES/messages.po +++ superset/translations/pl/LC_MESSAGES/messages.po @@ -12803,7 +12803,7 @@ #, fuzzy msgid "Send as Excel" -msgstr "Wyślij jako tekst" +msgstr "Wyślij jako Excel" #, fuzzy msgid "Send as PDF"
-
-
superset/translations/pt/LC_MESSAGES/messages.po - 1
-
Missing Portuguese translation · Line 900-901The xlsx timeout message now has an empty `msgstr`, causing English text to display to Portuguese users instead of a localized message. All other timeout messages in this file have Portuguese translations. This creates an inconsistent user experience for pt-BR users.
-
-
superset/translations/es/LC_MESSAGES/messages.po - 4
-
Fuzzy entry with placeholder mismatch · Line 238-240This entry is marked fuzzy (won't be used) and msgstr 'Dimensiones' doesn't use placeholders. File naming pattern won't display correctly to Spanish users.
-
Missing translation · Line 7051-7055Empty msgstr leaves file size error untranslated. Spanish users will see English error message about file upload limits.
-
Missing translation · Line 11441-11444Empty msgstr for resample method validation error. Spanish users will see English validation message.
-
Missing translations · Line 16099-16109Two error messages have empty msgstr. Spanish users will see English errors about query complexity limits.
-
-
superset/translations/lv/LC_MESSAGES/messages.po - 1
-
Empty translation for error message · Line 10679-10680The msgstr is empty, but this is an operator-facing error message, not a user-facing UI label. Consider adding translation for consistency, though impact is lower.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/translations/de/LC_MESSAGES/messages.po - 1
- Format string variable mismatch · Line 228-233
-
tests/unit_tests/commands/report/execute_test.py - 1
- Test assertion message mismatch · Line 1058-1060
Review Details
-
Files reviewed - 31 · Commit Range:
a3acdce..870adf4- superset/translations/ar/LC_MESSAGES/messages.po
- superset/translations/ca/LC_MESSAGES/messages.po
- superset/translations/cs/LC_MESSAGES/messages.po
- superset/translations/de/LC_MESSAGES/messages.po
- superset/translations/en/LC_MESSAGES/messages.po
- superset/translations/es/LC_MESSAGES/messages.po
- superset/translations/fa/LC_MESSAGES/messages.po
- superset/translations/fi/LC_MESSAGES/messages.po
- superset/translations/fr/LC_MESSAGES/messages.po
- superset/translations/it/LC_MESSAGES/messages.po
- superset/translations/ja/LC_MESSAGES/messages.po
- superset/translations/ko/LC_MESSAGES/messages.po
- superset/translations/lv/LC_MESSAGES/messages.po
- superset/translations/messages.pot
- superset/translations/mi/LC_MESSAGES/messages.po
- superset/translations/nl/LC_MESSAGES/messages.po
- superset/translations/pl/LC_MESSAGES/messages.po
- superset/translations/pt/LC_MESSAGES/messages.po
- superset/translations/pt_BR/LC_MESSAGES/messages.po
- superset/translations/ru/LC_MESSAGES/messages.po
- superset/translations/sk/LC_MESSAGES/messages.po
- superset/translations/sl/LC_MESSAGES/messages.po
- superset/translations/th/LC_MESSAGES/messages.po
- superset/translations/tr/LC_MESSAGES/messages.po
- superset/translations/uk/LC_MESSAGES/messages.po
- superset/translations/zh/LC_MESSAGES/messages.po
- superset/translations/zh_TW/LC_MESSAGES/messages.po
- superset/commands/report/execute.py
- superset-frontend/src/features/alerts/AlertReportModal.test.tsx
- tests/unit_tests/commands/report/execute_test.py
- tests/unit_tests/reports/notifications/email_tests.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
There was a problem hiding this comment.
Code Review Agent Run #561edf
Actionable Suggestions - 10
-
superset/translations/pt_BR/LC_MESSAGES/messages.po - 2
- Empty translation for format string · Line 237-239
- Inconsistent fuzzy flag and translation state · Line 12596-12604
-
superset/translations/es/LC_MESSAGES/messages.po - 1
- Missing Spanish translation · Line 976-977
-
superset/translations/zh_TW/LC_MESSAGES/messages.po - 5
- Translation Quality: Fuzzy Flag Removal · Line 191-193
- Translation Quality: Fuzzy Flag Removal · Line 892-894
- Translation Quality: Fuzzy Flag Removal · Line 6300-6302
- Translation Quality: Fuzzy Flag Removal · Line 10971-10975
- Translation Quality: Fuzzy Flag Removal · Line 12328-12333
-
superset/translations/pl/LC_MESSAGES/messages.po - 2
- Incorrect translation replaced with empty · Line 967-968
- Incorrect CSV translation replaced with empty · Line 6605-6607
Review Details
-
Files reviewed - 6 · Commit Range:
870adf4..83d22a1- superset/translations/pl/LC_MESSAGES/messages.po
- superset/translations/zh/LC_MESSAGES/messages.po
- superset/translations/zh_TW/LC_MESSAGES/messages.po
- superset/translations/es/LC_MESSAGES/messages.po
- superset/translations/fa/LC_MESSAGES/messages.po
- superset/translations/pt_BR/LC_MESSAGES/messages.po
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
|
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. |
SUMMARY
Adds XLSX as an output format for chart-based email reports.
This change:
.xlsxfiles..ziparchives.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.
After
Chart reports can be delivered as XLSX email attachments.
TESTING INSTRUCTIONS
ALERT_REPORTSfeature flag and configure email reports..xlsxattachment that opens successfully..ziparchive with XLSX files.ADDITIONAL INFORMATION
ALERT_REPORTS