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.

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

[Feature][Connector-V2] Add Enterprise Wechat sink connector #2412

Merged
merged 15 commits into from
Aug 17, 2022

Conversation

531651225
Copy link
Contributor

#1946 Add Enterprise WeChart source base on HttpSink

Purpose of this pull request

Check list

@531651225 531651225 changed the title Wechat connector sink [Feature][Connector-V2] Wechat connector sink Aug 15, 2022
@531651225 531651225 changed the title [Feature][Connector-V2] Wechat connector sink [Feature][Connector-V2] Add Enterprise Wechat sink connector Aug 15, 2022

A sink plugin which use Enterprise WeChart robot send message

## Options
Copy link
Member

Choose a reason for hiding this comment

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

Missing description of other options, such as common options in http connector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing description of other options, such as common options in http connector

I have added description of other options,detail in new commit

private final String webHookUrl = "url";

@Override
public void prepare(Config pluginConfig) throws PrepareFailException {
Copy link
Member

Choose a reason for hiding this comment

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

This overwrite method is useless, because httpSink had checked the parameter url.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method use to construct httpParameter which WeChatHttpSinkWriter constructor needed.
WeChatHttpSinkWriter redefind a SeaTunnelRowType for WeChat send content in constructor.

Copy link
Member

Choose a reason for hiding this comment

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

In my option, you can change httpParameter that in connector-http-base from private to protected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my option, you can change httpParameter that in connector-http-base from private to protected.

Thx,i will try .this way is more convenient.

Copy link
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

If it is simply outputting the result without any formatting, then the sink is of little significance. For this kind of connector, what the user needs to see is a formatted result.

@531651225
Copy link
Contributor Author

If it is simply outputting the result without any formatting, then the sink is of little significance. For this kind of connector, what the user needs to see is a formatted result.

I have added a Commonly used output format ,detail in doc.

@531651225
Copy link
Contributor Author

If it is simply outputting the result without any formatting, then the sink is of little significance. For this kind of connector, what the user needs to see is a formatted result.

I have added a Commonly used output format ,detail in doc.

new output format in wethat actual display like this
图片

## Description

A sink plugin which use Enterprise WeChat robot send message
> For example, if the data from upstream is [`"告警状态": "firing", "告警时间": "2022-08-03 01:38:49","告警内容": "磁盘使用超出阈值"`], the output content to WeChat Robot is the following:
Copy link
Member

Choose a reason for hiding this comment

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

I suggest you change the chinese to english.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suggest you change the chinese to english.

Thx,i have changed in new committ

}
HashMap<Object, Object> objectMap = new HashMap<>();
objectMap.put(WeChatSinkConfig.WECHAT_SEND_MSG_CONTENT_KEY, stringBuffer.toString());
if (!weChatSinkConfig.getMentionedMobileList().isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

getMentionedMobileList() may return null value here. So I suggest you use CollectionUtils.isEmpty here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx,i have changed in new committ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getMentionedMobileList() may return null value here. So I suggest you use CollectionUtils.isEmpty here.

Thx,i have changed in new committ

EricJoy2048
EricJoy2048 previously approved these changes Aug 16, 2022
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.

+1

@531651225
Copy link
Contributor Author

+1

fix code checkstyle problem,can you help me review or approve

@CalvinKirs
Copy link
Member

AFAIK, There are other things that enterprise WeChat can do, and it will be better if you can improve it. Looking forward to your optimization PR.
There are other things that enterprise WeChat can do, and it will be better if you can improve it. Looking forward to your optimization PR.

@531651225
Copy link
Contributor Author

AFAIK, There are other things that enterprise WeChat can do, and it will be better if you can improve it. Looking forward to your optimization PR. There are other things that enterprise WeChat can do, and it will be better if you can improve it. Looking forward to your optimization PR.

Thx,I will try to do it.

@CalvinKirs CalvinKirs merged commit 3e200e0 into apache:dev Aug 17, 2022
TyrantLucifer pushed a commit to TyrantLucifer/incubator-seatunnel that referenced this pull request Sep 18, 2022
…#2412)

* [Feature][Connector-V2] Add Enterprise WeChart source output format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants