feat: support custom config transformations #653
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@christo/custom-config-migration#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 christo/custom-config-migrationHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Full)3 698 tests 3 687 ✅ 18m 7s ⏱️ Results for commit 26d09a0. ♻️ This comment has been updated with latest results. |
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant DeclarativeManifest
participant ModelParser
participant ModelToComponentFactory
participant CustomTransformationClass
DeclarativeManifest->>ModelParser: Parse manifest with CustomConfigTransformation
ModelParser->>ModelToComponentFactory: Instantiate CustomConfigTransformationModel
ModelToComponentFactory->>CustomTransformationClass: Create instance using class_name and parameters
CustomTransformationClass-->>ModelToComponentFactory: Return component instance
ModelToComponentFactory-->>ModelParser: Return instantiated transformation
ModelParser-->>DeclarativeManifest: Transformation ready for config processing
Suggested reviewers
Would you like me to suggest additional test cases focusing on error handling or edge cases for custom transformation classes, or do you feel the current coverage is sufficient? Wdyt? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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)
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: 2
🔭 Outside diff range comments (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
2163-2187: Pipeline formatting issue needs attention.The Ruff linter is flagging formatting issues in this range. Since this is an auto-generated file from
declarative_component_schema.yaml, would you mind running the code formatter or regenerating the file to resolve these style violations?
🧹 Nitpick comments (2)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (2)
224-230: Pipeline fails – please sort the import you just added
ruffflagged the import block as unsorted after the addition ofCustomConfigTransformationModel. Runningruff --fixorisortwill automatically place the new import in the correct position (alongside the otherCustom*models), keeping the huge import section deterministic. wdyt?
690-696: Explicit constructor could aid type-safety
CustomConfigTransformationModelis wired to the genericcreate_custom_component. That works, but all other *_TransformationModel² (e.g.AddFields,RemoveFields) have explicit helpers that enforce theConfigTransformationinterface at construction time.
Would it make sense to add a smallcreate_custom_config_transformationwrapper (delegating tocreate_custom_component) so future refactors can rely on static dispatch? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(3 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(3 hunks)airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py(2 hunks)unit_tests/sources/declarative/transformations/config_transformations/test_custom_config_transformation.py(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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.
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (6)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (6)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
</retrieved_learning>
<retrieved_learning>
Learnt from: ChristoGrab
PR: #221
File: airbyte_cdk/sources/utils/transform.py:0-0
Timestamp: 2025-01-16T00:50:39.069Z
Learning: In the TypeTransformer class, the data being transformed comes from API responses or source systems, so only standard JSON-serializable types are expected. The python_to_json mapping covers all expected types, and it's designed to fail fast (KeyError) on unexpected custom types rather than providing fallbacks.
</retrieved_learning>
unit_tests/sources/declarative/transformations/config_transformations/test_custom_config_transformation.py (1)
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#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/models/declarative_component_schema.py (5)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: ChristoGrab
PR: #221
File: airbyte_cdk/sources/utils/transform.py:0-0
Timestamp: 2025-01-16T00:50:39.069Z
Learning: In the TypeTransformer class, the data being transformed comes from API responses or source systems, so only standard JSON-serializable types are expected. The python_to_json mapping covers all expected types, and it's designed to fail fast (KeyError) on unexpected custom types rather than providing fallbacks.
</retrieved_learning>
🧬 Code Graph Analysis (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
CustomConfigTransformation(163-174)
🪛 GitHub Actions: Linters
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
[error] 5-617: Ruff: Import block is un-sorted or un-formatted. Organize imports. 1 error found, 1 fixable with the '--fix' option.
unit_tests/sources/declarative/transformations/config_transformations/test_custom_config_transformation.py
[error] 25-35: Ruff formatting check failed. The file requires reformatting to comply with code style.
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
[error] 2163-2187: Ruff formatting check failed. The file requires reformatting to comply with code style.
🔇 Additional comments (11)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
3852-3856:CustomConfigTransformationadded to the normal-sync transformation list – looks good!
No issues spotted in this hunk.
3884-3890:CustomConfigTransformationalso wired intoConfigMigration– great coverage
Everything hooks up cleanly here as well.unit_tests/sources/declarative/transformations/config_transformations/test_custom_config_transformation.py (5)
1-10: LGTM!The imports and file header look clean and follow the expected conventions.
12-19: Nice implementation!The mock class structure is clean and well-documented. The constructor properly handles optional parameters with safe defaults.
21-32: Great logic implementation!The transform method correctly identifies user keys and applies transformations appropriately. The parameter handling is clean and the comments provide helpful context.
35-43: Solid test coverage!The test properly validates both the preservation of original config and the addition of transformed fields. Clean and focused.
46-56: Excellent parameterized test!This test nicely validates that parameters are correctly applied during transformation. The assertions comprehensively check all expected fields.
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
163-175: The newCustomConfigTransformationclass structure looks solid!The implementation follows the established pattern of other custom classes in the codebase with proper
Configsettings, type literal, and class name validation. The description and example clearly indicate the expected fully-qualified naming format. This should work well for the Mailchimp migration use case mentioned in the PR objectives, wdyt?
2166-2166: Clean integration into ConfigMigration transformations union!Adding
CustomConfigTransformationto the transformations list allows it to be used within migration workflows, which aligns perfectly with the PR's goal of supporting configuration migrations.
2183-2183: Good addition to ConfigNormalizationRules transformations as well!This ensures custom transformations can be used both in migrations and normalization rules, providing flexibility for different use cases.
1-5: Note: This is an auto-generated fileBased on the retrieved learnings, this file is auto-generated from
declarative_component_schema.yamland should generally be ignored in the reviewing order. The changes here should reflect updates made to the source YAML schema. Just wanted to make sure this is intentional and the schema file was updated accordingly, wdyt?
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
...rces/declarative/transformations/config_transformations/test_custom_config_transformation.py
Outdated
Show resolved
Hide resolved
What
Adds support for custom config transformations for config migrations. Added to support the migration of source Mailchimp, which makes a separate API call to a metadata endpoint to obtain the connection's data center.
How
Summary by CodeRabbit