You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "yayunn (via GitHub)" <gi...@apache.org> on 2026/01/19 10:19:47 UTC
[PR] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
yayunn opened a new pull request, #37245:
URL: https://github.com/apache/superset/pull/37245
## Summary
This PR fixes an issue where CSV attachments generated by Reports are missing the UTF-8 BOM, even when `CSV_EXPORT.encoding` is set to `utf-8-sig`.
This causes non-ASCII characters (e.g. CJK) to appear garbled in Excel and some email clients.
## Problem
- SQL Lab CSV exports correctly include the BOM
- Report email CSV attachments do not include the BOM
- This leads to inconsistent behavior between export paths
## Solution
- Ensure that CSV bytes returned by the Reports execution pipeline contain a UTF-8 BOM when `CSV_EXPORT.encoding` is `utf-8-sig`
- Avoid double BOM
- Scope the fix only to Reports export path
## Screenshots / Logs
Before:
- CSV bytes start with: `e5 94 ae ...` (no BOM)
After:
- CSV bytes start with: `ef bb bf e5 94 ae ...` (correct UTF-8 BOM)
## Checklist
- [x] Code compiles
- [x] Fix is scoped to Reports only
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-3767564923
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 880edd721ebc0ef550fffbd418f59ae7b0619dad |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/696e05448ec78c00082617a2 |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-37245--superset-docs-preview.netlify.app](https://deploy-preview-37245--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> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-37245--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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-3767549930
CodeAnt AI is reviewing your PR.
---
### Thanks for using CodeAnt! π
We're free for open-source projects. if you're enjoying it, help us grow by sharing.
[Share on X](https://twitter.com/intent/tweet?text=Just%20tried%20%40CodeAntAI%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos.%20Worth%20checking%20out%3A&url=https%3A//codeant.ai) Β·
[Reddit](https://www.reddit.com/submit?title=Check%20out%20CodeAnt%20for%20automated%20code%20review&text=Just%20tried%20CodeAnt%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos.%20Worth%20checking%20out%3A%20https%3A//codeant.ai) Β·
[LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcodeant.ai&mini=true&title=Check%20out%20CodeAnt%20for%20automated%20code%20review&summary=Just%20tried%20CodeAnt%20for%20automated%20code%20review%20and%20I%27m%20impressed%21%20Free%20for%20open%20source%20with%20a%20free%20trial%20for%20private%20repos)
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #37245:
URL: https://github.com/apache/superset/pull/37245#discussion_r2713911223
##########
superset/commands/report/execute.py:
##########
@@ -436,6 +436,20 @@ def _get_pdf(self) -> bytes:
return pdf
+ def _ensure_utf8_bom(self, csv_data: bytes, encoding: str) -> bytes:
+ """
+ Ensure CSV bytes contain UTF-8 BOM when encoding is utf-8-sig.
+ Avoid double BOM.
+ """
+ if not csv_data:
+ return csv_data
+
+ enc = (encoding or "").lower().replace("_", "-")
+ if enc in ("utf-8-sig", "utf8-sig"):
Review Comment:
The encoding normalization at line 447 replaces underscores with hyphens, converting both "utf_8_sig" and "utf-8-sig" to "utf-8-sig". Therefore, checking for "utf8-sig" (without any separator) in line 448 is unnecessary after this normalization.
Consider simplifying to just check for "utf-8-sig":
```python
enc = (encoding or "").lower().replace("_", "-")
if enc == "utf-8-sig":
```
Alternatively, if you want to handle "utf8sig" (no separator), you could check for both "utf-8-sig" and "utf8sig", but the current check for "utf8-sig" (with hyphen but no separators elsewhere) won't match any real input after normalization.
```suggestion
if enc == "utf-8-sig":
```
##########
superset/commands/report/execute.py:
##########
@@ -436,6 +436,20 @@ def _get_pdf(self) -> bytes:
return pdf
+ def _ensure_utf8_bom(self, csv_data: bytes, encoding: str) -> bytes:
+ """
+ Ensure CSV bytes contain UTF-8 BOM when encoding is utf-8-sig.
+ Avoid double BOM.
+ """
+ if not csv_data:
+ return csv_data
+
+ enc = (encoding or "").lower().replace("_", "-")
+ if enc in ("utf-8-sig", "utf8-sig"):
+ if not csv_data.startswith(b"\xef\xbb\xbf"):
+ return b"\xef\xbb\xbf" + csv_data
+ return csv_data
Review Comment:
The new method _ensure_utf8_bom lacks test coverage. Given that this addresses a specific encoding issue that could lead to data corruption (garbled characters in Excel), it would be valuable to add a test that verifies:
1. When CSV_EXPORT.encoding is "utf-8-sig", the returned CSV bytes start with the UTF-8 BOM (b"\xef\xbb\xbf")
2. When a BOM already exists, it's not duplicated
3. When the encoding is not utf-8-sig, no BOM is added
Consider adding a test in tests/integration_tests/reports/commands_tests.py similar to test_email_chart_report_schedule_with_csv but with assertions on the BOM presence.
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-4716000119
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-5802582287
Thanks @yayunku, and sorry for the runaround here. #40801 landed on `master` in June and fixed `CsvResponse` to honor the `CSV_EXPORT` encoding, so the chart data endpoint that reports pull from already emits the BOM for `utf-8-sig`. That makes this a no-op now, so I'm closing it. If you still see a BOM-less attachment on `master`, holler and we'll reopen.
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #37245: fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig
URL: https://github.com/apache/superset/pull/37245
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-4725261322
<!-- translation-regression-bot -->
## β οΈ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `de`. 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 |
|----------|-------------:|------------:|----:|
| `de` | 0 | 997 | +997 |
### 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 (`de`):
```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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-4698989252
Thanks @yayunn, let's see if we can get this across the finish line. CI is currently red: the Python-Integration suites (test-mysql/test-postgres/test-sqlite) are failing, likely an existing report/CSV test now seeing the BOM. I'm updating the branch now, but if we can get those green, and a small unit test asserting the BOM is prepended only for utf-8-sig, we'll take a fresh look.
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-3767564784
CodeAnt AI finished reviewing your PR.
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-4699449109
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-3780634604
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/37245?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 `10.00000%` with `9 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.12%. Comparing base ([`476e454`](https://app.codecov.io/gh/apache/superset/commit/476e454384dedb279493a780c4f6002b95fc478b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`880edd7`](https://app.codecov.io/gh/apache/superset/commit/880edd721ebc0ef550fffbd418f59ae7b0619dad?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 28 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/37245?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/37245?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=) | 10.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/37245?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 #37245 +/- ##
===========================================
+ Coverage 0 48.12% +48.12%
===========================================
Files 0 644 +644
Lines 0 48235 +48235
Branches 0 5288 +5288
===========================================
+ Hits 0 23211 +23211
- Misses 0 24361 +24361
- Partials 0 663 +663
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/37245/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/37245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `42.75% <10.00%> (?)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/37245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.34% <10.00%> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/37245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.11% <10.00%> (?)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/37245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (?)` | |
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 Sentry](https://app.codecov.io/gh/apache/superset/pull/37245?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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37245:
URL: https://github.com/apache/superset/pull/37245#issuecomment-3767550359
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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] fix(reports): ensure CSV attachments use UTF-8 BOM when encoding is utf-8-sig [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #37245:
URL: https://github.com/apache/superset/pull/37245#discussion_r3418901227
##########
superset/commands/report/execute.py:
##########
@@ -506,6 +506,20 @@ def _get_pdf(self) -> bytes:
return pdf
+ def _ensure_utf8_bom(self, csv_data: bytes, encoding: str) -> bytes:
+ """
+ Ensure CSV bytes contain UTF-8 BOM when encoding is utf-8-sig.
+ Avoid double BOM.
+ """
+ if not csv_data:
+ return csv_data
+
+ enc = (encoding or "").lower().replace("_", "-")
+ if enc in ("utf-8-sig", "utf8-sig"):
+ if not csv_data.startswith(b"\xef\xbb\xbf"):
+ return b"\xef\xbb\xbf" + csv_data
+ return csv_data
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests for new method</b></div>
<div id="fix">
New method `_ensure_utf8_bom` lacks unit tests. Per BITO.md rule [11730], new functionality must include tests covering success paths, error scenarios, and edge cases. Add tests to `tests/unit_tests/commands/report/execute_test.py`.
</div>
</div>
<small><i>Code Review Run #2c602d</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