feat(wren-ui): Support Redshift connector#1761
Conversation
WalkthroughThis change introduces comprehensive support for Amazon Redshift as a new data source throughout the codebase. It adds new enums, types, interfaces, and validation logic for Redshift connection methods (password and IAM), updates GraphQL schema and server logic, extends utility functions, and implements a new React component for Redshift configuration in the UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (RedshiftProperties)
participant Server
participant IbisAdaptor
participant Redshift
User->>UI (RedshiftProperties): Selects Redshift & fills form (Password/IAM)
UI (RedshiftProperties)->>Server: Submits Redshift connection info
Server->>IbisAdaptor: Converts to IbisRedshiftConnectionInfo
IbisAdaptor->>Redshift: Attempts connection using provided credentials
Redshift-->>IbisAdaptor: Returns connection result
IbisAdaptor-->>Server: Passes result
Server-->>UI (RedshiftProperties): Returns status to user
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wren-ui/src/components/pages/setup/dataSources/RedshiftProperties.tsx (1)
11-84: Consider extracting validation rules to improve maintainabilityThe password fields component is well-structured, but the validation rules are inline. Consider extracting them to a constants file for better reusability and maintenance.
+// Extract to a validation constants file +const REDSHIFT_VALIDATION_RULES = { + HOST: [{ required: true, validator: hostValidator }], + PORT: [{ required: true, message: ERROR_TEXTS.CONNECTION.PORT.REQUIRED }], + // ... other rules +}; <Form.Item label="Host" name="host" required - rules={[ - { - required: true, - validator: hostValidator, - }, - ]} + rules={REDSHIFT_VALIDATION_RULES.HOST} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
wren-ui/public/images/dataSource/redshift.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
wren-ui/src/apollo/client/graphql/__types__.ts(2 hunks)wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts(3 hunks)wren-ui/src/apollo/server/dataSource.ts(3 hunks)wren-ui/src/apollo/server/mdl/mdlBuilder.ts(1 hunks)wren-ui/src/apollo/server/mdl/type.ts(1 hunks)wren-ui/src/apollo/server/repositories/projectRepository.ts(3 hunks)wren-ui/src/apollo/server/schema.ts(1 hunks)wren-ui/src/apollo/server/types/dataSource.ts(1 hunks)wren-ui/src/components/pages/setup/dataSources/RedshiftProperties.tsx(1 hunks)wren-ui/src/components/pages/setup/utils.tsx(1 hunks)wren-ui/src/hooks/useSetupConnectionDataSource.tsx(2 hunks)wren-ui/src/utils/dataSourceType.ts(4 hunks)wren-ui/src/utils/enum/dataSources.ts(2 hunks)wren-ui/src/utils/error/dictionary.ts(1 hunks)
🔇 Additional comments (33)
wren-ui/src/apollo/server/types/dataSource.ts (1)
12-12: LGTM! Enum addition follows established pattern.The addition of
REDSHIFT = 'REDSHIFT'to theDataSourceNameenum is consistent with the existing naming convention and maintains alphabetical ordering.wren-ui/src/apollo/server/mdl/type.ts (1)
98-98: LGTM! Engine data source type addition is consistent.The addition of
REDSHIFT = 'REDSHIFT'to theWrenEngineDataSourceTypeenum follows the established pattern and maintains proper alphabetical ordering.wren-ui/src/apollo/server/mdl/mdlBuilder.ts (1)
515-516: LGTM! Required mapping for Redshift data source.The addition of the
DataSourceName.REDSHIFTcase that returnsWrenEngineDataSourceType.REDSHIFTfollows the established pattern and completes the necessary mapping for Redshift support in the MDL builder.wren-ui/src/components/pages/setup/utils.tsx (1)
110-114: LGTM! Data source configuration follows established pattern.The Redshift configuration entry properly follows the same pattern as other data sources, including the spread of
getDataSourceConfig()results, appropriate guide URL, and enabling the data source withdisabled: false.Please verify that the documentation URL is accessible:
#!/bin/bash # Description: Verify the Redshift documentation URL is accessible # Expected: HTTP 200 response curl -I -s "https://docs.getwren.ai/oss/guide/connect/redshift" | head -1wren-ui/src/apollo/server/schema.ts (2)
58-58: LGTM! DataSourceName enum extension is consistent.The addition of
REDSHIFTto theDataSourceNameenum follows the established GraphQL schema pattern and maintains consistency with other data source entries.
61-64: LGTM! New connection type enum supports authentication methods.The
RedshiftConnectionTypeenum properly defines the two authentication methods (redshiftfor username/password andredshift_iamfor AWS credentials) as outlined in the PR objectives. The naming convention follows GraphQL best practices with lowercase values.wren-ui/src/utils/error/dictionary.ts (1)
64-66: LGTM! Validation message follows established pattern.The new error message for cluster identifier validation is consistent with other connection validation messages and will properly support the Redshift IAM authentication method.
wren-ui/src/utils/enum/dataSources.ts (2)
1-1: LGTM! Semantic alias improves code clarity.Re-exporting
RedshiftConnectionTypeasREDSHIFT_AUTH_METHODprovides better semantic meaning in the UI context.
14-14: LGTM! Enum addition follows established pattern.Adding
REDSHIFT = 'REDSHIFT'to theDATA_SOURCESenum is consistent with other data source entries.wren-ui/src/apollo/client/graphql/__types__.ts (2)
324-324: LGTM! Generated enum addition is correct.Adding
REDSHIFT = 'REDSHIFT'to theDataSourceNameenum properly extends GraphQL type support for Redshift.
1226-1229: LGTM! Authentication method enum is well-defined.The
RedshiftConnectionTypeenum correctly defines the two authentication methods for Redshift: password-based (redshift) and IAM-based (redshift_iam).wren-ui/src/utils/dataSourceType.ts (4)
12-12: LGTM! Import follows established pattern.Importing
RedshiftPropertiescomponent is consistent with other data source property imports.
36-37: LGTM! Image path follows naming convention.The Redshift image path
/images/dataSource/redshift.svgis consistent with other data source image paths.
65-66: LGTM! Display name is clear and consistent.Returning
'Redshift'as the display name follows the simple naming pattern used by other data sources.
94-95: LGTM! Properties component integration is correct.Returning the
RedshiftPropertiescomponent follows the established pattern for data source configuration components.wren-ui/src/hooks/useSetupConnectionDataSource.tsx (2)
3-3: LGTM! Import supports Redshift authentication handling.Adding
REDSHIFT_AUTH_METHODimport enables proper authentication method detection in the form transformation logic.
105-113: LGTM! Redshift form transformation logic is well-implemented.The conditional logic correctly handles password placeholder only for the password authentication method (
REDSHIFT_AUTH_METHOD.redshift), while leaving IAM authentication properties unchanged. This aligns with the different field requirements of each authentication method.wren-ui/src/apollo/server/dataSource.ts (3)
9-11: LGTM: Clean import structureThe imports for Redshift types are properly organized and follow the existing pattern in the file.
24-27: LGTM: Consistent import patternThe repository type imports follow the established naming convention and are appropriately grouped.
342-397: Verify the field mapping consistency for IAM authenticationThe implementation correctly handles both authentication types, but there's a potential inconsistency in field naming between the repository types and Ibis types.
#!/bin/bash # Description: Verify field name consistency between repository and adaptor types # Expected: Field names should be consistent or properly mapped echo "Checking REDSHIFT_IAM_AUTH fields in repository types..." ast-grep --pattern 'interface REDSHIFT_IAM_AUTH { $$$ }' echo -e "\nChecking IbisRedshiftIAMAuth fields in adaptor types..." ast-grep --pattern 'interface IbisRedshiftIAMAuth { $$$ }' echo -e "\nChecking field mapping in toIbisConnectionInfo method..." rg -A 20 "redshiftType === IbisRedshiftConnectionType.REDSHIFT_IAM"wren-ui/src/components/pages/setup/dataSources/RedshiftProperties.tsx (3)
1-6: LGTM: Appropriate imports and dependenciesThe component imports the necessary dependencies for form handling, validation, and Redshift-specific constants.
86-170: LGTM: Well-structured IAM fields componentThe IAM authentication fields are properly organized with appropriate validation rules and helpful extras like the username description. The field disabling logic in edit mode is correctly implemented.
196-201: LGTM: Smart edit mode logicThe
getIsEditModeForComponentfunction correctly handles the edge case where the authentication method might change and ensures proper field disabling behavior.wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts (5)
77-80: LGTM: Well-defined enum for Redshift connection typesThe enum values are descriptive and follow a consistent naming pattern that clearly distinguishes between password and IAM authentication methods.
82-89: LGTM: Complete password authentication interfaceThe interface includes all necessary fields for password-based Redshift connections with appropriate types.
91-99: LGTM: Comprehensive IAM authentication interfaceThe IAM interface correctly includes AWS-specific fields like cluster_identifier, region, and access keys, which are essential for IAM-based Redshift connections.
101-104: LGTM: Proper union type definitionThe union type correctly represents either authentication method, providing type safety while maintaining flexibility.
115-115: LGTM: Consistent integration with existing patternsThe addition to the SupportedDataSource enum and dataSourceUrlMap follows the established pattern and maintains consistency with other data sources.
Also applies to: 128-128
wren-ui/src/apollo/server/repositories/projectRepository.ts (5)
12-12: LGTM: Proper import of shared typesThe import of
IbisRedshiftConnectionTypeensures type consistency between the repository and adaptor layers.
96-103: LGTM: Complete password authentication interfaceThe interface includes all required fields for password-based authentication with proper typing.
105-113: LGTM: Comprehensive IAM authentication interfaceThe IAM interface correctly includes all AWS-specific fields needed for IAM-based Redshift connections.
115-117: LGTM: Clean union type definitionThe union type properly represents both authentication methods with clear naming.
129-130: LGTM: Proper integration with existing typesThe addition of
REDSHIFT_CONNECTION_INFOto the union type maintains backward compatibility while extending support for the new data source.
| useEffect(() => { | ||
| if (isEditMode) { | ||
| if (redshiftType && initialRedshiftTypeRef.current === null) { | ||
| initialRedshiftTypeRef.current = redshiftType; | ||
| } | ||
| } else { | ||
| form.setFieldsValue({ | ||
| redshiftType: REDSHIFT_AUTH_METHOD.redshift, | ||
| }); | ||
| } | ||
| }, [isEditMode, form, redshiftType]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with useEffect dependency and side effects
The useEffect has a potential issue where the form.setFieldsValue call could trigger unnecessary re-renders or conflicts with user input.
useEffect(() => {
if (isEditMode) {
if (redshiftType && initialRedshiftTypeRef.current === null) {
initialRedshiftTypeRef.current = redshiftType;
}
} else {
- form.setFieldsValue({
- redshiftType: REDSHIFT_AUTH_METHOD.redshift,
- });
+ if (!redshiftType) {
+ form.setFieldsValue({
+ redshiftType: REDSHIFT_AUTH_METHOD.redshift,
+ });
+ }
}
}, [isEditMode, form, redshiftType]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (isEditMode) { | |
| if (redshiftType && initialRedshiftTypeRef.current === null) { | |
| initialRedshiftTypeRef.current = redshiftType; | |
| } | |
| } else { | |
| form.setFieldsValue({ | |
| redshiftType: REDSHIFT_AUTH_METHOD.redshift, | |
| }); | |
| } | |
| }, [isEditMode, form, redshiftType]); | |
| useEffect(() => { | |
| if (isEditMode) { | |
| if (redshiftType && initialRedshiftTypeRef.current === null) { | |
| initialRedshiftTypeRef.current = redshiftType; | |
| } | |
| } else { | |
| if (!redshiftType) { | |
| form.setFieldsValue({ | |
| redshiftType: REDSHIFT_AUTH_METHOD.redshift, | |
| }); | |
| } | |
| } | |
| }, [isEditMode, form, redshiftType]); |
🤖 Prompt for AI Agents
In wren-ui/src/components/pages/setup/dataSources/RedshiftProperties.tsx around
lines 184 to 194, the useEffect hook calls form.setFieldsValue unconditionally
when not in edit mode, which may cause unnecessary re-renders or interfere with
user input. To fix this, add a condition to only call form.setFieldsValue if the
current form value for redshiftType is different from
REDSHIFT_AUTH_METHOD.redshift, preventing redundant updates and side effects.
This pull request introduces support for the Redshift data source.
Description
Support Redshift connector
HostandDatabasefields cannot be edited.Cluster identifierandDatabaseandAWS regionfields cannot be edited.Redshift Data Source:
Logo:
Engine API information
Development version: 0.16.4
Ref UI
Username and password authentication method
AWS credentials authentication method
Summary by CodeRabbit
New Features
Bug Fixes
Style