Skip to content

fix(streaming): keep CsvTransform options intact across repeated use - #8565

Merged
leandrodamascena merged 1 commit into
aws-powertools:developfrom
ArfiArfath21:fix/csv-transform-keep-options
Oct 17, 2026
Merged

leandrodamascena merged 1 commit into
aws-powertools:developfrom
ArfiArfath21:fix/csv-transform-keep-options

Conversation

@ArfiArfath21

@ArfiArfath21 ArfiArfath21 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Issue number: closes #8563

Summary

Changes

CsvTransform.transform() no longer pops encoding and newline from its own transform_options; it works on a copy. Adds a regression test that applies one CsvTransform(encoding="latin-1", delimiter=";") to two objects.

User experience

Before: the second use of the same CsvTransform instance (or a re-created S3Object transformed stream) silently decoded as UTF-8 and could raise UnicodeDecodeError or return wrong text. After: every use honours the configured options.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

AI assistance disclosure: prepared with Claude Code; I reviewed the change and ran the tests locally.

@ArfiArfath21
ArfiArfath21 requested a review from a team as a code owner October 16, 2026 18:01
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 16, 2026
@ArfiArfath21
ArfiArfath21 force-pushed the fix/csv-transform-keep-options branch from d275445 to 551d5d9 Compare October 16, 2026 18:25

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch @ArfiArfath21, and thanks for the clear repro! I confirmed the second use of the same CsvTransform fails on develop and works with this change, with options left intact. Approving. Heads-up: this and #8566 touch the same import line in test_s3_object.py, so whichever lands second will need a quick rebase.

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.93%. Comparing base (bf2e917) to head (5ef2ce0).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8565   +/-   ##
========================================
  Coverage    96.93%   96.93%           
========================================
  Files          318      318           
  Lines        15998    15999    +1     
  Branches      1419     1419           
========================================
+ Hits         15507    15508    +1     
  Misses         349      349           
  Partials       142      142           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CsvTransform.transform popped 'encoding' and 'newline' from its own
transform_options, so reusing an instance (or letting S3Object re-create its
transformed stream) silently fell back to utf-8. Work on a copy instead.
@ArfiArfath21

Copy link
Copy Markdown
Contributor Author

Rebased onto develop now that #8566 has landed. The only conflict was the shared import line and the end of test_s3_object.py in the tests, so I kept both sets of tests. No changes to csv.py. Needs a quick re-approval since the push dismissed the earlier one. Thanks!

@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after the rebase on top of #8566, the change is identical. Thanks!

@leandrodamascena
leandrodamascena merged commit ca28871 into aws-powertools:develop Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files. streaming tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: CsvTransform loses its encoding and newline options after the first use

2 participants