-
Notifications
You must be signed in to change notification settings - Fork 4.8k
[DSIP-83][datasource-plugin] Add DolphinDB datasource #16962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md) |
Related DSIP #16818 |
|
||
String[] hostSeperator = connectionParams.getAddress().split(Constants.DOUBLE_SLASH); | ||
String[] hostPortArray = hostSeperator[hostSeperator.length - 1].split(Constants.COMMA); | ||
dataSourceParamDTO.setPort(Integer.parseInt(hostPortArray[0].split(Constants.COLON)[1])); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add dolphinscheduler-datasource-dolphindb
to https://github.com/apache/dolphinscheduler/blob/dev/config/plugins_config
34168bb
to
28e1bab
Compare
Please fix CI errors. @eco8848 |
faa1bf6
to
94799bf
Compare
I have referred to the official license requirements at https://dolphinscheduler.apache.org/zh-cn/docs/3.2.2/contribute/join/DS-License. It is necessary to add a description of the newly introduced third-party library in |
The CI errors have already been fixed. @SbloodyS |
I saw that there is a new check requires a milestone label. So is there a general plan for the integration of this PR? @SbloodyS |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces a new DolphinDB datasource plugin to support DolphinDB connections in the system. Key changes include:
- Implementation of the DolphinDB datasource module with connection parameters, processors, and client/channel classes.
- Addition of comprehensive test cases for various components of the datasource.
- Updates to documentation and dependency management for the new JDBC driver.
Reviewed Changes
Copilot reviewed 23 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
dolphinscheduler-datasource-dolphindb/src/test/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/utils/DataSourceUtilsTest.java | Added tests for datasource utilities. |
dolphinscheduler-datasource-dolphindb/src/test/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/param/DolphinDBDataSourceProcessorTest.java | Added tests for datasource processor functionality. |
dolphinscheduler-datasource-dolphindb/src/test/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBDataSourceChannelTest.java | Added tests for datasource channel client creation. |
dolphinscheduler-datasource-dolphindb/src/test/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBDataSourceChannelFactoryTest.java | Added tests for datasource channel factory. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/param/DolphinDBDataSourceProcessor.java | Implements the datasource processor for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/param/DolphinDBDataSourceParamDTO.java | Defines the parameter DTO for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/param/DolphinDBConnectionParam.java | Implements connection parameters for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBPooledDataSourceClient.java | Defines the pooled datasource client for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBDataSourceChannelFactory.java | Implements the datasource channel factory for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBDataSourceChannel.java | Defines the datasource channel for DolphinDB. |
dolphinscheduler-datasource-dolphindb/src/main/java/org/apache/dolphinscheduler/plugin/datasource/dolphindb/DolphinDBAdHocDataSourceClient.java | Defines the ad hoc datasource client for DolphinDB. |
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/constants/DataSourceConstants.java | Adds constants for the DolphinDB driver, JDBC URL prefix, and validation query. |
docs/docs/zh/guide/datasource/dolphindb.md & docs/docs/en/guide/datasource/dolphindb.md | Added documentation for the new DolphinDB datasource. |
docs/configs/docsdev.js | Updated navigation configuration to include DolphinDB documentation. |
Files not reviewed (5)
- config/plugins_config: Language not supported
- dolphinscheduler-bom/pom.xml: Language not supported
- dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-all/pom.xml: Language not supported
- dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-dolphindb/pom.xml: Language not supported
- dolphinscheduler-datasource-plugin/pom.xml: Language not supported
if (!baseDataSourceParamDTO.getType().equals(DbType.REDSHIFT)) { | ||
// due to redshift use not regular hosts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition incorrectly checks for DbType.REDSHIFT instead of DbType.DOLPHINDB, which may lead to incorrect host validation. Adjust the condition to compare against DbType.DOLPHINDB.
if (!baseDataSourceParamDTO.getType().equals(DbType.REDSHIFT)) { | |
// due to redshift use not regular hosts | |
if (baseDataSourceParamDTO.getType().equals(DbType.DOLPHINDB)) { | |
// validate host for DolphinDB |
Copilot uses AI. Check for mistakes.
Purpose of the pull request
close #16818
Brief change log
Verify this pull request
This change added tests and can be verified as follows: