feat: enable custom validation strategies #610
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for custom validation strategies in the declarative component framework. Key changes include:
- Importing and mapping the new CustomValidationStrategyModel in model_to_component_factory.py.
- Adding a new CustomValidationStrategy Pydantic model in declarative_component_schema.py.
- Updating YAML definitions and reordering some union type members in various component models.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py | Added import and mapping support for the new CustomValidationStrategyModel. |
| airbyte_cdk/sources/declarative/models/declarative_component_schema.py | Introduced the CustomValidationStrategy model and adjusted field types/unions in related models. |
| airbyte_cdk/sources/declarative/declarative_component_schema.yaml | Updated schema examples and definitions to include CustomValidationStrategy. |
Comments suppressed due to low confidence (1)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py:1599
- The 'class_name' field description mentions 'custom decoding' which may not align with the intended purpose of custom validation. Consider updating the description to reference 'custom validation logic' or a similar phrase to avoid confusion.
)
📝 WalkthroughWalkthroughThe updates introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeclarativeSchema
participant ModelFactory
participant CustomValidationClass
User->>DeclarativeSchema: Defines DpathValidator with CustomValidationStrategy
DeclarativeSchema->>ModelFactory: Passes CustomValidationStrategyModel
ModelFactory->>CustomValidationClass: Instantiates custom validation via class_name
CustomValidationClass-->>ModelFactory: Returns custom validator instance
ModelFactory-->>DeclarativeSchema: Returns component with custom validation
Suggested labels
Suggested reviewers
Would you like to add an example usage of the new 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Pylint (3.3.7)airbyte_cdk/sources/declarative/models/declarative_component_schema.py[refactor] 1588-1588: Too few public methods (0/2) (R0903) [refactor] 1587-1587: Too few public methods (0/2) (R0903) ⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (4)
✨ 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: 2
🧹 Nitpick comments (3)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (2)
225-227: Import block – alphabetical placement looks off
Nit: In this import cluster the entries are kept in alphabetical order.CustomValidationStrategyshould come beforeCustomTransformation(V vs T). Would you like to move the new line up one spot to keep the ordering tidy, wdyt?-from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( - CustomTransformation as CustomTransformationModel, -) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CustomTransformation as CustomTransformationModel, +) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CustomValidationStrategy as CustomValidationStrategyModel, +)
689-689: Mapping entry added – consider doc-string update
Great to seeCustomValidationStrategyModelwired in! Could we also update the class-level doc-string (or module docs) mentioning that custom validation strategies are now supported, so future readers spot the feature quickly, wdyt?airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
4207-4213: Example block is valid JSON but departs from the predominant YAML style – worth harmonising?All other example snippets in the file are expressed in YAML. Sticking to one style keeps the docs easier to scan and avoids surprising users who copy-paste. Would you be open to re-writing this example as YAML (e.g. using the
- name:notation), wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(3 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(14 hunks)airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py(2 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
[refactor] 1590-1590: Too few public methods (0/2)
(R0903)
[refactor] 1589-1589: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-amplitude
- GitHub Check: Check: source-intercom
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
🔇 Additional comments (4)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (1)
4360-4365: Addition ofCustomValidationStrategyto the union looks goodThe union now correctly accepts both built-in and custom strategies; no issues noticed here.
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (3)
2046-2050: Nice enhancement to support custom validation strategies!The union type update for
validation_strategycorrectly enables support for both the existingValidateAdheresToSchemaand the newCustomValidationStrategy. This change aligns perfectly with the PR objectives.
1927-1927: Great consistency improvements across union types!The reordering of union types throughout the file standardizes the placement of custom components and improves overall consistency. These changes enhance maintainability without affecting functionality.
Also applies to: 2305-2306, 2398-2398, 2415-2415, 2667-2667, 2647-2647, 2911-2911, 2927-2927, 2951-2951, 2967-2967
1783-1821: Excellent improvements to field organization and clarity!The field title updates in
AsyncRetriever("Decoder" → "HTTP Response Format") are much clearer and more descriptive. TheDatetimeBasedCursorenhancements with the newcursor_datetime_formatsfield and improved field ordering enhance both functionality and usability.Also applies to: 2911-2911, 2927-2927
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/models/declarative_component_schema.py
Outdated
Show resolved
Hide resolved
pnilan
deleted the
pnilan/validations/enable-custom-validation-strategies
branch
What
Note
declarative_component_schema.pyfrom a previous PR: e44362aSummary by CodeRabbit
New Features
Improvements