fix(declarative): make cursor datetime format mismatch errors actionable - #1086
Draft
devin-ai-integration[bot] wants to merge 1 commit into
Draft
fix(declarative): make cursor datetime format mismatch errors actionable#1086devin-ai-integration[bot] wants to merge 1 commit into
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…Exception 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/1785270012-datetime-format-mismatch-error#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/1785270012-datetime-format-mismatch-errorPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
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
Declarative cursor datetime-format mismatches surfaced to users as a raw Python
ValueError, e.g. the message reported inairbytehq/airbyte#68147(source-square, streamitems):That string leaks
strptimetokens, embeds the record value (so the same failure mode produces a different string per record — unusable as an aggregation key), and names neither the cursor field nor, on some paths, the stream.This introduces
DatetimeFormatMismatchErrorand raises it from the three parse sites, so the failure now carries a deterministic user-facing message with the raw detail moved tointernal_message:It subclasses both
AirbyteTracedExceptionandValueErroron purpose: every existing caller that handles parse failures (the multi-format fallback loops,ConcurrentCursor.observe/should_be_synced,StateDelegatingStreamcursor-age validation) catchesValueError, so behavior — including the deliberate fail-fast on unparseable cursor strings — is unchanged; only the exception's shape and message improve.Raise sites changed:
CustomFormatConcurrentStreamStateConverter.parse_timestampandDatetimeBasedCursor.parse_date(legacy) — the multi-format fallbacks that produced the reported message.DatetimeParser.parse— the barestrptimecall reached on single-format paths.The converter has the formats but not the stream/cursor context, so
ConcurrentCursor._get_concurrent_statere-raises with it viaerror.with_context(cursor_field=..., stream_name=...).One adjacent message fix: on the record path a mismatch was logged as
Could not find cursor field ... in record, which is wrong — the field is present, its value just does not parse. That case now logs its own warning and, as before, keeps the record.Reproduction
A local declarative source mirroring the source-square manifest (
cursor_datetime_formats: ["%Y-%m-%dT%H:%M:%S.%fZ"], cursorupdated_at, value2025-06-28T18:35:16Z) reproduced both paths onmain@6ce33eaf: the state path crashed atConcurrentCursorconstruction with the rawValueError, and the record path silently logged the misleading "could not find cursor field" warning. After the change the state path fails with the message above and the record path logs the accurate warning.Interaction with other PRs
DateTimeStreamStateConverter._get_sync_startand the factory in the same state-construction path; this PR touchesparse_timestamp/ConcurrentCursor._get_concurrent_state, so the overlap is adjacent rather than conflicting, and that PR's fail-fast-on-bad-strings guarantee is preserved.cursor_datetime_formatsanddatetime_formatin the UI #735 (robust datetime fallback inDatetimeBasedCursor) would reduce how often this error fires; it does not remove the need for a well-formed error, and nothing here depends on it.Out of scope: the source-square manifest fix (tracked in airbytehq/oncall#13180, community PR airbytehq/airbyte#68157).
Breaking change evaluation
Not breaking: no schema, spec, stream, cursor-semantics, or state-format change. The raised exception remains a
ValueErrorsubclass, so existingexcept ValueErrorhandling and connector code keep working.Declarative-First Evaluation
Not applicable — this is a CDK core/parser change, not a connector manifest change. The connector-level declarative fix (adding a whole-second
cursor_datetime_formatsentry) is deliberately excluded here.Test Coverage
New regression tests, each failing on
mainand passing here:unit_tests/sources/declarative/datetime/test_datetime_parser.py:DatetimeParser.parseraisesDatetimeFormatMismatchError(still aValueError) with the deterministic message,system_errortype, and raw detail ininternal_message;with_contextnames stream and cursor field.unit_tests/sources/streams/concurrent/test_datetime_state_converter.py:parse_timestampraise site.unit_tests/sources/streams/concurrent/test_cursor.py: state path raises with stream + cursor-field context; record path logs the new warning and still syncs the record.unit_tests/legacy/sources/declarative/incremental/test_datetime_based_cursor.py:parse_dateraise site (existingpytest.raises(ValueError)assertion extended).unit_tests/sources/declarative/test_state_delegating_stream.py::test_cursor_age_validation_raises_error_for_unparseable_cursorupdated: still fails fast, now asserting the new message and that the offending value stays ininternal_message.Full
poetry run pytest unit_testspasses (4144 tests), as doruff check,ruff format --check, andmypy --config-file mypy.ini airbyte_cdk.Resolves https://github.com/airbytehq/oncall/issues/13185:
Link to Devin session: https://app.devin.ai/sessions/5e9c950d15694125886a3ea5d01f2a20