Repository navigation
feat(standard-tests): read incremental streams from a far-future state - #1265
Conversation
Port CAT's test_state_with_abnormally_large_values. An incremental entry with future_state.future_state_path becomes its own scenario; the new test reads its incremental streams from that state and fails when a stream returns records, emits no state, has no state in the file, or (declarative) emits a cursor outside its declared format.
…e state A read from a future state expects no records, so a stream without data still shows whether the connector parses its state and emits state. The future-state scenario keeps only its own empty_streams.
… future state A test catalog that configures a stream full_refresh no longer exempts it from the future-state read: every stream of the entry that discover reports as supporting incremental sync is read in incremental mode.
👋 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@tolik0/cdk/future-state-read#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 tolik0/cdk/future-state-readPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Fast)6 434 tests +126 6 397 ✅ +111 6m 21s ⏱️ -24s Results for commit 21e5dbb. ± Comparison against base commit 4f921fe. This pull request removes 1 and adds 127 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
PyTest Results (Full)6 437 tests +126 6 401 ✅ +112 11m 17s ⏱️ - 1m 4s Results for commit 21e5dbb. ± Comparison against base commit 4f921fe. This pull request removes 1 and adds 127 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…k image test` Mark test_docker_image_build_and_read_from_future_state with image_tests and the docker skipif, as its sibling Docker read. Connector CI reads Python connectors whose pinned CDK predates the in-process suite through `airbyte-cdk image test`, which selects only image_tests, so the future-state read never ran there. A --read-scenarios list is now checked against the read and future-state configs in both Docker reads, so a typo fails and a name only one of them reads is not an error. --read-from-streams narrows the catalog before the state file's coverage is checked, so the state must cover only the streams read.
…config, and fail a future-state catalog that missing_streams empties
find_cursor_format_problems called streams(config={}), which a Python
declarative source's streams override may read; it now passes the source's own
config, and reports a failure to build the streams as one more problem (naming
the exception type only) instead of raising over the read's verdict.
build_future_state_catalog checked for an incremental stream before leaving out
future_state.missing_streams, so an entry listing every stream read an empty
catalog and passed. The check now runs on the streams left to read.
…ail a declarative source built without the read's state test_read_from_future_state now passes the secret-leak check to its discover and read, as test_basic_read does, so a leaked secret fails before an assertion prints raw messages. A suite that builds a ConcurrentDeclarativeSource with the inherited create_connector builds it with no state, and the source reads only the state it was built with. The test now fails naming the fix (DeclarativeSourceTestSuite or a create_connector override) instead of reporting that the connector ignored its state.
… Docker branches Add tests for the GLOBAL state branch of the cursor format check, its recursion into per-partition states, the parent_state and partition keys it skips, a declarative source rebuilt with components.py, and the Docker future-state read's --read-scenarios=none skip, empty stream-list skip and coverage check. Correct the comment on the rebuilt source: the read's config is the first source's config after its migrations (ConcurrentDeclarativeSource.configure returns it), not a config without the manifest.
Daryna Ishchenko (darynaishchenko)
left a comment
There was a problem hiding this comment.
Approving. This changes only the test harness, so it isn't breaking and a minor release fits (same as 7.42.0). I ran the base and head collectors over every acceptance-test-config.yml on airbyte master. Only source-datascope and source-partnerstack newly fail collection, and #90878 fixes both. Existing scenarios and test IDs come out identical.
Release note: please hold the CDK release until airbytehq/airbyte#90877 and #90878 merge. Without them, those two manifest-only suites fail collection, and the unpinned CLI turns many future-state entries red.
Two non-blocking comments are inline.
…gle string literals
…for secrets before raising their errors
…nst the declared format, with the source built from the read's state The cursor format check of test_read_from_future_state now checks the state file as well as the emitted state. A value is in the format when the cursor's datetime_format alone parses it, so a file value that only one of the cursor's cursor_datetime_formats parses fails, although the read passes. A value the format parses but spells differently passes on both sides. The check builds its source with the read's catalog and state, as the read does, so a StateDelegatingStream is checked against its incremental cursor. Streams whose cursor declares no datetime format are named in a CursorFormatWarning.
… of the secret scan The platform stores state and never logs it, and the test does not print it. A partition can hold an airbyte_secret value that is also API data: source-twilio partitions by account_sid, which its records carry too, so its future-state read failed the scan. Logs, traces, connection statuses, records and stderr are still scanned. find_leaked_secrets and assert_no_secrets_in_output take a skip_message_types argument for this; other callers are unchanged.
Daryna Ishchenko (darynaishchenko)
left a comment
There was a problem hiding this comment.
Round 2 at 6b4eade3: both round-1 comments are fixed (state-file format check, Docker secret scan). One test-only issue below. Separately, airbytehq/airbyte#91059 still needs to merge before release.
…runs without the spec cache
What
Adds
test_read_from_future_stateto the source standard tests, with a Docker twin,test_docker_image_build_and_read_from_future_state. It replaces CAT'stest_state_with_abnormally_large_values(airbytehq/airbyte-internal-issues#18407, P2.2a).Every
incrementalentry ofacceptance-test-config.ymlwith afuture_state.future_state_pathbecomes a scenario of its own, with the test id'<config>' Test Scenario future state <state file stem>, for example'config' Test Scenario future state abnormal_state. The test:STREAMorGLOBALstate messages; a legacy{stream: state}mapping or an untyped message fails, naming the file and the message index.configured_catalog_path(or every discovered stream when it has none) thatdiscoverreports as supporting incremental sync, whateversync_modethe catalog gives them. The read leaves out the entry's ownempty_streamsand itsfuture_state.missing_streams, and reads each stream in incremental mode with the catalog's cursor field or its default one.missing_streamsleaves no incremental stream to read. Failure messages name the state file, streams and fields, never record or state values, because CI logs are public.discoverandreadoutput for the config'sairbyte_secretvalues, astest_checkdoes, before any assertion prints the connector's messages. The read's STATE messages are left out (design choices below). The Docker twin runs the same check before raising the connector's errors.datetime_formatalone. The source for this check is built with the read's state and catalog, as the read builds it, so aStateDelegatingStreamis checked against its incremental cursor. A stream whose cursor declares no datetime format (anIncrementingCountCursor, for example) is named in aCursorFormatWarning. The check fails if the streams cannot be built.SourceTestSuiteBasewithout overridingcreate_connector), instead of reporting records it read from the start.Other changes:
Input state:
run_test_jobtakes an inputstateforreadand passes it with--state, both in process and in Docker.Declarative sources:
DeclarativeSourceTestSuitebuilds the source with the read's state and catalog, assource-declarative-manifestdoes.ConcurrentDeclarativeSource.readignores itsstateargument.Keys:
future_state: {future_state_path, missing_streams, bypass_reason}is read inincrementalentries. These fail collection:future_state_path;future_stateoutsideincremental;cursor_formatand any other unknown key.skip_comprehensive_incremental_testsandclient_container_configare accepted and not read.Bypasses: a bypass of
incrementalis reported as a skipped test. It does not satisfy the "no runnable scenario" collection guard, becauseScenarioBypass.explains_no_scenariosis false for anincremental-only bypass.Docker twin:
test_docker_image_build_and_read_from_future_stateis markedimage_tests, soairbyte-cdk image testruns it. That is the command CI runs for a Python connector whose locked CDK predates the in-process standard tests. Liketest_docker_image_build_and_read, it is off unless--read-from-streamsis set, and it has no cursor format check.--read-scenariosdefaultandallread every future-state entry; a list reads the entries of the configs it names, and an unknown name fails.--read-from-streamsnarrows the streams before the state coverage check, so the state only has to cover the streams read.Design choices
empty_streams. A stream thatbasic_readlists as empty is still read, because it must parse its state and emit state, as in CAT. Inheriting thebasic_readempty streams left amazon-seller-partner, instagram and paypal-transaction with nothing to read.incremental. A catalog that reads a stream asfull_refreshdoes not exempt the stream from honouring state. This is stricter than CAT, but in the dry run it changed only source-insightly (now read, and passing) and source-rss (now read, and failing on a pre-existing config error).cursor_datetime_formatsand writes its state back in its own format, so the read passes and the emitted state is in format even when the file is not: only the file shows the problem. A value is in format whendatetime_formatalone parses it. A round trip (output_format(parse(value)) == value) flagged 31 connectors in the dry run on spellings the format accepts:Zfor+0000, an integer for%s, three fraction digits for six. The emitted state gets the same parse check, so a stream that echoes its input state does not fail on a spelling its file passed with.account_sid, which its spec marksairbyte_secretand its records carry too, so scanning state failed it with nothing leaked. LOG, TRACE, CONNECTION_STATUS, RECORD and stderr are still scanned.find_leaked_secretsandassert_no_secrets_in_outputtake askip_message_typesargument for this; their other callers are unchanged.Fleet dry run
The fleet is every connector in airbytehq/airbyte with a
future_state_path: 83 live sources with 101 entries. Not counted: 11 connectors still on the legacy top-leveltests:layout, which already fail collection the same way on main, and 4 archived connectors. It ran certified connectors first, then the rest. Each entry ran in process on its GSM test config:Excluded from the live runs:
Airbyte master
31d1fa65bf6, this branch at413c7268:Failures on master's state files, by cause:
ticket_activitiesdeleted_productscustomersconfig_glob(certified)historydatabases, no longer discoveredfuture_state_pathfails collection (also legacy mappings)vendorshistoryThese still fail with both airbyte PRs merged. Each fails
test_basic_readwith the same error on the same config, so the failures predate this change:ConnectorStateManager.__init__() got an unexpected keyword argument 'stream_instance_map'resolve_annotationsremovedDatetimeBasedCursor, which the CDK removed'ConnectionError' object has no attribute 'headers'time data '' does not match formatdomain_url,access_token)constremovedchannels:writescope'DefaultStream' object has no attribute 'read_records'source-partnerstack has no GSM secret, so it was not run. Its fixed file loads and covers its catalog.
Of these, azure-table, braintree, iterable, kyriba, marketo, microsoft-onedrive, recharge, rss, surveymonkey and zenloop lock a CDK older than the in-process standard tests, so CI does not run them in process: it runs the Docker read below.
Docker dry run
CI runs
airbyte-cdk image testwith--read-from-streams=defaultfor a Python connector whose locked CDK predates the in-process standard tests. 10 such connectors have a future state. I ran that command with this branch's CLI (8861764d) on master's published image of each, on its GSM test config:dispute_streamstate written as2222-01-01 00:00:00, but the cursor format is%Y-%m-%dcampaignssurvey_responsesreturns 9 recordstest_docker_image_build_and_readfails the same waytest_docker_image_build_and_readfails the same wayrecharge's
test_docker_image_build_and_readfails a schema check on master; its future-state read passes. source-zoho-crm now locks CDK 7.45.1, so it runs in process.Bypassed entries. 13 connectors bypassed the CAT test as "This test does not make sense using Concurrent CDK", so they are outside the 89 entries above. The reason no longer holds for this test, which checks that no records come back and that every stream emits state. airbytehq/airbyte#90877 removes the 12 certified bypasses (13 entries) and fixes their files: 12 entries pass. source-tiktok-marketing's test credentials get a 403 that
test_basic_readalso gets. source-mixpanel keeps its bypass: it locks CDK 6.45.0 and its components do not load on CDK 7.Catalogs that narrowed the read. An entry without a catalog reads every stream that
discoverreports as incremental. airbytehq/airbyte#90981 (stacked on #90877) removes the catalog from nine certified connectors' entries and adds state for the streams the catalogs left out: 69 more streams, about 23 s more CI time, all 9 entries pass. #90878 does the same for source-ramp. klaviyo, salesforce, orb, twitter, tiktok-marketing and snapchat-marketing keep their catalogs: the extra streams are slow, fail or are not readable on the test account.Cursor format of the state files
The format check on the state files alone, without a read, for every manifest-only connector with a future state, at airbyte
365ca9b0dbd(#90981's head): 71 entries, 56 pass. Nine connectors have values in another format:+00:00offset, format has a literalZ%Y%m%dT%H+00:00andZoffsets, format has microsecondsTseparator inmodify_timeT…Z, format is a date or uses a space+00:00offset, format has noneThe other six entries can't run in a dry run: gitlab, quickbooks and uptick rotate a single-use refresh token, so their GSM secrets were not used; partnerstack has no test secret; braze's
components.pyfails to import on CDK 7. The full run below found source-okta too, a Python connector that pins CDK 7.45.0.airbytehq/airbyte#91059 rewrites those values in each cursor's own format. On its head (merged with master after #90981), the dry run passes all 66 runnable entries, and the future-state test passes for all ten connectors except tiktok-marketing, which fails on its 403 (below).
Full run at this head
All 82 connectors that can run, in process, at airbyte
8cb574c1352(master with #91059's first commit, before #90981 merged), this branch at2a69934b: 100 entries, 80 pass. The 20 failures:create_connector. They lock a CDK older than the in-process suite, so CI runs their Docker test instead.channel_messagesreturns 216 records, because the test workspace has channels the state file has no partition for, and the state has no globalstateto fall back on. slack already failstest_basic_readin CI (deps(source-slack): 🐙 update dependencies [2026-11-10] airbyte#88808), so this adds no new red check; its state file is a follow-up.account_sidin the emitted state partitions. Both pass at0e09847f, which leaves STATE out of the secret scan.secret, a real leak. Itstest_basic_readalready fails on the same 403.CI cost (measured)
The test adds one read per future-state entry. With the airbyte fixes, the 73 passing entries take 515 s in total: median 1.9 s, p90 13.7 s, max 115.8 s (source-microsoft-sharepoint, which spends the time listing the test site's files). The next slowest are gcs
config_glob(46.8 s), google-analytics-data-api (44.6 s) and zoho-crm (32.9 s). Stale state files made reads slow before the fix: railz took 564 s and posthog 164 s and 216 s. The 12 un-bypassed entries add 350 s, most of it snapchat-marketing's daily entry (271.3 s, now the slowest; it read 66 records per stream before its file was fixed).Tests
unit_tests/test/test_future_state_read.pyadds 80 test cases (59 test functions). They cover:run_test_jobstate;load_future_stateerrors, which print no values;requests_mockthat makes 0 requests;--statemount;image_testsmark, scenario selection with--read-scenariosdefault, all, a list and an unknown name, and stream narrowing before the coverage check;missing_streamsempties;discoverorreadoutput, in process and in Docker, and a secret in a STATE message, which passes;cursor_datetime_formatsparse,Zand integer spellings the format accepts, aStateDelegatingStreamchecked with the read's state, and anIncrementingCountCursorstream reported as unchecked;test_scenario_sections.py,test_scenario_keys.pyandtest_empty_scenarios.pycover the new section. The Docker section test stubs_run_docker_spec, so it passes when run alone, without a spec another test left in the session-wide cache.Mutations
Each guard was reverted or flipped on a scratch copy, and the standard-test unit tests were run against it. All 31 mutations of the first commits fail at least one test:
test_collection_fails_when_no_scenario_and_no_bypasstest_zero_scenarios_without_bypass_lists_why_each_part_produced_nothingtest_future_state_bypass_reason_is_a_bypass_of_the_incremental_sectiontest_future_state_bypass_reason_is_a_bypass_of_the_incremental_sectiontest_misplaced_future_state_keys_are_rejected[future_state_outside_incremental]test_misplaced_future_state_keys_are_rejected[flat_future_state_path]test_future_state_fields_are_readtest_incremental_entry_is_its_own_scenario_without_the_read_empty_streamstest_incremental_entry_is_its_own_scenario_without_the_read_empty_streamstest_incremental_entry_is_its_own_scenario_without_the_read_empty_streamstest_run_test_job_passes_the_input_state_to_readtest_run_test_job_rejects_state_outside_readtest_declarative_read_starts_from_the_future_statetest_load_future_state_fails_naming_the_file_not_the_values[legacy_mapping]test_load_future_state_fails_naming_the_file_not_the_values[empty_list]test_load_future_state_fails_naming_the_file_not_the_values[untyped_message]test_catalog_reads_every_incremental_stream_but_the_missing_onestest_catalog_without_an_incremental_stream_failstest_state_must_cover_every_stream_readtest_read_fails_per_stream_on_records_and_missing_statetest_read_fails_per_stream_on_records_and_missing_statetest_cursor_value_in_another_format_fails_without_printing_it[2999-01-01T00:00:00Z]test_declarative_read_fails_on_cursor_format_problemstest_declarative_read_fails_on_cursor_format_problemstest_docker_read_passes_the_state_filetest_docker_read_is_off_by_defaulttest_source_suite_runs_each_test_for_its_sections[test_read_from_future_state-spec]test_docker_suite_runs_each_test_for_its_sections[test_docker_image_build_and_read_from_future_state-spec]test_incremental_entry_is_its_own_scenario_without_the_read_empty_streamstest_catalog_reads_the_incremental_streams_whatever_the_declared_sync_modetest_catalog_reads_every_incremental_stream_but_the_missing_onesThe Docker run and the later fixes add 18 mutations. 17 fail at least one test:
image_testsmarktest_docker_read_runs_in_image_test_like_the_docker_readreadconfigstest_docker_read_reads_every_future_state_entry_by_default[read_scenarios2]test_docker_read_fails_on_an_unknown_read_scenario--read-from-streamsnarrowingtest_docker_read_needs_state_only_for_the_streams_it_readstest_docker_read_needs_state_only_for_the_streams_it_reads--read-scenarios noneignoredtest_docker_read_is_off_with_read_scenarios_nonetest_docker_read_fails_on_a_stream_without_stateconfig={}tryaround building the streamsmissing_streamsfiltertest_catalog_whose_incremental_streams_are_all_missing_failstest_generic_suite_fails_a_declarative_source_built_without_statediscovertest_in_process_read_fails_on_a_secret_in_the_output[discover]readtest_in_process_read_fails_on_a_secret_in_the_output[read]_NOT_CURSOR_VALUESemptiedThe 18th, rebuilding the declarative source from the
configpassed toreadinstead of the injected config, survives because it is equivalent:ConcurrentDeclarativeSource.configurereturns the injected, migrated config, soreadalready receives it. A components test covers the rebuild's behaviour.The review fixes add 28 mutations. All fail at least one test:
discovertest_docker_read_fails_when_the_connector_prints_a_secret[discover]readtest_docker_read_fails_when_the_connector_prints_a_secret[read]readraises before the secret checktest_docker_read_fails_when_the_connector_prints_a_secret[read]discoverraises before the secret checktest_docker_read_fails_when_the_connector_prints_a_secret[discover]discovererrors not raisedtest_docker_read_still_raises_the_connector_error_without_a_leak[discover]readerrors not raisedtest_docker_read_still_raises_the_connector_error_without_a_leak[read]specnot skippedtest_docker_read_is_skipped_when_spec_failstest_file_cursor_value_in_another_format_fails_without_printing_ittest_emitted_cursor_value_in_another_format_fails_without_printing_ittest_cursor_values_in_the_declared_format_passtest_a_stream_without_a_datetime_cursor_is_uncheckedtest_cursor_format_is_checked_in_a_global_statetest_declarative_read_checks_a_state_delegating_stream_with_the_read_statetest_declarative_read_checks_a_state_delegating_stream_with_the_read_statetest_declarative_read_fails_on_a_file_value_the_cursor_parses_in_another_formatCursorFormatWarningtest_declarative_read_reports_cursor_format_problems_and_unchecked_streamstest_value_that_the_declared_format_parses_passescursor_datetime_formatstootest_declarative_read_fails_on_a_file_value_the_cursor_parses_in_another_formattest_value_that_the_declared_format_parses_passesfind_leaked_secretsignoresskip_message_typestest_find_leaked_secrets_skips_the_message_types_it_is_givenassert_no_secrets_in_outputdrops ittest_in_process_read_does_not_scan_state_messages_for_secretstest_in_process_read_does_not_scan_state_messages_for_secretstest_in_process_read_does_not_scan_state_messages_for_secretstest_docker_read_does_not_scan_state_messages_for_secretstest_in_process_read_does_not_scan_state_messages_for_secretstest_in_process_read_fails_on_a_secret_in_the_output[read]test_docker_read_fails_when_the_connector_prints_a_secret[read]_run_docker_specstub, file run alonetest_docker_suite_runs_each_test_for_its_sections[test_docker_image_build_and_read_from_future_state-incremental]Merge order
test_basic_readoverride would also run on the future-state scenario.airbyte-cdkCLI unpinned, so on release manifest-only connectors get the in-process test, and Python connectors whose locked CDK predates the in-process suite get the Docker test through the CI fallback. Other Python connectors get the in-process test when they bump the CDK.CI on this PR
On
8861764d,Check: source-shopifyfailed on the missingdeleted_productsstate that airbytehq/airbyte#90877 has since fixed, andCheck: destination-motherduckfailed on infrastructure (ConnectorSecretWithNoValidVersionsError), as on other open CDK PRs. Both re-run on the current head.cdk-migrations.mdhas the entry under the unreleased section.git merge-treeagainst the open CDK PRs that touch the same files, counting only conflicts that this branch adds:airbyte_cdk/test/standard_tests/declarative_sources.py, where this PR wrapsread.discoverandread) conflicts in_spec_lint.py: it skips RECORD messages infind_leaked_secretsfor every caller, where this PR addsskip_message_types. Whichever merges second keeps both. feat(standard-tests): run the secret leak check on discover and read output #1267 still scans STATE, so ontest_basic_readit would fail source-twilio on the sameaccount_sid; and its RECORD skip breakstest_in_process_read_fails_on_a_secret_in_the_output[read]here, which must move to a LOG leak.test_basic_read) conflicts insource_base.py, feat(standard-tests): build sources that require catalog, config and state #1232 (sources that require catalog, config and state) inconnector_base.py, and feat(standard-tests): lint spec structure, form rendering and OAuth config paths #1227 (spec lint) in_spec_lint.py.🤖 Generated with Claude Code