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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
