chore: more reordering and title description updates in declarative_component_schema #570
Conversation
lmossman
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR reorganizes schema definitions for cleaner UI presentation, deprecates the JsonFileSchemaLoader, and adds new datetime-related fields to support flexible cursor and time options.
- Reordered
anyOfentries and updated component titles for improved clarity in the Builder UI - Introduced
cursor_datetime_formats,start_datetime,end_datetime, and related time-injection options - Marked
JsonFileSchemaLoaderas deprecated in favor ofInlineSchemaLoader
Comments suppressed due to low confidence (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml:4077
- [nitpick] The title uses “Http” but elsewhere the acronym is uppercased as “HTTP” (e.g., “HTTP Response Format”). Consider renaming this to “HTTP Components Resolver” for consistency.
title: Http Components Resolver
airbyte_cdk/sources/declarative/declarative_component_schema.yaml:910
- New schema properties for cursor and start/end datetime have been added here and below. We should add or update unit/integration tests to validate these fields' presence, formats, and anyOf branches in the schema loader.
cursor_datetime_formats:
📝 WalkthroughWalkthroughThe update reorganizes and enhances the declarative component schema YAML. It consolidates and expands properties for datetime cursors, introduces deprecation messaging for a schema loader, adjusts the order of union types in several definitions, allows arrays for schema loaders, and updates titles for certain components. No core validation logic is changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeclarativeStream
participant DatetimeBasedCursor
participant SchemaLoader
User->>DeclarativeStream: Configure stream with retriever, schema_loader(s), cursor
DeclarativeStream->>DatetimeBasedCursor: Use new datetime properties (formats, start/end, options)
DeclarativeStream->>SchemaLoader: Accept array or single loader (including deprecated types)
Possibly related PRs
Suggested labels
Suggested reviewers
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 (4)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (4)
1434-1442: DeclarativeStream: support multiple schema loaders
Adding an array option forschema_loaderenables merging schemas from multiple sources—should we update our spec docs or add an example snippet to illustrate merge precedence? wdyt?
2365-2366: JsonFileSchemaLoader: mark as deprecated
The deprecation flag and message are clear—can we confirm that the Builder UI hidesJsonFileSchemaLoaderand surfaces migration warnings for existing connectors? wdyt?
4077-4089: HttpComponentsResolver: add title & reorder retriever
Addingtitle: Http Components Resolverand promotingSimpleRetrieveraligns with other resolvers—should we update our reference docs to reflect this new component title? wdyt?
4135-4136: ConfigComponentsResolver: add title
Nice addition oftitle: Config Components Resolver—would it help to include an example of using multipleStreamConfigentries? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- 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 (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 (9)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (9)
303-304: SelectiveAuthenticator: reorder authenticators list
We movedCustomAuthenticatorbeforeNoAuthto surface custom code options earlier in the UI—did we confirm the Builder enumeration respects this new order? wdyt?
910-953: DatetimeBasedCursor: added flexible datetime properties
Great consolidation of cursor options withcursor_datetime_formats,start_datetime,start_time_option,end_datetime, andend_time_option. Have we ensured the required fields list still matches expectations (e.g., onlystart_datetimeremains mandatory)? wdyt?
1458-1458: DeclarativeStream: move CustomTransformation last
Custom transformations now appear at the end of the list for better UX—looks good. Have we validated the UI rendering order aligns here? wdyt?
2321-2323: DynamicSchemaLoader: prioritize SimpleRetriever first
Reordering putsSimpleRetrieverat the top—this mirrors the DeclarativeStream change. Can we double-check the underlying generator logic uses this new order? wdyt?
2343-2343: DynamicSchemaLoader: move CustomTransformation last
Custom transformations landing last improves discoverability of built-ins—looks consistent with other lists.👍
1694-1698: DefaultErrorHandler: defer CustomBackoffStrategy to the end
Custom backoff now appears last inbackoff_strategies—nice for surfacing defaults first. All good here? wdyt?
3966-3969: GroupingPartitionRouter: move CustomPartitionRouter last
Ensures built-in routers appear first, with custom hooks available at the end—makes sense. Have we added a migration note for connectors if they relied on the previous index order? wdyt?
3769-3772: AsyncRetriever: rename decoder title
ChangingdecodertoHTTP Response Formatimproves clarity—have we verified this label in the UI matches other retriever components? wdyt?
3781-3783: AsyncRetriever: rename download_decoder title
Updatingdownload_decodertoDownload HTTP Response Formatis consistent—can we confirm the UI field tooltip updated accordingly? wdyt?
lmossman
changed the title
lmossman
deleted the
lmossman/more-reordering-and-title-description-updates
branch
This PR does some more reordering of fields and anyOf options, and updates to titles of components to make things clearer in the Builder UI.
It also marks
JsonFileSchemaLoaderas deprecated since I don't believe we want users to see that in the Builder UI. But let me know if you disagree with this decision, as it does mean that any connectors which use that today (seems to be ~9 in the airbyte repo) will have a deprecation message until they are switched to use InlineSchemaLoader.Summary by CodeRabbit
New Features
Improvements
Other Changes