Skip to content

fix(declarative): make cursor datetime format mismatch errors actionable - #1086

Draft
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1785270012-datetime-format-mismatch-error
Draft

fix(declarative): make cursor datetime format mismatch errors actionable#1086
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1785270012-datetime-format-mismatch-error

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Declarative cursor datetime-format mismatches surfaced to users as a raw Python ValueError, e.g. the message reported in airbytehq/airbyte#68147 (source-square, stream items):

Exception while syncing stream items: No format in ['%Y-%m-%dT%H:%M:%S.%fZ'] matching 2025-06-28T18:35:16Z

That string leaks strptime tokens, 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 DatetimeFormatMismatchError and raises it from the three parse sites, so the failure now carries a deterministic user-facing message with the raw detail moved to internal_message:

class DatetimeFormatMismatchError(AirbyteTracedException, ValueError): ...

# message           -> 'Cursor field "updated_at" of stream "items" matches none of the configured datetime formats.'
# internal_message  -> "No format in ['%Y-%m-%dT%H:%M:%S.%fZ'] matching 2025-06-28T18:35:16Z"
# failure_type      -> FailureType.system_error, original ValueError chained as the cause

It subclasses both AirbyteTracedException and ValueError on purpose: every existing caller that handles parse failures (the multi-format fallback loops, ConcurrentCursor.observe / should_be_synced, StateDelegatingStream cursor-age validation) catches ValueError, 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_timestamp and DatetimeBasedCursor.parse_date (legacy) — the multi-format fallbacks that produced the reported message.
  • DatetimeParser.parse — the bare strptime call reached on single-format paths.

The converter has the formats but not the stream/cursor context, so ConcurrentCursor._get_concurrent_state re-raises with it via error.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"], cursor updated_at, value 2025-06-28T18:35:16Z) reproduced both paths on main @ 6ce33eaf: the state path crashed at ConcurrentCursor construction with the raw ValueError, 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

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 ValueError subclass, so existing except ValueError handling 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_formats entry) is deliberately excluded here.

Test Coverage

New regression tests, each failing on main and passing here:

  • unit_tests/sources/declarative/datetime/test_datetime_parser.py: DatetimeParser.parse raises DatetimeFormatMismatchError (still a ValueError) with the deterministic message, system_error type, and raw detail in internal_message; with_context names stream and cursor field.
  • unit_tests/sources/streams/concurrent/test_datetime_state_converter.py: parse_timestamp raise 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_date raise site (existing pytest.raises(ValueError) assertion extended).
  • unit_tests/sources/declarative/test_state_delegating_stream.py::test_cursor_age_validation_raises_error_for_unparseable_cursor updated: still fails fast, now asserting the new message and that the offending value stays in internal_message.

Full poetry run pytest unit_tests passes (4144 tests), as do ruff check, ruff format --check, and mypy --config-file mypy.ini airbyte_cdk.

Resolves https://github.com/airbytehq/oncall/issues/13185:

Link to Devin session: https://app.devin.ai/sessions/5e9c950d15694125886a3ea5d01f2a20

…Exception

Co-Authored-By: bot_apk <apk@cognition.ai>
@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/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-error

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 144 tests  +5   4 132 ✅ +5   7m 57s ⏱️ +8s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 4e7ce65. ± Comparison against base commit 6ce33ea.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 147 tests  +5   4 135 ✅ +5   10m 56s ⏱️ - 1m 30s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 4e7ce65. ± Comparison against base commit 6ce33ea.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants