Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Feature][Connector-V2] Add Dingtalk Sink #2257 #2285

Merged
merged 2 commits into from
Aug 2, 2022
Merged

[Feature][Connector-V2] Add Dingtalk Sink #2257 #2285

merged 2 commits into from
Aug 2, 2022

Conversation

MRYOG
Copy link
Contributor

@MRYOG MRYOG commented Jul 28, 2022 •
edited
Loading

Add Dingtalk Sink #2257 ,include sink and api example

@ic4y
Copy link
Contributor

ic4y commented Jul 28, 2022

cc @Hisoka-X

Hisoka-X
Hisoka-X reviewed Jul 28, 2022
View reviewed changes
pom.xml Outdated
@@ -87,7 +87,7 @@
<module>seatunnel-plugin-discovery</module>
<module>seatunnel-formats</module>
<module>seatunnel-dist</module>
<module>seatunnel-server</module>
<module>seatunnel-server</module>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<module>seatunnel-server</module>
<module>seatunnel-server</module>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

seatunnel-connectors-v2/connector-dingtalk/pom.xml Outdated
<parent>
<artifactId>seatunnel-connectors-v2</artifactId>
<groupId>org.apache.seatunnel</groupId>
<version>2.1.3-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<version>2.1.3-SNAPSHOT</version>
<version>${revision}</version>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

seatunnel-connectors-v2/connector-dingtalk/pom.xml Outdated

<artifactId>connector-dingtalk</artifactId>

<properties>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove maven jdk version. We should make sure our connector can compiled success both on jdk8 and jdk11.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

...ctor-dingtalk/src/main/java/org/apache/seatunnel/connectors/seatunnel/sink/DingTalkSink.java Outdated

@Override
public void prepare(Config pluginConfig) throws PrepareFailException {
this.pluginConfig = pluginConfig;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check your config, make sure url and secret parameter must included in pluginConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

...ctor-v2-example/src/main/java/org/apache/seatunnel/example/flink/v2/SeaTunnelApiExample.java Outdated
@@ -31,7 +31,7 @@
public class SeaTunnelApiExample {

public static void main(String[] args) throws FileNotFoundException, URISyntaxException, CommandException {
String configFile = getTestConfigFile("/examples/fake_to_console.conf");
String configFile = getTestConfigFile("/examples/fake_to_dingtalk.conf");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't change this default config. We should make sure other developer can run this example easy. You can create a SeaTunnelDingTaskExample.java or create a e2e test for DingTalk.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@Hisoka-X
Copy link
Member

Also please add DingTalk properties to plugin-mapping.properties file.

@MRYOG
Copy link
Contributor Author

MRYOG commented Jul 28, 2022

Got it

EricJoy2048
EricJoy2048 requested changes Jul 28, 2022
View reviewed changes
Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the document for this connector. The path is docs/en/connector-v2/sink.
Thanks.

@MRYOG
Copy link
Contributor Author

MRYOG commented Jul 28, 2022

Please add the document for this connector. The path is docs/en/connector-v2/sink. Thanks.

@MRYOG MRYOG closed this Jul 28, 2022
@MRYOG MRYOG reopened this Jul 28, 2022
EricJoy2048
EricJoy2048 reviewed Jul 29, 2022
View reviewed changes
Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't update this file .idea/vcs.xml

@EricJoy2048
Copy link
Member

Please add connector-dingtalk to seatunnel-connector-v2-dist/pom.xml.

@MRYOG MRYOG force-pushed the dev branch 2 times, most recently from 8d2f23e to 8a3906e Compare July 29, 2022 11:14
@Hisoka-X
Copy link
Member

Hi Please resolve ci problem. Thanks!

@MRYOG
Copy link
Contributor Author

MRYOG commented Jul 31, 2022

add doc , dingtalk.md
add v2-dist pom.xml
add plugin-mapping.properties
add DingTalk example
fix {reversion}
confim vcs.xml , pom.xml

@MRYOG
Copy link
Contributor Author

MRYOG commented Aug 1, 2022

add doc , dingtalk.md add v2-dist pom.xml add plugin-mapping.properties add DingTalk example fix {reversion} confim vcs.xml , pom.xml

It's done

@Hisoka-X
Copy link
Member

Hisoka-X commented Aug 1, 2022

Please fix ci error.

@MRYOG MRYOG force-pushed the dev branch 3 times, most recently from 4cfb144 to defe1bb Compare August 1, 2022 06:35
@MRYOG
Copy link
Contributor Author

MRYOG commented Aug 1, 2022

Sorry, my checkstyle is not seatunnel config, so it has many problem.

@EricJoy2048
Copy link
Member

Sorry, my checkstyle is not seatunnel config, so it has many problem.

SeaTunnel checkstyle file in tools/checkstyle

@MRYOG
Copy link
Contributor Author

MRYOG commented Aug 1, 2022

Sorry, my checkstyle is not seatunnel config, so it has many problem.

SeaTunnel checkstyle file in tools/checkstyle

Yes,I fix it

@EricJoy2048
Copy link
Member

cc @CalvinKirs @Hisoka-X First-time contributors need a maintainer to approve running workflow

@Hisoka-X Hisoka-X merged commit 88a26d5 into apache:dev Aug 2, 2022
TyrantLucifer pushed a commit to TyrantLucifer/incubator-seatunnel that referenced this pull request Sep 18, 2022
@MRYOG @TyrantLucifer
[Feature][Connector-V2] Add Dingtalk Sink apache#2257 (apache#2285)
3454537 
* [Feature][Connector2] Add Dingtalk Sink apache#2257

* [Feature][Connector2] Add Dingtalk Sink apache#2257
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EricJoy2048 EricJoy2048 EricJoy2048 requested changes

@Hisoka-X Hisoka-X Hisoka-X approved these changes

Assignees

@MRYOG MRYOG

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MRYOG @ic4y @Hisoka-X @EricJoy2048