Feat: Publish the response to process as stream_status.response in the transformation's context #190
Conversation
* 'main' of https://github.com/rpopov/airbyte-python-cdk: fix: fix sorting & __init__ imports (airbytehq#189)
rpopov
changed the title
📝 WalkthroughWalkthroughThe changes modify the Changes
Sequence DiagramsequenceDiagram
participant RecordSelector
participant DpathExtractor
participant ResponseProcessor
RecordSelector->>DpathExtractor: Extract records from response
DpathExtractor-->>RecordSelector: Return extracted records
RecordSelector->>ResponseProcessor: Filter and transform records
RecordSelector->>RecordSelector: Clean up stream state
The sequence diagram illustrates the enhanced record selection process, showing how the new Wdyt? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Nitpick comments (2)
airbyte_cdk/sources/declarative/extractors/record_selector.py (2)
25-25: Consider adjusting or clarifying the constant name.
STREAM_SLICE_RESPONSE_KEY = "response"is quite straightforward, but do you think a more specific name like"root_response"might help avoid collisions or confusion, especially if other keys get added later? wdyt?
56-57: Possibility of reusing existing config or parameters.The newly created
response_root_extractor = DpathExtractor(field_path=[], config={}, parameters={})is fine as is, but do you want to leverage the class-levelconfigorparametersfor consistency or future flexibility? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/extractors/record_selector.py(4 hunks)
🔇 Additional comments (2)
airbyte_cdk/sources/declarative/extractors/record_selector.py (2)
10-10: Looks good importing DpathExtractor.
This addition seems logically consistent with the rest of the code. Would you consider adding a short docstring or comment for future reference, so others better understand why it's being used here? wdyt?
92-101: Watch out for potential concurrency or pop fallback.
The try/finally block ensures stream_state is updated then cleaned up. Do you think it might be safer to use stream_state.pop(STREAM_SLICE_RESPONSE_KEY, None) to avoid a possible KeyError if there's parallel usage or unexpected conditions? wdyt?
rpopov
changed the title
|
Not ready yet |
#50395 Publish the response to process as stream_status.response in the transformation's context
This will keep the existing public interfaces and contracts unchanged. The only change is the contents of the stream_state object. The existing implementation publishes the response object for filtering and transformation steps (see record_selector.py).
See airbytehq/airbyte#49971
Summary by CodeRabbit
New Features
Bug Fixes