[Improve][SourceConnector] Unified schema parameter, update IoTDB sou… #3896

sunnyzhuzhu · 2023-01-07T17:17:50Z

Purpose of this pull request

close #3823

Check list

Code changed are covered with tests, or it does not need tests for reason:
If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update change log that in connector document. For more details you can refer to connector-v2
2. Update plugin-mapping.properties and add new connector information in it
3. Update the pom file of seatunnel-dist
Update the release-note.

CalvinKirs

TBD: doc

sunnyzhuzhu · 2023-01-12T06:56:01Z

TBD: doc

Thanks for your suggestions！I have updated doc and checked code style with docs/contribution/coding-guide.md. Can you help to approve the ci workflows again, since this is my first-time contribution. Thanks very much! @CalvinKirs

TyrantLucifer · 2023-01-12T13:40:07Z

Thank you for your contribution. Let's waiting CI process.

TyrantLucifer

Please update some code as #3897. You should update option rulle

...-iotdb/src/main/java/org/apache/seatunnel/connectors/seatunnel/iotdb/source/IoTDBSource.java

TyrantLucifer · 2023-01-17T05:26:14Z

...-iotdb/src/main/java/org/apache/seatunnel/connectors/seatunnel/iotdb/source/IoTDBSource.java

+        try {
+            Config schemaConfig = pluginConfig.getConfig(SeaTunnelSchema.SCHEMA.key());
+            this.typeInfo = SeaTunnelSchema.buildWithConfig(schemaConfig).getSeaTunnelRowType();
+            pluginConfig.entrySet().forEach(entry -> configParams.put(entry.getKey(), entry.getValue().unwrapped()));
+        } catch (Exception e) {
+            throw new IotdbConnectorException(SeaTunnelAPIErrorCode.CONFIG_VALIDATION_FAILED,
+                String.format("PluginName: %s, PluginType: %s, Message: %s",
+                    getPluginName(), PluginType.SOURCE, e)
+            );
        }


Suggested change

try {

Config schemaConfig = pluginConfig.getConfig(SeaTunnelSchema.SCHEMA.key());

this.typeInfo = SeaTunnelSchema.buildWithConfig(schemaConfig).getSeaTunnelRowType();

pluginConfig.entrySet().forEach(entry -> configParams.put(entry.getKey(), entry.getValue().unwrapped()));

} catch (Exception e) {

throw new IotdbConnectorException(SeaTunnelAPIErrorCode.CONFIG_VALIDATION_FAILED,

String.format("PluginName: %s, PluginType: %s, Message: %s",

getPluginName(), PluginType.SOURCE, e)

);

}

SeaTunnelSchema seaTunnelSchema = SeaTunnelSchema.buildWithConfig(pluginConfig.getConfig(SeaTunnelSchema.SCHEMA.key()));

this.typeInfo = seaTunnelSchema.getSeaTunnelRowType();

pluginConfig.entrySet().forEach(entry -> configParams.put(entry.getKey(), entry.getValue().unwrapped()));

Revert this logic. As is.

@TyrantLucifer Thanks for your suggestions.I have done this.

TyrantLucifer · 2023-01-17T05:27:09Z

...src/main/java/org/apache/seatunnel/connectors/seatunnel/iotdb/source/IoTDBSourceFactory.java

@@ -49,7 +50,7 @@ public String factoryIdentifier() {
    @Override
    public OptionRule optionRule() {
        return OptionRule.builder()
-                .required(NODE_URLS, USERNAME, PASSWORD, SQL)
+                .required(NODE_URLS, USERNAME, PASSWORD, SQL, SeaTunnelSchema.SCHEMA)


static import is better

@TyrantLucifer Thanks for your suggestions.I have done this.

…rce fields to schema

TyrantLucifer

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs

CalvinKirs · 2023-01-17T07:29:35Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳

I have one question, like this #3959

TyrantLucifer · 2023-01-17T07:32:43Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳

I have one question, like this #3959

Unified parameters is convenient for frontend display the parameters. Do I understand right? @EricJoy2048

EricJoy2048 · 2023-01-17T09:02:38Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳

I have one question, like this #3959

Unified parameters is convenient for frontend display the parameters. Do I understand right? @EricJoy2048

Yes, I like this pr. Thanks for your contirbution.

CalvinKirs · 2023-01-17T10:04:27Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳↳

I have one question, like this #3959↳

Unified parameters is convenient for frontend display the parameters. Do I understand right? @EricJoy2048↳

Yes, I like this pr. Thanks for your contirbution.↳

I think you didn't get my point, what we need to consider is compatibility

wfrong · 2023-01-17T10:59:51Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳↳

I have one question, like this #3959↳

Unified parameters is convenient for frontend display the parameters. Do I understand right? @EricJoy2048↳

Yes, I like this pr. Thanks for your contirbution.↳

I think you didn't get my point, what we need to consider is compatibility

I think you mean we should support previous usage only config fields without schema?

sunnyzhuzhu · 2023-01-17T11:41:54Z

LGTM, thank you for your contirbution. Looking forward your next pull request! BTW, before this pr be merged CI should be passed. Let's waiting CI. cc @hailin0 @Hisoka-X @EricJoy2048 @CalvinKirs↳↳

I have one question, like this #3959↳

Unified parameters is convenient for frontend display the parameters. Do I understand right? @EricJoy2048↳

Yes, I like this pr. Thanks for your contirbution.↳

I think you didn't get my point, what we need to consider is compatibility

Sounds reasonable! Should I change on this pr to support previous usage that may exists in some users' jobs configure? @TyrantLucifer @EricJoy2048 @CalvinKirs

CalvinKirs reviewed Jan 11, 2023

View reviewed changes

wfrong mentioned this pull request Jan 16, 2023

[Improve][SourceConnector] Unified schema parameter, update IoTDB source fields to schema #3823

Closed

3 tasks

TyrantLucifer mentioned this pull request Jan 16, 2023

[Improve][SourceConnector] Unifie InfluxDB source fields to schema #3897

Merged

5 tasks

TyrantLucifer requested changes Jan 16, 2023

View reviewed changes

...-iotdb/src/main/java/org/apache/seatunnel/connectors/seatunnel/iotdb/source/IoTDBSource.java Outdated Show resolved Hide resolved

...-iotdb/src/main/java/org/apache/seatunnel/connectors/seatunnel/iotdb/source/IoTDBSource.java Outdated Show resolved Hide resolved

sunnyzhuzhu requested a review from TyrantLucifer January 16, 2023 15:32

sunnyzhuzhu force-pushed the IoTDB_source_schema branch from c2f0209 to 0213cc1 Compare January 16, 2023 16:06

TyrantLucifer reviewed Jan 17, 2023

View reviewed changes

eaglezhu added 7 commits January 17, 2023 15:14

[Improve][SourceConnector] Unified schema parameter, update IoTDB sou…

dad1da0

…rce fields to schema

update IoTDB source doc

7092cf6

update e2e conf & fix code style

72d4e7f

throw exception when source schema is not configured

f4bf830

update try catch scope

a115416

update change log

dd07bf1

use static import & update exception handling

173697a

sunnyzhuzhu force-pushed the IoTDB_source_schema branch from 605d990 to 173697a Compare January 17, 2023 07:15

sunnyzhuzhu requested a review from TyrantLucifer January 17, 2023 07:18

TyrantLucifer approved these changes Jan 17, 2023

View reviewed changes

github-actions bot added approved reviewed labels Jan 17, 2023

liugddx approved these changes Jan 17, 2023

View reviewed changes

EricJoy2048 merged commit a0959c5 into apache:dev Jan 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improve][SourceConnector] Unified schema parameter, update IoTDB sou… #3896

[Improve][SourceConnector] Unified schema parameter, update IoTDB sou… #3896

sunnyzhuzhu commented Jan 7, 2023 •

edited

CalvinKirs left a comment

sunnyzhuzhu commented Jan 12, 2023

TyrantLucifer commented Jan 12, 2023

TyrantLucifer left a comment

TyrantLucifer Jan 17, 2023 •

edited

TyrantLucifer Jan 17, 2023

sunnyzhuzhu Jan 17, 2023

TyrantLucifer Jan 17, 2023

sunnyzhuzhu Jan 17, 2023

TyrantLucifer left a comment •

edited

CalvinKirs commented Jan 17, 2023

TyrantLucifer commented Jan 17, 2023

EricJoy2048 commented Jan 17, 2023

CalvinKirs commented Jan 17, 2023

wfrong commented Jan 17, 2023

sunnyzhuzhu commented Jan 17, 2023

[Improve][SourceConnector] Unified schema parameter, update IoTDB sou… #3896

[Improve][SourceConnector] Unified schema parameter, update IoTDB sou… #3896

Conversation

sunnyzhuzhu commented Jan 7, 2023 • edited

Purpose of this pull request

Check list

CalvinKirs left a comment

Choose a reason for hiding this comment

sunnyzhuzhu commented Jan 12, 2023

TyrantLucifer commented Jan 12, 2023

TyrantLucifer left a comment

Choose a reason for hiding this comment

TyrantLucifer Jan 17, 2023 • edited

Choose a reason for hiding this comment

TyrantLucifer Jan 17, 2023

Choose a reason for hiding this comment

sunnyzhuzhu Jan 17, 2023

Choose a reason for hiding this comment

TyrantLucifer Jan 17, 2023

Choose a reason for hiding this comment

sunnyzhuzhu Jan 17, 2023

Choose a reason for hiding this comment

TyrantLucifer left a comment • edited

Choose a reason for hiding this comment

CalvinKirs commented Jan 17, 2023

TyrantLucifer commented Jan 17, 2023

EricJoy2048 commented Jan 17, 2023

CalvinKirs commented Jan 17, 2023

wfrong commented Jan 17, 2023

sunnyzhuzhu commented Jan 17, 2023

sunnyzhuzhu commented Jan 7, 2023 •

edited

TyrantLucifer Jan 17, 2023 •

edited

TyrantLucifer left a comment •

edited