Skip to content

fix(low-code): skip null parent keys in SubstreamPartitionRouter - #1207

Merged
Baz (bazarnov) merged 1 commit into
mainfrom
bazarnov/fix-substream-null-parent-key
Oct 6, 2026
Merged

Baz (bazarnov) merged 1 commit into
mainfrom
bazarnov/fix-substream-null-parent-key

Conversation

@bazarnov

Copy link
Copy Markdown
Contributor

Overview

👉 TL;DR: Child streams now skip parent records whose linking field is null, the same way they already skip records where that field is missing. Before, those records produced child requests with no usable ID, which usually failed the sync or fetched data without the parent filter.

Specifically, SubstreamPartitionRouter.stream_slices now reads parent_key with dpath.get(..., default=None) and yields no slice when the value is None, logging the skip at debug level. 0, False and "" are still emitted.

Sacha Brouck (@SBrouck) proposed the same fix in the issue thread.

Changes

  • substream_partition_router.py: replace the try/except KeyError around the parent_key lookup with dpath.get(..., default=None) and skip the record when the value is None (missing path, null leaf or null intermediate object). The skip is logged at debug level, which resolves the FIXME asking for that log.
  • test_substream_partition_router.py: parametrize the missing-key test into test_substream_partition_router_skips_parent_record_without_partition_value with missing_key, null_value, nested_null_value and null_intermediate cases. The null_value case uses 0 as the valid key, and the test now also asserts that the parent cursor observes the skipped record.

Review Spotlight

Reviewers with limited time, please review first:

Risks

Parent records with a null parent_key value no longer produce child slices. Manifest-only connectors get this with the next SDM base image bump, Python connectors with their CDK bump.

Blast radius on airbyte monorepo master (fc800755): git grep -l SubstreamPartitionRouter over connector manifest.yaml files finds 241 manifests, and resolving $refs with ManifestReferenceResolver gives 1325 ParentStreamConfigs under a stream in 238 connectors. 1002 key on the parent's primary key, which is never null. A review of the other 323 (54 connectors) finds realistic null keys in:

  • source-circleci jobs (job_number, nullable in the schema, e.g. for approval jobs): the path is job/{{ config['job_number'] or stream_partition['job_parent_number'] }} with spec default "2", so by default only duplicate /job/2 requests go away. With the override cleared, /job/None (404, sync fails) is no longer requested.
  • source-looker content_metadata (content_metadata_id): /content_metadata/None (404, sync fails) is no longer requested. The stream crosses four parents on the same field, so combinations from a null-keyed later parent, which repeated the first parent's request, also drop out. It is a Python connector on airbyte-cdk ^4.0.1, so this only reaches it with a CDK major upgrade.
  • source-survicate respondents_attributes and respondents_responses (respondent): respondents//attributes and respondents//responses are no longer requested.
  • source-linkedin-ads videos_creative_posts (content/reference): already filtered on the parent side. The filter is still needed for empty strings.

Other effects:

  • request_option (37 configs, 16 connectors): _get_request_option drops a None value (if value:), so the child request went out without the parent filter and its records landed in a None partition. That request is no longer made.
  • Configs whose partition value is not used in the child request (38 configs, 7 connectors), where a skip could drop a working request: all key on ids or on fields that are never null in practice (monday sets board_id/pulse_id only when present, google-ads filters change_status by resource type, free-agent period is a period index whose 0 still emits).
  • State: parent state is unchanged, including with incremental_dependency (125 configs, 34 connectors), checked with a local end-to-end read where the null-keyed record holds the max parent cursor. No new {partition_field: null} per-partition state is written; existing entries stay unused.

Test plan

  • python -m pytest unit_tests/sources/declarative/partition_routers/test_substream_partition_router.py::test_substream_partition_router_skips_parent_record_without_partition_value -q -p no:cacheprovider
    • With substream_partition_router.py from main: 2 failed, 2 passed. null_value and nested_null_value fail with Left contains one more item: {'partition_field': None, 'parent_slice': {'slice': 'second'}}.
    • With the fix: 4 passed.
  • python -m pytest unit_tests/sources/declarative/partition_routers unit_tests/sources/declarative/incremental unit_tests/sources/declarative/retrievers unit_tests/sources/declarative/parsers unit_tests/sources/declarative/test_concurrent_declarative_source.py -q -p no:cacheprovider: 773 passed.
  • Local end-to-end read (ConcurrentDeclarativeSource with requests_mock, parents with a valid, a null and a missing content/reference): on main the child requests include /posts/None and an unscoped /comments; with the fix only /posts/urn1 and /comments?post=urn1 are requested.
  • ruff format, ruff check and mypy --config-file mypy.ini on the changed files: clean.

Fixes #1127

A parent record whose `parent_key` path exists but holds null produced a
slice with a None partition value, so child requests rendered `None` into
the path or, with `request_option`, went out without the filter.

Treat a null value like a missing path: skip the record and log it at
debug level. The parent cursor still observes the record and closes the
partition, so parent state is unchanged. 0, False and "" are still emitted.
@bazarnov
Baz (bazarnov) requested a review from a team as a code owner October 6, 2026 07:59
@github-actions

github-actions Bot commented Oct 6, 2026

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@bazarnov/fix-substream-null-parent-key#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 bazarnov/fix-substream-null-parent-key

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

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

PyTest Results (Fast)

4 985 tests  +3   4 973 ✅ +3   7m 15s ⏱️ - 2m 18s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 338d092. ± Comparison against base commit d214b0c.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_closes_partition_even_when_parent_key_missing
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[missing_key]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[nested_null_value]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[null_intermediate]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[null_value]

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

PyTest Results (Full)

4 988 tests  +3   4 976 ✅ +3   15m 55s ⏱️ -9s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 338d092. ± Comparison against base commit d214b0c.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_closes_partition_even_when_parent_key_missing
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[missing_key]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[nested_null_value]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[null_intermediate]
unit_tests.sources.declarative.partition_routers.test_substream_partition_router ‑ test_substream_partition_router_skips_parent_record_without_partition_value[null_value]

@bazarnov
Baz (bazarnov) merged commit 4c45106 into main Oct 6, 2026
29 of 30 checks passed
@bazarnov
Baz (bazarnov) deleted the bazarnov/fix-substream-null-parent-key branch October 6, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SubstreamPartitionRouter emits partitions for explicitly null parent_key values, producing literal 'None' in request paths

2 participants