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

Adding slack sink connector #60

Closed

Kanugula wants to merge 3 commits into hazelcast:master from Kanugula:slack

Kanugula commented Mar 26, 2020 •

edited by devOpsHazelcast

Loading

PR closed by Hazelcast automation as no activity (>6 months). Please reopen with comments, if necessary. Thank you for using Hazelcast and your valuable contributions


          Adding slack sink connector

Collaborator

devOpsHazelcast commented Mar 26, 2020

Can one of the admins verify this patch?

cangencer reviewed

View reviewed changes

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

Contributor

cangencer commented Mar 26, 2020

Thanks a lot for the contribution! I couldn't see your name in our contributor list: could you follow the steps at https://hazelcast.atlassian.net/wiki/spaces/COM/pages/6357071/Hazelcast+Contributor+Agreement ?

viliam-durina reviewed

View reviewed changes

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

viliam-durina reviewed

View reviewed changes

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

eminn added the new module label

eminn linked an issue

that may be closed by this pull request

Open

Contributor

eminn commented Mar 27, 2020

verify


          * Added additional method with slackChannelName to name the sink.

c545228

* Added retry http handler to handle some of the error causes.
* Variable names are updated as per review comments.
* Addressed other review comments.

Signed-off-by: Rajasekhar Kanugula <[email protected]>

Kanugula force-pushed the slack branch from 13f9867 to c545228 Compare

March 30, 2020 16:07

viliam-durina suggested changes

View reviewed changes

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java

+                              urlParameters.add(new BasicNameValuePair("text", message));
+                              request.setEntity(new UrlEncodedFormEntity(urlParameters));
+                              response = (CloseableHttpResponse) closeableHttpClient.execute(request);
+                              result = EntityUtils.toString(response.getEntity());

Contributor

viliam-durina Mar 31, 2020

We return the result, but the returned value is ignored. Is there something worthwhile to check?

Author

Kanugula Apr 21, 2020

Added the simple to pojo class to parse the response. In error scenarios sample message will look like this { "ok": false, "error": "not_authed" } If the Response.ok false means then throwing the RuntimeException.

slack/src/test/java/com/hazelcast/jet/contrib/slack/SlackSInksTest.java Outdated Show resolved Hide resolved

slack/src/test/java/com/hazelcast/jet/contrib/slack/SlackSInksTest.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

slack/src/main/java/com/hazelcast/jet/contrib/slack/SlackSinks.java Outdated Show resolved Hide resolved

Collaborator

devOpsHazelcast commented Apr 5, 2020 •

edited

Loading

All committers have signed the CLA.

Holmistr commented Apr 15, 2020

Hi @Kanugula , just checking in if there's anything we can help you out with? :)

Holmistr added the Source: Community label


          > Updated the documentation as per the review comments.

da4a948

> Processing the Rest response to SlackResponse to handle failure causes from the slack apis.

Signed-off-by: Rajasekhar Kanugula <[email protected]>

Holmistr commented Apr 21, 2020

@Kanugula Glad to see your edits :) @viliam-durina Could you please take another look?

viliam-durina self-requested a review

April 22, 2020 07:06

Contributor

cangencer commented Jun 29, 2020

@viliam-durina do you mind picking this up again?

viliam-durina removed their request for review

October 21, 2021 15:02

devOpsHazelcast closed this

devOpsHazelcast added the Automation: PR auto closed label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Automation: PR auto closed new module Source: Community