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

Include 'Content-Type' header in transport_options #250

Merged
merged 1 commit into from
Apr 29, 2017

Conversation

mthssdrbrg
Copy link
Contributor

Starting with Elasticsearch 5.3, (REST) requests without an explicit Content-Type are deprecated and Elasticsearch will log a warning for every request that is sent without a Content-Type header.

I addressed this in a similar manner to how elasticsearch-ruby did, see 1, 2.

(check all that apply)

  • tests added
  • tests passing
  • README updated (if needed)
  • README Table of Contents updated (if needed)
  • History.md and version in gemspec are untouched
  • backward compatible
  • feature works in elasticsearch_dynamic (not required but recommended)

Starting with Elasticsearch 5.3, rest requests without an explicit
Content-Type are deprecated and it'll log a warning for every request
that is sent without a Content-Type header.

I addressed this in a similar manner to how `elasticsearch-ruby` did,
see [1], [2].

[1]: elastic/elasticsearch-ruby#400
[2]: elastic/elasticsearch-ruby@76f8679
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.488% when pulling ce123ee on mthssdrbrg:content-type-header into 83f978f on uken:master.

1 similar comment
@coveralls
Copy link

coveralls commented Apr 13, 2017

Coverage Status

Coverage remained the same at 94.488% when pulling ce123ee on mthssdrbrg:content-type-header into 83f978f on uken:master.

@chancez
Copy link
Contributor

chancez commented Apr 21, 2017

👍 I'm seeing lots of logs complaining about this on my ES cluster. This would be very welcomed

@Tom-Fawcett
Copy link

Would be great to see this merged 👍

@pitr @repeatedly @gihad could we please get some clarification on who is the active maintainer, discussion on #240 suggests we don't currently have one?

@gihad gihad merged commit a3749f7 into uken:master Apr 29, 2017
@gihad
Copy link
Contributor

gihad commented Apr 29, 2017

@Tom-Fawcett That's correct, we currently don't have the bandwidth within Uken to actively maintain this repo but we can keep it open sourced and hopefully we will be able to spend more time on it soon.
External maintainers are welcomed, if anyone wants to contribute as a maintainer here please send me a message. I will make some effort to find some maintainers outside of Uken.

Having said that I merged your changed and bumped the plugin version.

@Tom-Fawcett
Copy link

@gihad thanks for clarifying, and merging; much appreciated!

Regarding maintance - have you read @repeatedly 's comments on #240 regarding possible help?

@gihad
Copy link
Contributor

gihad commented Apr 29, 2017

Thanks for the suggestion. I will reach out to him.

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.

5 participants