-
Notifications
You must be signed in to change notification settings - Fork 82
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
Update slack upload flow v2 #292
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure to rebase the branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not fixed, please rebase your branch
...-notifications-api/src/main/java/guru/qa/allure/notifications/clients/slack/SlackClient.java
Show resolved
Hide resolved
(cherry picked from commit 8670931)
@valfirst review please |
try (CloseableHttpClient client = HttpClients.createDefault()) { | ||
postMessage(client, messageData); | ||
} catch (IOException e) { | ||
log.error("Failed to post message to Slack", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrap into MessagingException
and re-throw it
slack.getTemplatePath())) | ||
.asString() | ||
.getBody(); | ||
@SneakyThrows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it needed?
postMessage(client, messageData, filePermalink); | ||
|
||
} catch (IOException e) { | ||
log.error("Failed to post message with file to Slack", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrap into MessagingException
and re-throw it
} | ||
|
||
private String getTextData(MessageData messageData) { | ||
return String.format("channel=%s&text=%s", slack.getChat(), proceedMessageData(messageData)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use UrlEncodedFormEntity
instead
} | ||
|
||
//todo wait until file processing to complete | ||
Thread.sleep(2000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there way to check is file processing completed instead of hardcoded wait?
@valfirst fix review. |
@Phoenix124 please rebase the changes against |
@valfirst updated from master |
HttpUriRequest request = RequestBuilder | ||
.get("https://slack.com/api/files.getUploadURLExternal") | ||
.addHeader(HttpHeaders.AUTHORIZATION, "Bearer " + slack.getToken()) | ||
.addHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this header really needed for GET request?
No description provided.