[Feature-9467] add DAMENG DataSource #12860

lenian · 2022-11-10T10:08:20Z

Purpose of the pull request

Brief change log

Verify this pull request

#9467

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

DarkAssassinator · 2022-11-10T15:35:45Z

.../src/main/java/org/apache/dolphinscheduler/plugin/datasource/dm/param/DmConnectionParam.java

+                + ", password='" + password + '\''
+                + ", address='" + address + '\''


IMHO, remove password in toString() should be better, this prevents the password from being printed to the log file. WDYT.

The toString() method of the OracleConnectionParam、MySQLConnectionParam class has the password attribute printed.I modified it according to the mysql and oracle specifications。Now I need to remove password in toString()?

The toString() method of the OracleConnectionParam、MySQLConnectionParam class has the password attribute printed.I modified it according to the mysql and oracle specifications。Now I need to remove password in toString()?

Please remove all password in those classes, also, consider use lombok ToString(excludes = to simplify the codes

ok, I'll remove password in toString()

kezhenxu94 · 2022-11-12T05:45:13Z

.../src/main/java/org/apache/dolphinscheduler/plugin/datasource/dm/param/DmConnectionParam.java

+                + ", password='" + password + '\''
+                + ", address='" + address + '\''


The toString() method of the OracleConnectionParam、MySQLConnectionParam class has the password attribute printed.I modified it according to the mysql and oracle specifications。Now I need to remove password in toString()?

Please remove all password in those classes, also, consider use lombok ToString(excludes = to simplify the codes

dolphinscheduler-bom/pom.xml

…2、DmConnectionParam remove password in toString()

…d in toString()

.../main/java/org/apache/dolphinscheduler/plugin/datasource/dm/param/DmDataSourceProcessor.java

…2、DmConnectionParam remove password in toString()

…d in toString()

…ve testGetDmConnection method

davidzollo · 2022-12-05T15:50:20Z

good job
Please use dameng instead of dm, nobody knows what dm means
Please add license file refer to this doc, https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/contribute/join/DS-License.html

lenian · 2022-12-06T03:32:31Z

good job Please use dameng instead of dm, nobody knows what dm means Please add license file refer to this doc, https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/contribute/join/DS-License.html
I will improve it according to your suggestion

1、modification package dm->dameng; 2、add Dameng DmJdbcDriver18 License

...va/org/apache/dolphinscheduler/plugin/datasource/dameng/param/DamengDataSourceProcessor.java

+        String address = connectionParams.getAddress();
+        String[] hostSeperator = address.split(Constants.DOUBLE_SLASH);
+        String[] hostPortArray = hostSeperator[hostSeperator.length - 1].split(Constants.COMMA);
+        damengDatasourceParamDTO.setPort(Integer.parseInt(hostPortArray[0].split(Constants.COLON)[1]));


# Conflicts: # dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-all/pom.xml # dolphinscheduler-datasource-plugin/pom.xml # dolphinscheduler-dist/release-docs/LICENSE # dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/enums/DbType.java # dolphinscheduler-ui/src/service/modules/data-source/types.ts # dolphinscheduler-ui/src/views/datasource/list/use-form.ts # dolphinscheduler-ui/src/views/projects/task/components/node/fields/use-datasource.ts # tools/dependencies/known-dependencies.txt

CalvinKirs · 2022-12-29T10:02:14Z

dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/pom.xml

+        <dependency>
+            <groupId>com.alibaba</groupId>
+            <artifactId>druid</artifactId>
+        </dependency>
    </dependencies>


Why are you introducing a new database connection pool? we already have HikariCP.

HikariCP does not support Dameng, druid supports Dameng

I don't understand, it's just a data source connector management, why doesn't it support dm, can you explain why?

thanks，I just tested the DaMeng database with HikariCP，HikariCP supports DaMeng

ruanwenjun · 2022-12-29T14:54:02Z

...va/org/apache/dolphinscheduler/plugin/datasource/dameng/param/DamengDataSourceProcessor.java

+        damengConnectionParam.setDriverClassName(getDatasourceDriver());
+        damengConnectionParam.setValidationQuery(getValidationQuery());
+        damengConnectionParam.setOther(transformOther(dmDatasourceParam.getOther()));
+        damengConnectionParam.setProps(dmDatasourceParam.getOther());


Please don't use setProps, I remember we don't have props in BaseDataSourceParamDTO, you need to rebase the upstream code.

Ok, I have rebase BaseDataSourceParamDTO

Tianqi-Dotes

LGTM, thanx for the efforts.

SbloodyS · 2023-01-04T03:27:34Z

dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/pom.xml

@@ -141,5 +141,6 @@
            <groupId>com.zaxxer</groupId>
            <artifactId>HikariCP</artifactId>
        </dependency>
+


Please remove unnessnary change.

Ok, I'll remove the unnecessary changes right away

docs/docs/zh/guide/datasource/dameng.md

SbloodyS

+1

sonarcloud · 2023-01-04T06:03:26Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
12 Code Smells

48.6% Coverage
0.0% Duplication

lenian added 2 commits November 9, 2022 15:10

[Feature-9467][DataSource] add DM（达梦） data source

e5357f1

[Feature-9467] add DM datasource doc (Issue apache#9467)

c2b365c

lenian requested review from kezhenxu94, zhongjiajie, Tianqi-Dotes, EricGao888, songjianet, Amy0104 and caishunfeng as code owners November 10, 2022 10:08

github-actions bot added backend document UI ui and front end related labels Nov 10, 2022

EricGao888 assigned lenian Nov 10, 2022

EricGao888 added this to the 3.2.0 milestone Nov 10, 2022

[Feature-9467] add DM IDataBaseOptionKeys

fe43cf3

lenian closed this Nov 10, 2022

lenian reopened this Nov 10, 2022

DarkAssassinator reviewed Nov 10, 2022

View reviewed changes

kezhenxu94 reviewed Nov 12, 2022

View reviewed changes

lenian added 2 commits November 12, 2022 16:00

[Feature-9467] add DM DataSource:1、remove DmJdbcDriver18 test scope; …

3b54440

…2、DmConnectionParam remove password in toString()

[Feature-9467] add DM DataSource: DmDataSourceParamDTO remove passwor…

bccd90e

…d in toString()

github-advanced-security bot found potential problems Nov 12, 2022

View reviewed changes

.../main/java/org/apache/dolphinscheduler/plugin/datasource/dm/param/DmDataSourceProcessor.java Fixed Show fixed Hide fixed

lenian added 7 commits November 12, 2022 21:35

[Feature-9467][DataSource] add DM（达梦） data source

ce54f59

[Feature-9467] add DM datasource doc (Issue apache#9467)

5f0efa8

[Feature-9467] add DM IDataBaseOptionKeys

885dbab

[Feature-9467] add DM DataSource:1、remove DmJdbcDriver18 test scope; …

2f589a0

…2、DmConnectionParam remove password in toString()

[Feature-9467] add DM DataSource: DmDataSourceParamDTO remove passwor…

66a13f4

…d in toString()

Merge remote-tracking branch 'origin/feat_dm_9467' into feat_dm_9467

923665c

[Feature-9467] add DM DataSource: DmDataSourceChannelFactoryTest remo…

a64e13c

…ve testGetDmConnection method

lenian closed this Nov 12, 2022

lenian requested a review from SbloodyS November 30, 2022 04:27

lenian added 2 commits December 7, 2022 09:35

[Feature-9467] add DM DataSource:

22969a2

1、modification package dm->dameng; 2、add Dameng DmJdbcDriver18 License

[Feature-9467] add DM DataSource: update dameng doc

c642406

davidzollo removed the Waiting for user feedback Waiting for feedback from issue/PR author label Dec 8, 2022

github-advanced-security bot found potential problems Dec 8, 2022

View reviewed changes

CalvinKirs reviewed Dec 29, 2022

View reviewed changes

ruanwenjun reviewed Dec 29, 2022

View reviewed changes

lenian added 2 commits December 30, 2022 10:04

[Feature-9467] add DM DataSource: update DamengConnectionParam

29f1dcf

[Feature-9467] add DM DataSource: remove DruidDataSourceClient

631efb8

Tianqi-Dotes previously approved these changes Jan 4, 2023

View reviewed changes

SbloodyS reviewed Jan 4, 2023

View reviewed changes

[Feature-9467] add DM DataSource: remove unnecessary changes

7a12dc7

lenian dismissed Tianqi-Dotes’s stale review via 7a12dc7 January 4, 2023 03:50

Tianqi-Dotes previously approved these changes Jan 4, 2023

View reviewed changes

SbloodyS reviewed Jan 4, 2023

View reviewed changes

docs/docs/zh/guide/datasource/dameng.md Outdated Show resolved Hide resolved

Tianqi-Dotes changed the title ~~[Feature-9467] add DM DataSource~~ [Feature-9467] add DAMENG DataSource Jan 4, 2023

[Feature-9467] add DM DataSource: doc use English screenshot

3799dcb

lenian dismissed Tianqi-Dotes’s stale review via 3799dcb January 4, 2023 04:12

lenian requested review from Tianqi-Dotes and SbloodyS and removed request for Tianqi-Dotes and SbloodyS January 4, 2023 04:14

SbloodyS approved these changes Jan 4, 2023

View reviewed changes

Tianqi-Dotes approved these changes Jan 4, 2023

View reviewed changes

Tianqi-Dotes merged commit bcf03ad into apache:dev Jan 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature-9467] add DAMENG DataSource #12860

[Feature-9467] add DAMENG DataSource #12860

lenian commented Nov 10, 2022

DarkAssassinator Nov 10, 2022

lenian Nov 11, 2022 •

edited

Loading

kezhenxu94 Nov 12, 2022

lenian Nov 12, 2022

kezhenxu94 Nov 12, 2022

davidzollo commented Dec 5, 2022

lenian commented Dec 6, 2022

CalvinKirs Dec 29, 2022

lenian Dec 30, 2022

CalvinKirs Dec 30, 2022

lenian Dec 30, 2022

ruanwenjun Dec 29, 2022 •

edited

Loading

lenian Dec 30, 2022

Tianqi-Dotes left a comment

SbloodyS Jan 4, 2023

lenian Jan 4, 2023

SbloodyS left a comment

sonarcloud bot commented Jan 4, 2023

		+ ", password='" + password + '\''
		+ ", address='" + address + '\''

[Feature-9467] add DAMENG DataSource #12860

[Feature-9467] add DAMENG DataSource #12860

Conversation

lenian commented Nov 10, 2022

Purpose of the pull request

Brief change log

Verify this pull request

Choose a reason for hiding this comment

lenian Nov 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidzollo commented Dec 5, 2022

lenian commented Dec 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanwenjun Dec 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tianqi-Dotes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SbloodyS left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 4, 2023

lenian Nov 11, 2022 •

edited

Loading

ruanwenjun Dec 29, 2022 •

edited

Loading