docs: Add inline description to $parameters in declarative component schema #834
Conversation
…schema - Add title and description to all $parameters fields in the schema - Explain automatic parameter propagation mechanism - Document that parameters are applied to component fields when not already set - Link to comprehensive documentation page for details - This description will appear in the Connector Builder UI and YAML Reference docs Requested by: AJ Steers (@aaronsteers) Co-Authored-By: AJ Steers <aj@airbyte.io>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 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@devin/1762664873-improve-parameters-schema-description#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 devin/1762664873-improve-parameters-schema-descriptionHelpful 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 inline documentation to the $parameters field throughout the declarative component schema, explaining the automatic parameter propagation mechanism that occurs in the Connector Builder framework. This documentation will be visible in the Connector Builder UI, YAML Reference documentation, and IDE autocomplete/tooltips.
- Added
titleanddescriptionfields to 73$parametersdefinitions - The description explains automatic parameter application, recursive propagation, and links to comprehensive documentation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughCentralized the Changes
Sequence Diagram(s)(omitted — schema/model-only changes; no control-flow modifications) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Would you like a short checklist of two or three serialization/deserialization tests I can add to validate representative components, wdyt? Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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 (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(73 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-18T23:40:06.391Z
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk 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.
Applied to files:
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
📚 Learning: 2024-12-11T16:34:46.319Z
Learnt from: pnilan
Repo: airbytehq/airbyte-python-cdk PR: 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.
Applied to files:
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
⏰ 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). (16)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-shopify
- GitHub Check: MyPy Check
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: preview_docs
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (python)
- Add ParametersProp definition in definitions section - Replace 71 inline $parameters blocks with $ref to ParametersProp - Run poe build to regenerate Pydantic models - This makes the schema more maintainable by defining the $parameters description once Co-Authored-By: AJ Steers <aj@airbyte.io>
…roach Co-Authored-By: AJ Steers <aj@airbyte.io>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
PyTest Results (Full)3 820 tests 3 808 ✅ 11m 1s ⏱️ Results for commit fd9aba8. ♻️ This comment has been updated with latest results. |
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (2)
1-3: Address the Ruff formatting issue.The pipeline indicates this file needs formatting. Since this is an auto-generated file, you'll want to run the formatter after regeneration.
Run the following command to fix the formatting:
poetry run ruff format airbyte_cdk/sources/declarative/models/declarative_component_schema.py
2002-2002: Update these 3 classes to useParametersPropfor consistency with the rest of the codebase.The verification confirms a genuine inconsistency. Out of 73+ parameter field definitions in the file, 71 use
Optional[ParametersProp]while only 3 outliers still useOptional[Dict[str, Any]]:DefaultErrorHandler(line 2002),DeclarativeStream(line 2537), andCustomConfigTransformation(line 1476).Since
ParametersPropis a simple BaseModel withextra = Extra.allowthat's functionally equivalent toDict[str, Any], there's no apparent semantic reason for the divergence. These three should align with the established pattern, wdyt?
🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
16-20: Consider adding documentation to ParametersProp.The new
ParametersPropclass serves as a centralized type for the$parametersfield but lacks a docstring explaining its purpose and behavior. While I understand this file is auto-generated from the YAML schema, would it make sense to add documentation in the source YAML that would appear here? This would help developers understand that this class accepts arbitrary additional parameters through theextra = Extra.allowconfiguration.Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py(68 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk 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.
📚 Learning: 2024-12-11T16:34:46.319Z
Learnt from: pnilan
Repo: airbytehq/airbyte-python-cdk PR: 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.
Applied to files:
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
📚 Learning: 2024-11-18T23:40:06.391Z
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk 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.
Applied to files:
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
airbyte_cdk/sources/declarative/models/base_model_with_deprecations.py (1)
Config(44-49)airbyte_cdk/sources/declarative/extractors/response_to_file_extractor.py (1)
ResponseToFileExtractor(23-176)airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py (2)
ConfigComponentsResolver(46-211)StreamConfig(29-41)airbyte_cdk/sources/declarative/resolvers/components_resolver.py (1)
ComponentMappingDefinition(17-27)
🪛 GitHub Actions: Linters
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
[error] 14-14: Ruff formatting required. 1 file would be reformatted. Run 'poetry run ruff format .' to fix code style issues.
⏰ 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). (14)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-shopify
- GitHub Check: Analyze (python)
- GitHub Check: Pytest (Fast)
- GitHub Check: Analyze (python)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
Replace JSON Schema $ref with YAML anchors to achieve DRY schema while avoiding Pydantic BaseModel generation that caused MyPy type errors. - Add YAML anchor definition (x-parameters-definition) at top of schema - Replace 71 $ref occurrences with anchor merges (<<: *parameters-definition) - Generated models now use Dict[str, Any] instead of ParametersProp - All 84 MyPy errors resolved with zero code changes to model_to_component_factory.py This approach preserves DRY in the source YAML while generating type-compatible Python models that work seamlessly with existing code. Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
1811-1813: Incomplete migration: DefaultErrorHandler$parametersnot converted to anchor pattern; typo present.The
$parametersblock here remains inline and has the same typo as DeclarativeStream:additional_properties(line 1813) should beadditionalProperties.Apply this diff to fix:
$parameters: - type: object - additional_properties: true + <<: *parameters-definition
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(72 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(67 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk 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.
📚 Learning: 2024-11-18T23:40:06.391Z
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk 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.
Applied to files:
airbyte_cdk/sources/declarative/declarative_component_schema.yamlairbyte_cdk/sources/declarative/models/declarative_component_schema.py
📚 Learning: 2024-12-11T16:34:46.319Z
Learnt from: pnilan
Repo: airbytehq/airbyte-python-cdk PR: 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.
Applied to files:
airbyte_cdk/sources/declarative/declarative_component_schema.yamlairbyte_cdk/sources/declarative/models/declarative_component_schema.py
📚 Learning: 2024-11-15T01:04:21.272Z
Learnt from: aaronsteers
Repo: airbytehq/airbyte-python-cdk 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.
Applied to files:
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
airbyte_cdk/sources/declarative/extractors/response_to_file_extractor.py (1)
ResponseToFileExtractor(23-176)airbyte_cdk/sources/declarative/auth/declarative_authenticator.py (1)
NoAuth(33-42)airbyte_cdk/sources/declarative/resolvers/config_components_resolver.py (2)
ConfigComponentsResolver(46-211)StreamConfig(29-41)airbyte_cdk/sources/declarative/resolvers/components_resolver.py (1)
ComponentMappingDefinition(17-27)
⏰ 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). (15)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-intercom
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: SDM Docker Image Build
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: preview_docs
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (3)
7-11: Centralized$parametersdefinition successfully introduced.The new YAML anchor pattern is clean and reusable. Good use of
<<: *parameters-definitionthroughout the file to DRY out parameter metadata.
4709-4712: CustomConfigTransformation$parameterswas not converted to anchor pattern; custom description remains inline.Unlike the majority of converted components, the
$parametersblock here retains an inline definition with a custom description:"Additional parameters to be passed to the custom config transformation."Wdyt—should this custom description be preserved (in which case it's correct to stay inline), or should it be standardized to use the centralized anchor and drop the component-specific description for consistency?
If you'd like to keep the custom description, this is fine as-is. If standardization is preferred, apply:
$parameters: - type: object - description: Additional parameters to be passed to the custom config transformation. - additionalProperties: true + <<: *parameters-definition
7-4712: I found several components with inline$parametersdefinitions that weren't converted to use the anchor merge. Let me dig deeper to understand if this is intentional:Now let me count the actual number of occurrences to verify the claim:Based on my analysis of the script output and the provided code, I can now provide the rewritten review comment:Several
$parametersdefinitions still use inline expansion instead of anchor merge.The script found 8+ locations where
$parametersis defined inline withtype: objectandadditional_propertiesrather than using<<: *parameters-definition. Components like DeclarativeStream, DefaultErrorHandler, HTTPAPIBudget, and DefaultPaginator appear to be candidates for conversion—they don't include additional properties beyond what the anchor provides.However, CustomConfigTransformation includes a custom
descriptionfield, which would justify its inline definition since the anchor can't accommodate component-specific metadata. Wdyt—should these inline definitions without custom properties be converted to use the anchor pattern for consistency, or were they intentionally left inline?airbyte_cdk/sources/declarative/models/declarative_component_schema.py (2)
1-3: Auto-generated file from YAML schemaBased on learnings, this file is auto-generated from
declarative_component_schema.yaml, so any necessary changes should typically be made to the source YAML and regenerated rather than edited directly here. I'll focus on consistency checks rather than detailed code review.Based on learnings
2251-2251: Inconsistency in$parametersfield metadata confirmedVerification confirms that both
DefaultErrorHandler(line 2251) andDeclarativeStream(line 2826) are missing thedescriptionandtitlemetadata that all other$parametersfield additions throughout this file include. Every other class follows the pattern of a multi-line Field definition with descriptive metadata, while only these two use the bare single-line format.Was this intentional, or should both be updated to match the established convention? Wdyt?
Convert all 71 $parameters anchor merges from two-liner to one-liner format:
- Before: $parameters:\n <<: *parameters-definition
- After: $parameters: { <<: *parameters-definition }
This improves readability while maintaining identical YAML semantics and
generated Python code. MyPy verification confirms no behavioral changes.
Co-Authored-By: AJ Steers <aj@airbyte.io>
schema: centralize $parameters via YAML anchor; add title/description
Summary
Centralizes the
$parametersschema definition using a single YAML anchor and merges it into all 71 component definitions. This eliminates duplicated schema blocks and adds consistent title/description metadata for$parametersacross the schema. No runtime behavior changes.Changes vs main
x-parameters-definition: ¶meters-definitionwith title "$parameters", description (explaining parameter propagation),type: object, andadditionalProperties: true$parameters: { <<: *parameters-definition }poe build. Theparametersfield remainsOptional[Dict[str, Any]](alias "$parameters") but now includes consistent title/description metadata in Field()Impact
$parametersdefinition (easier maintenance)Optional[Dict[str, Any]], existing configs continue to workReview & Testing Checklist for Human
poe buildlocally - Verify generated models match the PR (confirms YAML anchor resolution works correctly)parametersfields areOptional[Dict[str, Any]], NOT a BaseModel class$parameters(e.g., Stripe) and verify it still worksNotes
Dict[str, Any](not a BaseModel)poe buildcompleted successfully, MyPy passes onmodel_to_component_factory.py