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][My Hours]Add My Hours Source Connector #3162

Closed
wants to merge 0 commits into from

Conversation

TaoZex
Copy link
Contributor

@TaoZex TaoZex commented Oct 22, 2022

Purpose of this pull request

#3018 My Hours Source Connector
config1
res1
config2
res2

Check list

@TaoZex
Copy link
Contributor Author

TaoZex commented Oct 24, 2022

@EricJoy2048 @TyrantLucifer @hailin0 PTAL

Comment on lines 86 to 88
ObjectMapper om = new ObjectMapper();
Map<String, String> contentMap = om.readValue(content, Map.class);
return contentMap.get(MyHoursSourceConfig.ACCESSTOKEN);
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
ObjectMapper om = new ObjectMapper();
Map<String, String> contentMap = om.readValue(content, Map.class);
return contentMap.get(MyHoursSourceConfig.ACCESSTOKEN);
Map<String, String> contentMap = JsonUtils.toMap(content);
return contentMap.get(MyHoursSourceConfig.ACCESSTOKEN);

return contentMap.get(MyHoursSourceConfig.ACCESSTOKEN);
}
}
log.error("login http client execute exception, http response status code:[{}], content:[{}]", response.getCode(), response.getContent());
Copy link
Member

Choose a reason for hiding this comment

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

If get token failed, I think should throw exception there not print error message, because without token connector will generate data failed.

bodyParams.put(MyHoursSourceConfig.PASSWORD, password);
bodyParams.put(MyHoursSourceConfig.CLIENTID, MyHoursSourceConfig.API);
ObjectMapper om = new ObjectMapper();
String body = om.writeValueAsString(bodyParams);
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
String body = om.writeValueAsString(bodyParams);
String body = JsonUtils.toJsonString(bodyParams);


import lombok.Data;

@Data
Copy link
Member

Choose a reason for hiding this comment

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

Remove useless annotation.

import java.util.Map;

@Data
@SuppressWarnings("MagicNumber")
Copy link
Member

Choose a reason for hiding this comment

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

Why add this annotation?

@TyrantLucifer
Copy link
Member

BTW, I think it is important to standardize commit messages.

@TaoZex
Copy link
Contributor Author

TaoZex commented Oct 24, 2022

BTW, I think it is important to standardize commit messages.

Thanks for your review and advice.I will fix it.

@EricJoy2048
Copy link
Member

Hi, @TaoZex Thanks for your contribution. My Hours have some API and every API can read as a table. So I think the My Hours Connector can config as this:

MyHours {
   url: "xxxxx"
   schema: {
   xxxx,
   xxxx
   }
   xxx
   xxxx
   xxxx
}

@TaoZex
Copy link
Contributor Author

TaoZex commented Oct 25, 2022

Hi, @TaoZex Thanks for your contribution. My Hours have some API and every API can read as a table. So I think the My Hours Connector can config as this:

MyHours {
   url: "xxxxx"
   schema: {
   xxxx,
   xxxx
   }
   xxx
   xxxx
   xxxx
}

Thanks for your advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants