fix(low-code): set transform_before_filtering from manifest if client_side_incremental_sync=true #515
Conversation
📝 WalkthroughWalkthroughThe changes update the logic for handling the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant Manifest as Manifest (deep copy)
participant Source as ConcurrentDeclarativeSource
participant Stream as DefaultStream
participant Selector as RecordSelector
Test->>Manifest: Enable is_client_side_incremental
Test->>Manifest: Set transform_before_filtering (True/False/None)
Test->>Source: Instantiate with Manifest and state
Source->>Stream: Group streams
Stream->>Selector: Access record_selector
Test->>Selector: Assert transform_before_filtering as expected
Possibly related PRs
Suggested labels
Suggested reviewers
Would you like to consider adding more test cases for other attributes affected by similar logic, wdyt? 📜 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)
✨ Finishing Touches
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. 🪧 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py(2 hunks)unit_tests/sources/declarative/test_concurrent_declarative_source.py(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Linters
unit_tests/sources/declarative/test_concurrent_declarative_source.py
[error] 5-55: Ruff: Import block is un-sorted or un-formatted. Organize imports. 1 error found, fixable with '--fix' option.
⏰ 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 (All, Python 3.11, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
2830-2846: Improvement to transform_before_filtering handlingThis change looks good! Instead of mutating the model attributes directly, you're now properly using a local variable with conditional expressions. This preserves any explicit value set in the manifest while still defaulting to
Truewhen needed for client-side incremental sync.The approach is cleaner and respects configuration settings better. Nice work!
unit_tests/sources/declarative/test_concurrent_declarative_source.py (1)
1880-1941: Well-structured test for transform_before_filtering behavior!This test nicely verifies that
transform_before_filteringis correctly set from the manifest whenis_client_side_incremental=true. The parameterized approach covers all cases elegantly - explicit True/False values and the default behavior.
unit_tests/sources/declarative/test_concurrent_declarative_source.py
Outdated
Show resolved
Hide resolved
|
/autofix
|
darynaishchenko
deleted the
daryna/low-code/fix-client-side-incremental-transforming
branch
For client side incremental sync use
transform_before_filteringfrom manifest, default True if not providedSummary by CodeRabbit
Bug Fixes
transform_before_filteringsetting to ensure consistent behavior based on configuration without altering internal state.Tests
transform_before_filteringoption with client-side incremental syncs.