fix: do not crash on unparseable cursor state value during concurrent cursor construction#1070
Draft
devin-ai-integration[bot] wants to merge 2 commits into
Draft
Conversation
… cursor construction 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/1783684033-fix-datetime-state-converter-bad-cursor-value#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/1783684033-fix-datetime-state-converter-bad-cursor-valuePR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
…il-fast Co-Authored-By: bot_apk <apk@cognition.ai>
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
Fixes a hard source startup crash where a boolean value under the cursor field key reaches the datetime state converter during
ConcurrentCursorconstruction and raisesValueError: No format in [...] matching True, failing the entire source before any stream is read.Investigated for oncall issue
airbytehq/oncall#13084(HubSpot). The customer's saved connection state contained boolean bookkeeping markers (use_global_cursor: true,__ab_no_cursor_state_message: true); one reached the parent cursor's datetime parser via theassociations_*substream partition router. Because the source dies at startup, no new state is ever committed — which is why manual state edits appeared to "revert" after each failed sync.The read/observe path already tolerates a bad cursor value (
ConcurrentPerPartitionCursor.observe, #758). The construction / state-init path (ConcurrentCursor.__init__→_get_concurrent_state→convert_from_sequential_state→_get_sync_start) was still unguarded.Change
In
DateTimeStreamStateConverter._get_sync_start, ignore a boolean saved cursor value (never a valid datetime) and fall back to the configured start date instead of crashing:The guard is intentionally scoped to
boolrather than swallowing everyValueError. A genuinely malformed datetime string must still raise:StateDelegatingStream'sapi_retention_periodcursor-age validation (#890) deliberately fails fast on unparseable cursor strings, andtest_cursor_age_validation_raises_error_for_unparseable_cursorasserts that. Booleans are unambiguously invalid cursor values (they originate from internal state markers, not user data), so skipping only them fixes the crash without regressing that behavior.This is a robustness fix (fall back rather than abort); no public interface changes, non-breaking.
Test Coverage
Added regression tests in
unit_tests/sources/streams/concurrent/test_datetime_state_converter.py:_get_sync_startcases with a boolean state value (CustomFormatconverter) — falls back tostartand tozero_value.test_get_sync_start_with_boolean_state_value_does_not_raiseandtest_convert_from_sequential_state_with_boolean_state_value_does_not_raise.All fail without the fix (reproducing the
ValueError) and pass with it. Full converter file passes (36 tests) andtest_cursor_age_validation_raises_error_for_unparseable_cursorstill passes.ruff check,ruff format, andmypyclean on the changed files.Related to https://github.com/airbytehq/oncall/issues/13084:
Link to Devin session: https://app.devin.ai/sessions/ee9a064393c44676a341378e32c2a746