fix(declarative): raise CSV decoder field size limit and attribute overflows - #1116
Draft
devin-ai-integration[bot] wants to merge 2 commits into
Draft
fix(declarative): raise CSV decoder field size limit and attribute overflows#1116devin-ai-integration[bot] wants to merge 2 commits into
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou 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/1786558917-csv-field-size-limit#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/1786558917-csv-field-size-limitPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Contributor
Author
|
Cross-reference: #1114 was opened from a separate Devin session for the same oncall issue and covers a subset of this change (raises the limit, no |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The declarative
CsvDecoder's parser built acsv.DictReaderwithout ever callingcsv.field_size_limit, so it inherited CPython's 131,072-character default. Any low-code/manifest-only connector streaming a CSV response with a field longer than that failed the sync. The file-based CDK parser has raised the limit to2**31since airbytehq/airbyte#36320; the declarative parser never got the same treatment.Confirmed locally that the failure is a hard error, not silent truncation — feeding a 200,000-character field through the reader exactly as the parser does raises
_csv.Error: field larger than field limit (131072), and nothing in the CDK catchescsv.Error, so it escaped through the generic handler with no attribution.Three changes:
CsvParser.max_field_sizedefaults to2**31, matching the file-based parser.max_field_sizeproperty onCsvDecoder(schema + regenerated model +ModelToComponentFactory), so a manifest can tune it. Existing manifests are unaffected.AirbyteTracedException(config_error) instead of a bare_csv.Error:Python's
csvmodule reports neither the offending column nor the observed field size, so the message names only the constraint it can actually attribute. Non-field-limitcsv.Errors propagate unchanged.Note on process-global state
csv.field_size_limit()mutates module-global state in_csv. Rather than mutating it at construction time (what the file-based parser does), this sets the limit at the start of the parse and restores the prior value in afinally, so the decoder doesn't leave a surprising limit in place for other parsers in the same process:The limit must be in effect for the whole iteration, not just reader construction, because
parseyields lazily.Scope note: the parser keeps its
# TODO: migrate implementation to re-use file-base classescomment — that migration is deliberately not attempted here.Test Coverage
Added to
unit_tests/sources/declarative/decoders/test_composite_decoder.py: a 200,000-character field parses successfully with the new default; a lowmax_field_sizeoverride is honored and produces the traced exception with the expected message andFailureType; and the globalcsv.field_size_limit()is restored after parsing.Related to https://github.com/airbytehq/oncall/issues/13293:
Originating community discussion: airbytehq/airbyte#84337 (reporter Juan (@jnr0790)). Product tracking: https://github.com/airbytehq/product-request-backlog/issues/115.
Requested by Devin Bot via the
/ai-fixworkflow. Suggested reviewers based on recent history incomposite_raw_decoder.py: aldogonzalez8 (addedset_values_to_noneto this same parser), and Anatolii Yatsuk.Link to Devin session: https://app.devin.ai/sessions/ad59c78feefe40a9abef98654ae80402