Repository navigation
fix(low-code): warn when incremental_dependency is set on a stream without incremental_sync - #1099
Merged
Serhii Lazebnyi (lazebnyi) merged 3 commits intoAug 7, 2026
Conversation
👋 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@lazebnyi/warn-incremental-dependency-no-cursor#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 lazebnyi/warn-incremental-dependency-no-cursorPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughChangesIncremental Dependency Validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
unit_tests/sources/declarative/parsers/test_model_to_component_factory.py (1)
5684-5786: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the supported router shapes.
Could you add parameter cases for a
partition_routerlist and aGroupingPartitionRouterthat wraps theSubstreamPartitionRouter? The factory has separate handling for both shapes, but this test only exercises the direct router case. wdyt?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unit_tests/sources/declarative/parsers/test_model_to_component_factory.py` around lines 5684 - 5786, Extend test_incremental_dependency_without_incremental_sync_warns with parameterized definitions covering a partition_router list and a GroupingPartitionRouter wrapping the SubstreamPartitionRouter. Keep the existing direct-router cases and warning expectations, and construct each supported shape through _stream_definition_with_incremental_dependency_parent so the factory’s separate router-handling paths are exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@unit_tests/sources/declarative/parsers/test_model_to_component_factory.py`:
- Around line 5684-5786: Extend
test_incremental_dependency_without_incremental_sync_warns with parameterized
definitions covering a partition_router list and a GroupingPartitionRouter
wrapping the SubstreamPartitionRouter. Keep the existing direct-router cases and
warning expectations, and construct each supported shape through
_stream_definition_with_incremental_dependency_parent so the factory’s separate
router-handling paths are exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5ac1fd5-77b1-4df5-99b8-3e989398074c
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.pyunit_tests/sources/declarative/parsers/test_model_to_component_factory.py
Patrick Nilan (pnilan)
approved these changes
Jul 31, 2026
Patrick Nilan (pnilan)
left a comment
Patrick Nilan (pnilan)
left a comment
Collaborator
There was a problem hiding this comment.
approving, one non-blocking comment.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
4594-4649: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCould you pass
model.parametersto each interpolation?Lines 4595-4647 pass
parameters={}. This ignores$parametersdeclared onRateLimitedMultipleTokenAuthenticatorModel. Token values, the quota-status URL, headers, andmax_wait_timecannot resolve component parameters.Use
model.parameters or {}for theseInterpolatedString.createcalls. Wdyt?Proposed fix
- InterpolatedString.create(model.tokens, parameters={}).eval(config) + InterpolatedString.create(model.tokens, parameters=model.parameters or {}).eval(config) - InterpolatedString.create(token, parameters={}).eval(config) + InterpolatedString.create(token, parameters=model.parameters or {}).eval(config) - InterpolatedString.create(model.quota_status_source.url, parameters={}).eval(config) + InterpolatedString.create(model.quota_status_source.url, parameters=model.parameters or {}).eval(config) - InterpolatedString.create(value, parameters={}).eval(config) + InterpolatedString.create(value, parameters=model.parameters or {}).eval(config) - InterpolatedString.create(model.max_wait_time or "PT2H", parameters={}).eval(config) + InterpolatedString.create(model.max_wait_time or "PT2H", parameters=model.parameters or {}).eval(config)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py` around lines 4594 - 4649, Update the InterpolatedString.create calls in the RateLimitedMultipleTokenAuthenticatorModel conversion flow to pass model.parameters or {} instead of parameters={}. Apply this consistently to token interpolation, quota_status_source.url, request headers, and max_wait_time so all component parameters resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py`:
- Around line 4594-4649: Update the InterpolatedString.create calls in the
RateLimitedMultipleTokenAuthenticatorModel conversion flow to pass
model.parameters or {} instead of parameters={}. Apply this consistently to
token interpolation, quota_status_source.url, request headers, and max_wait_time
so all component parameters resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8355ac53-6e8b-48a3-a626-07cccb306277
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
Serhii Lazebnyi (lazebnyi)
deleted the
lazebnyi/warn-incremental-dependency-no-cursor
branch
August 7, 2026 02:13
This was referenced Sep 29, 2026
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
2 participants
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.
What
Log a warning when a stream has
incremental_dependency: truein its parent stream configuration but does not defineincremental_syncof its own.Why
The parent stream's cursor is only persisted under the
parent_statekey of an incremental substream's state. On a full-refresh substream this setting is silently ignored — the parent is re-read from the beginning and all child partitions are re-fetched on every sync. This is exactly the trap behind airbytehq/airbyte#61567 / #1067.How
create_default_streamnow checks the stream's partition router (including routers wrapped inGroupingPartitionRouterand lists of routers) and logs a warning explaining the misconfiguration and how to fix it.Test
test_model_to_component_factory.py: a full-refresh substream withincremental_dependency: trueemits the warning; the same substream withincremental_syncdoes not.Summary by CodeRabbit
Bug Fixes
Tests