fix: (Low-Code) (DeclarativeOAuthFlow) - fix tooltip examples and labels representation to show values instead of full object
#172
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces substantial modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Wdyt about these suggestions? Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (6)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (3)
806-806: Great simplification of the scope example!The space-separated scope format is now much clearer. Would you consider adding an example with a more complex scope string that includes special characters that might need URL encoding, wdyt?
- examples=["user:read user:read_orders workspaces:read"], + examples=[ + "user:read user:read_orders workspaces:read", + "https://api.example.com/scope1 https://api.example.com/scope2" + ],
820-820: Good example of base64 encoded authorization header!The example shows how to properly format Basic Auth headers. Have you considered adding an example with a different authorization scheme as well, for completeness?
- examples=[{"Authorization": "Basic {base64Encoder:{client_id}:{client_secret}}"}], + examples=[ + {"Authorization": "Basic {base64Encoder:{client_id}:{client_secret}}"}, + {"X-Custom-Auth": "Token {client_id}"} + ],
826-826: Simple and clear access_token_params example!The example demonstrates parameter interpolation well. Would it be helpful to add an example with multiple parameters to show how they can be combined?
- examples=[{"{client_id_key}": "{{client_id_key}}"}], + examples=[ + {"{client_id_key}": "{{client_id_key}}"}, + { + "{client_id_key}": "{{client_id_key}}", + "{grant_type}": "authorization_code" + } + ],airbyte_cdk/sources/declarative/declarative_component_schema.yaml (3)
2139-2140: The consent URL examples look good but could be more descriptive, wdyt?The examples are valid but could be more self-documenting. Consider adding comments to explain what each parameter represents:
- https://domain.host.com/marketing_api/auth?{client_id_key}={{client_id_key}}&{redirect_uri_key}={urlEncoder:{{redirect_uri_key}}}&{state_key}={{state_key}} + # Basic auth URL with client ID, redirect URI, and state + https://domain.host.com/marketing_api/auth?{client_id_key}={{client_id_key}}&{redirect_uri_key}={urlEncoder:{{redirect_uri_key}}}&{state_key}={{state_key}}
2155-2155: The access token URL example is good but could be more comprehensive, what do you think?Consider adding an example that shows different authentication methods:
- https://auth.host.com/oauth2/token?{client_id_key}={{client_id_key}}&{client_secret_key}={{client_secret_key}}&{auth_code_key}={{auth_code_key}}&{redirect_uri_key}={urlEncoder:{{redirect_uri_key}}} + # Client credentials flow + https://auth.host.com/oauth2/token?{client_id_key}={{client_id_key}}&{client_secret_key}={{client_secret_key}}&grant_type=client_credentials + # Authorization code flow + https://auth.host.com/oauth2/token?{client_id_key}={{client_id_key}}&{client_secret_key}={{client_secret_key}}&{auth_code_key}={{auth_code_key}}&{redirect_uri_key}={urlEncoder:{{redirect_uri_key}}}
2198-2240: The key override examples are consistent and clear.The examples follow a good pattern of showing custom key names. However, consider adding a note about common provider-specific variations:
+ # Examples of provider-specific key names: + # - client_id: app_id, consumer_key, api_key + # - client_secret: app_secret, consumer_secret, api_secret + # - auth_code: code, authorization_code + # - redirect_uri: callback_url, return_url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(4 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(1 hunks)
🔇 Additional comments (6)
airbyte_cdk/sources/declarative/models/declarative_component_schema.py (4)
798-799: Nice improvement to the consent_url examples!
The examples now clearly show how to handle both simple and complex OAuth consent URLs with proper placeholder usage. This makes it much easier for developers to understand the templating system.
813-813: Clear and practical access_token_url example!
The example effectively demonstrates the URL template with all necessary OAuth parameters. The format is consistent with common OAuth implementations.
832-832: Comprehensive extract_output example!
The example covers the most common OAuth fields. This will help users understand which fields they typically need to extract from the OAuth response.
838-874: Clear examples for OAuth parameter customization!
The examples for state configuration and key overrides are clear and follow a consistent pattern. Each example demonstrates how to customize the OAuth flow for different provider requirements.
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
2147-2147: LGTM! Clear and concise scope example.
The space-separated list of scopes is a common format used by OAuth providers.
Line range hint 2163-2181: The examples for headers, params, and output look good!
The examples are clear and show common use cases:
- Basic auth header using base64 encoding
- Client credentials in params
- Common token fields in extract_output
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (3)
2142-2147: Consider adding more scope combination examples?The scope example is clear, but would it be helpful to add one more example showing how to combine multiple scopes with a different delimiter (e.g., comma or plus sign)? Some OAuth providers use different delimiters, wdyt?
examples: - user:read user:read_orders workspaces:read + - user.read,user.write,admin:read + - analytics+read+write
2157-2163: Nice example of Base64 encoding in headers!The example demonstrates a common pattern for Basic Auth. Would it be worth adding one more example showing a custom header format that some APIs use, wdyt?
examples: - {"Authorization": "Basic {base64Encoder:{client_id}:{client_secret}}"} + - {"X-Custom-Auth": "Key {client_id}, Secret {client_secret}"}
2165-2172: The access token params example could be more comprehensiveWhile the current example is clear, would it be helpful to show how to handle more complex parameter structures that some OAuth providers require? For instance, showing nested parameters or arrays?
examples: - {"{client_id_key}": "{{client_id_key}}"} + - { + "{client_id_key}": "{{client_id_key}}", + "grant_type": "authorization_code", + "scope[]": ["read", "write"] + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(2 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte_cdk/sources/declarative/models/declarative_component_schema.py
🔇 Additional comments (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
2133-2140: The consent URL examples look clearer now!
The simplified examples make it easier to understand the expected format. I particularly like how you've shown both simple and complex URL patterns with different parameter combinations. This aligns well with the PR objective of making tooltips more user-friendly.
2149-2155: The access token URL examples are much clearer now!
The example shows a good pattern with multiple parameters. This will help users understand how to construct complex URLs with proper parameter encoding.
bazarnov
changed the title
tooltip examples and labels representation to show values instead of full object
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
airbyte_cdk/sources/declarative/declarative_component_schema.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
2133-2140: The consent URL example looks good, but should we add more context?The example URL template is clear and shows the expected format. Would it be helpful to add a comment explaining what each placeholder represents? For example, what values would
client_id_keyandredirect_uri_keytypically contain? wdyt?
Line range hint
2187-2202: State configuration is well structuredThe state configuration with min/max values provides clear boundaries for state parameter generation. One suggestion: should we add validation to ensure min is always less than max?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml(2 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte_cdk/sources/declarative/models/declarative_component_schema.py
🔇 Additional comments (4)
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (4)
2142-2147: The scope example is much clearer now!
The example using space-separated scopes (user:read user:read_orders workspaces:read) is more intuitive than showing a complex object. This aligns well with OAuth2.0 standards.
2178-2185: Extract output example is more straightforward
The array format ["access_token", "refresh_token", "other_field"] clearly shows what fields to extract from the OAuth response.
2204-2244: Key override examples are consistent and clear
All the key override examples (client_id_key, client_secret_key, scope_key, state_key, auth_code_key, redirect_uri_key) follow a consistent pattern of showing a simple string value. This makes it easier for users to understand how to customize the OAuth parameter names.
2165-2176: Access token params example is simplified and clearer
The example now shows the essential parameters without unnecessary nesting, making it easier to understand. However, I notice that the description mentions JSON encoding - should we clarify if these parameters will be automatically JSON encoded by the framework?
What
Based on this review comment, there is a need to fix how tooltip are represented in order not to confuse Customers.
https://github.com/airbytehq/airbyte-platform-internal/pull/14785#discussion_r1881195226
How
declarative_component_schema.yamlUser Impact
No impact is expected. Not a breaking change.
Summary by CodeRabbit
New Features
ComponentMappingDefinition,HttpComponentsResolver, andStreamConfigfor enhanced data handling and configuration.Updates
OAuthConfigSpecificationwith clearer descriptions and simplified examples.DeclarativeStreamandDynamicDeclarativeStreamfor better clarity on their functionalities.Improvements
OauthConnectorInputSpecificationfor improved readability and consistency.