fix(low-code cdk): fix set_initial_state for incremental_dependency and empty parent state #411
Conversation
📝 WalkthroughWalkthroughThis pull request modifies the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Router as SubstreamPartitionRouter
participant ParentConf as ParentStreamConfig
Caller->>Router: set_initial_state(child_state)
Router->>ParentConf: Iterate over parent_stream_configs
alt Parent state is absent and stream has incremental dependency
Router->>Router: Migrate child state to parent state
else
Router-->>ParentConf: Skip migration
end
Router->>Caller: Return updated state
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
airbyte_cdk/sources/declarative/partition_routers/substream_partition_router.py (1)
1-376: 💡 Verification agent🧩 Analysis chain
Format code using Ruff
The pipeline is failing because of a formatting issue. Please run
poetry run ruff formatto fix code style issues in this file.
🏁 Script executed:
#!/bin/bash # Just displaying the linter error for reference echo "Ruff formatting check failed. 1 file would be reformatted." echo "Please run 'poetry run ruff format' to fix code style issues in this file."Length of output: 291
Fix code formatting using Ruff
It looks like the linter output confirms that one file would be reformatted. Could you please run
poetry run ruff formaton the file:
- airbyte_cdk/sources/declarative/partition_routers/substream_partition_router.py (lines 1-376)
to resolve the formatting issue in the pipeline? wdyt?
🧰 Tools
🪛 GitHub Actions: Linters
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'poetry run ruff format' to fix code style issues in this file.
🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/partition_routers/substream_partition_router.py (1)
285-288: Implementation looks more targeted and handles edge cases betterThis change improves the handling of the state migration for parent streams. By checking for empty parent state and incremental dependency together in the loop, you're ensuring that child state migration happens only when needed for specific parent streams.
One potential concern: Since
_migrate_child_state_to_parent_statereturns a complete parent state dictionary for all streams, assigning this toparent_staterepeatedly in the loop could potentially cause overwriting if multiple parent streams qualify for migration. But looking at line 342-346, the method builds parent state for all configurations at once, so this might not be an issue. Would you consider storing the migrated state once outside the loop for clarity, or is the current approach intentional? wdyt?# Set state for each parent stream with an incremental dependency for parent_config in self.parent_stream_configs: - if not parent_state.get(parent_config.stream.name, {}) and parent_config.incremental_dependency: - # Migrate child state to parent state format - parent_state = self._migrate_child_state_to_parent_state(stream_state) + # Check if any parent stream needs migration + needs_migration = any( + not parent_state.get(config.stream.name, {}) and config.incremental_dependency + for config in self.parent_stream_configs + ) + if needs_migration: + # Migrate child state to parent state format - only once + parent_state = self._migrate_child_state_to_parent_state(stream_state) + break
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/partition_routers/substream_partition_router.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Linters
airbyte_cdk/sources/declarative/partition_routers/substream_partition_router.py
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'poetry run ruff format' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Analyze (python)
|
/autofix
|
Fixed: https://github.com/airbytehq/airbyte-internal-issues/issues/11934
This PR updates
set_initial_stateto handle cases where child-to-parent state migration is skipped when the parent state looks like:Summary by CodeRabbit