feat(low-code cdk): add streams limit to full resolve manifest command #455
Conversation
|
/autofix
|
📝 WalkthroughWalkthroughThe pull request renames the Changes
Sequence Diagram(s)sequenceDiagram
participant C as ConnectorBuilderHandler
participant L as TestLimits
participant S as Source
participant M as ManifestResolver
C->>L: get_limits(config)
L-->>C: return TestLimits(max_streams)
C->>S: create_source(config, limits)
S->>S: read_stream(..., limits)
C->>M: full_resolve_manifest(source, limits)
M->>L: Validate stream count using max_streams
M-->>C: return AirbyteMessage with resolved manifest
Possibly related PRs
Suggested reviewers
How does this look to you? Any adjustments you'd like to make? 📜 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: 1
🔭 Outside diff range comments (1)
airbyte_cdk/connector_builder/connector_builder_handler.py (1)
1-162:⚠️ Potential issueFix formatting issue flagged by the pipeline
The pipeline failure indicates a formatting issue with this file that needs to be fixed.
According to the error message, you need to run 'poetry run ruff format' to fix the code style issues in this file.
🧰 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.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte_cdk/connector_builder/connector_builder_handler.py(4 hunks)airbyte_cdk/connector_builder/main.py(3 hunks)unit_tests/connector_builder/test_connector_builder_handler.py(14 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
airbyte_cdk/connector_builder/connector_builder_handler.py (4)
airbyte_cdk/sources/declarative/manifest_declarative_source.py (4)
ManifestDeclarativeSource(60-436)resolved_manifest(113-114)streams(144-169)dynamic_streams(121-124)unit_tests/connector_builder/test_connector_builder_handler.py (2)
resolved_manifest(678-679)streams(810-811)airbyte_cdk/sources/declarative/concurrent_declarative_source.py (1)
streams(179-188)unit_tests/sources/mock_server_tests/mock_source_fixture.py (1)
streams(406-413)
unit_tests/connector_builder/test_connector_builder_handler.py (1)
airbyte_cdk/connector_builder/connector_builder_handler.py (1)
TestLimits(39-43)
🪛 GitHub Actions: Linters
airbyte_cdk/connector_builder/connector_builder_handler.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 (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
- GitHub Check: Analyze (python)
🔇 Additional comments (12)
airbyte_cdk/connector_builder/main.py (3)
13-13: Updated import to useTestLimitsinstead ofTestReadLimitsThe import has been updated to use the new
TestLimitsclass instead of the previousTestReadLimits. This aligns with the renaming of the class in the connector_builder_handler.py file.
76-76: Updated parameter type toTestLimitsThe parameter type in the function signature has been updated to use
TestLimits, which is consistent with the renamed class.
86-86: Addedlimitsparameter tofull_resolve_manifestcallThe function call has been updated to pass the
limitsparameter to thefull_resolve_manifestfunction, which enables the stream limit functionality.unit_tests/connector_builder/test_connector_builder_handler.py (4)
23-23: Updated import to useTestLimitsinstead ofTestReadLimitsThe import statement has been updated to reference the renamed class.
387-387: Added max_streams configuration to test configThe test configuration now includes the max_streams parameter with a value of 2, which is used to test the new stream limiting functionality.
528-528: Updated all instances of TestReadLimits to TestLimitsAll object instantiations have been updated to use the renamed class. This ensures that the tests properly exercise the new functionality.
Also applies to: 732-732, 793-793, 829-829, 869-869, 986-986, 1068-1068, 1115-1115, 1199-1199, 1285-1285, 1343-1343
1371-1371: Using TestLimits with max_streams in full_resolve_manifest testThe test for full_resolve_manifest now explicitly sets max_streams to 2, ensuring that the new stream limiting functionality is tested properly.
airbyte_cdk/connector_builder/connector_builder_handler.py (5)
7-7: Added Dict to importsAdded Dict to the imports to properly type-hint the mapped_streams dictionary in the full_resolve_manifest function.
30-30: Added constants for maximum streamsAdded constants to define the default maximum number of streams and the corresponding configuration key. This provides consistent values throughout the codebase.
Also applies to: 35-35
39-43: Renamed class to TestLimits and added max_streams fieldThe TestReadLimits class has been renamed to TestLimits and now includes a max_streams field with a default value of 100. This is a core part of the PR changes.
57-57: Updated function signatures to use TestLimitsThe function signatures for create_source and read_stream have been updated to use the TestLimits type, which is consistent with the class rename.
Also applies to: 78-78
124-142: Implemented stream limiting in full_resolve_manifestThe full_resolve_manifest function now includes logic to limit the number of streams generated for each dynamic stream according to the max_streams setting. This is the core functionality being added in this PR.
The implementation:
- Creates a dictionary to track generated streams by dynamic stream name
- Restricts the number of generated streams to the configured limit
- Extends the streams list with the limited number of generated streams
lazebnyi
deleted the
lazebnyi/add-limit-streams-to-full-resolve-manifest-command
branch
Details
Fixed: https://github.com/airbytehq/airbyte-internal-issues/issues/12275
Summary by CodeRabbit
New Features
Refactor
TestReadLimitsclass toTestLimitsfor consistency across the codebase.