Skip to content

fix(declarative): raise CSV field size limit in CsvParser - #1114

Draft
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786550116-declarative-csv-field-size
Draft

fix(declarative): raise CSV field size limit in CsvParser#1114
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786550116-declarative-csv-field-size

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

The declarative CsvParser (sources/declarative/decoders/composite_raw_decoder.py) never called csv.field_size_limit, so any low-code connector reading a CSV response failed on a single value larger than Python's default 131,072 characters:

Error: field larger than field limit (131072)

The file-based CDK parser has raised this to 2**31 since airbytehq/airbyte#36320 (sources/file_based/file_types/csv_parser.py:261-269), but that is a separate parser implementation. Because csv.field_size_limit is process-global, the declarative path's effective limit was whatever the process happened to have — i.e. it depended on whether the file-based parser module had been imported, which is not something a manifest author controls.

This sets the limit explicitly in the declarative parser, using the same value as the file-based parser so the two paths agree:

CSV_FIELD_MAX_BYTES = 2**31
...
csv.field_size_limit(CSV_FIELD_MAX_BYTES)
reader = csv.DictReader(text_data, ...)

Reported by a customer whose source carried a 152,579-character free-text field: the affected records simply did not come through, with nothing in the connection configuration indicating a size cap.

Deliberately out of scope: exposing this as a CsvDecoder manifest option (no declarative_component_schema.yaml change). Happy to add it if reviewers would rather it be configurable per connector than a fixed default.

Test plan

New regression test in unit_tests/sources/declarative/decoders/test_composite_decoder.py decodes a 200 KiB single field value, with the global limit deliberately lowered to 131_072 beforehand (and restored after) so the test fails without the source change. Confirmed the pre-change failure mode is field larger than field limit (131072).

Also ran the declarative decoder suite (62 passed) and the file-based CSV parser suite (69 passed) to check the shared global limit change doesn't regress the other path.

Link to Devin session: https://app.devin.ai/sessions/474c9fe5bfca4e21acce8d128cba3c8a

Co-Authored-By: lucas.leadbetter@airbyte.io <5595530+lleadbet@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1786550116-declarative-csv-field-size#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1786550116-declarative-csv-field-size

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 230 tests  +1   4 218 ✅ +1   8m 28s ⏱️ +4s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 1fe4c7b. ± Comparison against base commit 68bc294.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 233 tests  +1   4 221 ✅ +1   12m 45s ⏱️ -2s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 1fe4c7b. ± Comparison against base commit 68bc294.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Cross-reference: #1116 was opened from a separate Devin session for the same oncall issue and is a superset of this change (also adds an optional max_field_size property on CsvDecoder and converts the overflow into an actionable AirbyteTracedException). These two should not both land — flagging for maintainers to pick one.


Devin session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants