Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Allows encoding while constructing HTTP request for sending notification #35

Merged
merged 1 commit into from Apr 24, 2019

Conversation

mihirsoni
Copy link
Contributor

@mihirsoni mihirsoni commented Apr 18, 2019

Issue #, if available:
#33
Description of changes:

We were not encoding the message while publishing, using the StringEntity constructor with the mime-type and charset.

  • Chime / Slack Defaults application/json
  • Custom Webhook , Default to application/json , user can choose other values.

Sample Chime notification :
Screen Shot 2019-04-24 at 9 45 55 AM

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mihirsoni mihirsoni requested a review from vamshin April 18, 2019 05:30
Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

Thanks Mihir for the changes. Minor comments

@mihirsoni
Copy link
Contributor Author

@vamshin I just had to use charset while in entity as we've already set in headers on the POST so it wasn't required. I've removed other changes and added the charset for Body String.

@mihirsoni mihirsoni requested a review from vamshin April 21, 2019 06:37
Copy link
Contributor

@lucaswin-amzn lucaswin-amzn left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix

@mihirsoni mihirsoni merged commit 488d63d into opendistro-for-elasticsearch:master Apr 24, 2019
@mihirsoni mihirsoni deleted the issue-33 branch April 24, 2019 20:01
@Pupkur
Copy link

Pupkur commented May 23, 2019

Can you please explain how to set up character encoding?
Add header to "Header information"?
key - charset
value - UTF-8

And which version will fix this issue?

@mihirsoni
Copy link
Contributor Author

Can you please explain how to set up character encoding?

Generally , you can setup like this

Content-Type: text/html; charset=utf-8

And which version will fix this issue?

This will be released in next Opendistro version.

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

Successfully merging this pull request may close these issues.

None yet

4 participants