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][Connector2] Add DingTalk Source #2684 #2757

Closed
wants to merge 2 commits into from

Conversation

MRYOG
Copy link
Contributor

@MRYOG MRYOG commented Sep 16, 2022

Description
support V2 Connector for DingTalk

Usage Scenario
use DingTalk API get data

Copy link
Member

@ashulin ashulin left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, please add the e2e test.

<dependency>
<groupId>org.apache.seatunnel</groupId>
<artifactId>connector-http-base</artifactId>
<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>${project.version}</version>

Copy link
Member

Choose a reason for hiding this comment

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

the dependency is not used.

Comment on lines +52 to +56
<dependency>
<groupId>com.aliyun</groupId>
<artifactId>dingtalk</artifactId>
<version>1.4.26</version>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

I checked the DingTalk documentation, this is the new version of the sdk; so can the old version of the sdk(alibaba-dingtalk-service-sdk) be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

old sdk version is used by sink plugin , next sink plugin update will be remove

protected final DingTalkParameter dtParameter;
protected DingTalkClient dtClient;
protected OapiV2DepartmentListsubRequest dtRequest;
protected final DeserializationSchema<SeaTunnelRow> deserializationSchema;
Copy link
Member

Choose a reason for hiding this comment

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

The filed is not used.

}
}
}
LOGGER.error("Ding Talk client execute exception, response status code:[{}], content:[{}]", response.getErrorCode(), response.getBody());
Copy link
Member

Choose a reason for hiding this comment

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

Output error log under any circumstances?I think you need else here.

## Key features

- [x] [batch](../../concept/connector-v2-features.md)
- [ ] [stream](../../concept/connector-v2-features.md)
Copy link
Member

Choose a reason for hiding this comment

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

It can be seen from the code that the connector supports the stream module. Please check it.

|-----------| ---------- | -------- | ------------- |
| api_client | string | yes | - |
| access_token | string | yes | - |
| app_key | string | yes | - |
Copy link
Member

Choose a reason for hiding this comment

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

check style

GetAccessTokenResponse res = client.getAccessToken(getAccessTokenRequest);
appToken = res.getBody().getAccessToken();
} catch (Exception e) {
e.printStackTrace();
Copy link
Member

Choose a reason for hiding this comment

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

Use slf4j logger error and throw Exception?


if (pluginConfig.hasPath(DingTalkConstant.SCHEMA)) {
Config schema = pluginConfig.getConfig(DingTalkConstant.SCHEMA);
this.rowType = SeaTunnelSchema.buildWithConfig(schema).getSeaTunnelRowType();
Copy link
Member

Choose a reason for hiding this comment

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

Unused fields in schema?

String tmpContent = response.getBody();
JsonNode bodyJson = JsonUtils.stringToJsonNode(tmpContent);
JsonNode resJson = bodyJson.get(DingTalkConstant.BODY_RESULT);
if (resJson.isArray()) {
Copy link
Member

Choose a reason for hiding this comment

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

If it's not an array, do you need to log exception message?

@Hisoka-X
Copy link
Member

Hisoka-X commented Oct 9, 2022

Please solve conflict, thanks!

@MonsterChenzhuo
Copy link
Contributor

@MRYOG Will this pr go ahead? If not, I would like to take over and finish it

@EricJoy2048
Copy link
Member

This pr will be closed because there is no response for too long.

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

6 participants