fix(connectors): replace overloaded InvalidRecord with distinct error variants#3194
fix(connectors): replace overloaded InvalidRecord with distinct error variants#3194atharvalade wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #3194 +/- ##
=============================================
- Coverage 74.10% 58.49% -15.62%
Complexity 943 943
=============================================
Files 1164 1163 -1
Lines 103048 92923 -10125
Branches 80081 69957 -10124
=============================================
- Hits 76368 54351 -22017
- Misses 23995 35971 +11976
+ Partials 2685 2601 -84
🚀 New features to boost your workflow:
|
Removed redundant phrasing from SchemaMismatch error documentation.
| #[error("Write failure: {0}")] | ||
| WriteFailure(String), | ||
| /// A catalog or transaction-level failure (e.g. applying or committing an | ||
| /// Iceberg transaction). Callers may retry on transient catalog outages. |
There was a problem hiding this comment.
doc "callers may retry on transient catalog outages" is misleading. action.apply() at router/mod.rs:213 is in-memory transaction prep - deterministic failures (invalid partition spec, schema validation) cannot be retried. only tx.commit(catalog) at router/mod.rs:222 hits the network. suggest dropping the retry claim, or splitting into ApplyError (deterministic) vs CommitError (transient-eligible).
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need a review, please ensure CI is green and the PR is rebased on the latest master. Don't hesitate to ping the maintainers - either @core on Discord or by mentioning them directly here on the PR. Thank you for your contribution! |
|
/author |
Which issue does this PR close?
Closes #3176
Rationale
Error::InvalidRecordwas used for five unrelated failure modes in the Iceberg sink'swrite_datafunction, making it impossible for callers to distinguish schema mismatches from I/O failures from catalog outages.What changed?
The Iceberg sink mapped Arrow schema conversion errors, Parquet write failures, and Iceberg catalog transaction failures all to
Error::InvalidRecord. Callers could not programmatically decide whether to fix a table definition, skip a corrupt message, or retry a catalog outage.Three new SDK error variants —
SchemaMismatch(String),WriteFailure(String),CatalogError(String)— replace the overloadedInvalidRecordat the appropriate call sites.InvalidRecordis preserved only for the genuine record-batch deserialization error.Local Execution
AI Usage