Conversation
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing 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@pnilan/feat/add-optional-condition-to-component-mapping-definition#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 pnilan/feat/add-optional-condition-to-component-mapping-definitionHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds conditional evaluation capability to component mapping definitions, allowing users to specify conditions that must be met for a component mapping to be applied. This enables dynamic component resolution based on runtime context such as user configuration or stream-specific values.
- Adds a new optional
conditionfield toComponentMappingDefinitionschema and model - Updates all component resolvers to evaluate conditions before applying mappings
- Implements condition evaluation using
InterpolatedBooleanwith access to configuration and component values
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| declarative_component_schema.yaml | Adds condition field definition to the schema with interpolation context |
| declarative_component_schema.py | Adds condition field to ComponentMappingDefinition model |
| model_to_component_factory.py | Updates factory to pass condition parameter and fixes method signature formatting |
| components_resolver.py | Adds condition field to both ComponentMappingDefinition and ResolvedComponentMappingDefinition classes |
| config_components_resolver.py | Implements condition evaluation logic in resolver |
| parametrized_components_resolver.py | Implements condition evaluation logic in resolver |
| http_components_resolver.py | Implements condition evaluation logic in resolver |
| test_config_components_resolver.py | Adds comprehensive test coverage for various condition scenarios |
unit_tests/sources/declarative/resolvers/test_config_components_resolver.py
Show resolved
Hide resolved
📝 WalkthroughWalkthroughThis change introduces an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigComponentsResolver
participant ComponentMappingDefinition
participant InterpolatedBoolean
User->>ConfigComponentsResolver: resolve_components(stream_template_config)
loop For each ComponentMappingDefinition
ConfigComponentsResolver->>ComponentMappingDefinition: Access condition
alt condition is set
ConfigComponentsResolver->>InterpolatedBoolean: Evaluate condition with contexts
InterpolatedBoolean-->>ConfigComponentsResolver: Return True/False
alt condition is True
ConfigComponentsResolver->>ConfigComponentsResolver: Apply mapping to config
else condition is False
ConfigComponentsResolver->>ConfigComponentsResolver: Skip mapping
end
else condition is not set
ConfigComponentsResolver->>ConfigComponentsResolver: Apply mapping to config
end
end
ConfigComponentsResolver-->>User: Return resolved components
Estimated code review effort3 (~45 minutes) Suggested reviewers
Would you like to suggest adding more test cases for edge conditions, such as invalid interpolation contexts or malformed expressions, wdyt? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
4175-4184: Alignconditiondefaults with existing schema conventions?Many other
conditionproperties in this schema (e.g.AddFields,RemoveFields,RecordFilter) specify an explicitdefault: "". Adding the same here would keep the contract symmetrical and avoid downstream validators treating the field as required when empty-string semantics are intended.description: A condition that must be met for the mapping to be applied. type: string + default: ""Would you like to add this default for consistency, wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(1 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(2 hunks)airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py(3 hunks)airbyte_cdk/sources/declarative/resolvers/components_resolver.py(3 hunks)airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py(4 hunks)airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py(4 hunks)airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py(4 hunks)unit_tests/sources/declarative/resolvers/test_config_components_resolver.py(3 hunks)
🧠 Learnings (8)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the `declarative_component_schema.py` file is auto-generated from `declarative_component_schema.yaml` and should be ignored in the recommended reviewing order.
airbyte_cdk/sources/declarative/resolvers/components_resolver.py (2)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
unit_tests/sources/declarative/resolvers/test_config_components_resolver.py (2)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: aaronsteers
PR: #174
File: unit_tests/source_declarative_manifest/resources/source_the_guardian_api/components.py:21-29
Timestamp: 2025-01-13T23:39:15.457Z
Learning: The CustomPageIncrement class in unit_tests/source_declarative_manifest/resources/source_the_guardian_api/components.py is imported from another connector definition and should not be modified in this context.
airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py (3)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py (2)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (4)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
🧬 Code Graph Analysis (4)
airbyte_cdk/sources/declarative/resolvers/components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the `declarative_component_schema.py` file is auto-generated from `declarative_component_schema.yaml` and should be ignored in the recommended reviewing order.
airbyte_cdk/sources/declarative/resolvers/components_resolver.py (2)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
unit_tests/sources/declarative/resolvers/test_config_components_resolver.py (2)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: aaronsteers
PR: #174
File: unit_tests/source_declarative_manifest/resources/source_the_guardian_api/components.py:21-29
Timestamp: 2025-01-13T23:39:15.457Z
Learning: The CustomPageIncrement class in unit_tests/source_declarative_manifest/resources/source_the_guardian_api/components.py is imported from another connector definition and should not be modified in this context.
airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py (3)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py (2)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (4)
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
🧬 Code Graph Analysis (4)
airbyte_cdk/sources/declarative/resolvers/components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_boolean.py (1)
InterpolatedBoolean(29-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-intercom
- GitHub Check: Pytest (Fast)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
🔇 Additional comments (21)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
1497-1501: LGTM! Theconditionfield addition looks great.The new optional
conditionfield is properly defined with correct typing, default value, and clear documentation. This supports the PR's goal of enabling conditional component mappings. Since this file is auto-generated from the YAML schema (as noted in the retrieved learnings), it looks like the generation process worked correctly, wdyt?airbyte_cdk/sources/declarative/resolvers/components_resolver.py (1)
12-12: LGTM! Clean implementation of the conditional mapping feature.The addition of the
conditionfield to both dataclasses is well-structured. I like how you differentiate between the raw string condition inComponentMappingDefinitionand the evaluableInterpolatedBooleaninResolvedComponentMappingDefinition- this follows a nice separation of concerns pattern, wdyt?Also applies to: 26-26, 40-40
unit_tests/sources/declarative/resolvers/test_config_components_resolver.py (4)
176-176: Nice improvement to test clarity!Adding explicit test IDs makes it much easier to identify specific test cases when they run or fail. This is a thoughtful enhancement to the existing test suite.
229-313: Comprehensive test coverage for conditional scenarios!I really appreciate how thorough these test manifests are. You've covered all the key scenarios - always true/false conditions, component-value-dependent conditions, and mixed scenarios. The use of Jinja templating syntax like
{{ components_values['name'] == 'item_1' }}nicely mirrors real-world usage patterns, wdyt?
316-376: Excellent parameterized test structure!The test parameterization is very well organized. I particularly like how the
expected_conditional_paramsclearly shows what parameters should be present for each stream based on the evaluated conditions. The test case mapping (e.g., item_1 gets conditional_param but item_2 doesn't) makes the expected behavior crystal clear, wdyt?
377-387: Clean and focused test implementation!The test logic is straightforward and effective. Directly checking
stream.retriever.requester._parametersis exactly the right place to verify that conditional mappings were applied correctly. The iteration through streams with the expected parameter validation provides good coverage, wdyt?airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (3)
3803-3803: LGTM! Clean addition of condition support.This change properly passes the
conditionattribute from the model to theComponentMappingDefinitioninstance, enabling the conditional component mapping feature as intended in the PR.
3855-3857: Nice formatting improvement!Breaking the method signature into explicit parameters on separate lines enhances readability. This makes the method signature more consistent with Python style guidelines, wdyt?
3877-3877: Good catch on parameter propagation!Adding
parameters=model.parametersensures consistent parameter forwarding when creating components from mapping definitions. This maintains the parameter context throughout the component creation lifecycle, which is essential for proper interpolation and component behavior, wdyt?airbyte_cdk/sources/declarative/resolvers/parametrized_components_resolver.py (4)
15-15: LGTM! Import addition looks good.The InterpolatedBoolean import is correctly added and aligns with the conditional evaluation feature being introduced.
59-64: Nice conditional instantiation pattern!The logic for creating
InterpolatedBooleaninstances only when a condition is present is clean and follows the expected null-safety pattern. This ensures we don't create unnecessary objects when no condition is specified. wdyt?
84-84: Condition properly passed to resolved component.The condition is correctly propagated to the
ResolvedComponentMappingDefinitioninstance, maintaining the data flow integrity.
101-106: Solid conditional evaluation logic!The short-circuit logic is implemented correctly - if a condition exists and evaluates to false, the component is skipped. This provides the expected filtering behavior for conditional component mappings. The evaluation context includes both config and kwargs, which should provide access to all necessary interpolation variables. wdyt?
airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py (4)
17-17: Import looks consistent across resolvers.Good to see the InterpolatedBoolean import following the same pattern as the other resolvers in this PR.
74-79: Consistent conditional instantiation pattern.This follows the exact same safe instantiation pattern as the parametrized resolver - creating
InterpolatedBooleanonly when needed. The architectural consistency across resolvers is excellent. wdyt about this uniform approach?
99-99: Condition propagation maintained.The condition is properly passed to the resolved component definition, maintaining consistency with the other resolvers.
166-171: Conditional evaluation logic matches other resolvers.The implementation is consistent with the parametrized resolver - proper null check and short-circuit behavior when conditions evaluate to false. The uniform implementation across all resolvers should make this feature predictable and maintainable. wdyt?
airbyte_cdk/sources/declarative/resolvers/http_components_resolver.py (4)
13-13: Import follows established pattern.The InterpolatedBoolean import is consistent with the other resolvers in this feature implementation.
53-58: Same solid conditional instantiation approach.The pattern matches the other resolvers perfectly - creating
InterpolatedBooleaninstances only when conditions are present. This consistency across all three resolver types should make the feature behavior predictable for users. wdyt?
77-77: Consistent condition propagation.The condition is properly passed through to the resolved component definition, maintaining the established pattern.
108-113: Conditional evaluation with appropriate context.The evaluation logic follows the same pattern as the other resolvers, with the addition of
stream_slicein the kwargs which is appropriate for HTTP-based resolution. The conditional short-circuit behavior looks correct. Nice consistency across all resolver implementations! wdyt?
PyTest Results (Fast)3 700 tests +5 3 689 ✅ +5 6m 37s ⏱️ +8s Results for commit af83b18. ± Comparison against base commit 410571c. This pull request removes 4 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
PyTest Results (Full)3 703 tests +5 3 692 ✅ +5 18m 19s ⏱️ +18s Results for commit af83b18. ± Comparison against base commit 410571c. This pull request removes 4 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
pnilan
deleted the
pnilan/feat/add-optional-condition-to-component-mapping-definition
branch
1 participant
What
ComponentMappingDefinitionshould resolve forConfigComponentsResolverHttpComponentsResolverandParameterizedComponentsResolveras these should always be mapped.incremental_synccomponent conditionally if a user provided acursor_fieldin their the dynamic stream config.Review Order
declarative_component_schema.pymodel_to_component_factory.pycomponents_resolver.pyconfig_components_resolver.pySummary by CodeRabbit
New Features
Bug Fixes
Tests