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

Increase api version for Beats monitoring #4793

Merged
merged 2 commits into from Aug 8, 2017

Conversation

Projects
None yet
3 participants
@monicasarbu
Copy link
Contributor

commented Jul 31, 2017

This PR contains two changes:

  • increase the version of the API for Beats monitoring
  • add the response body with the error message returned by Elasticsearch, in case of an error

@monicasarbu monicasarbu force-pushed the monicasarbu:increase_api_version_monitoring branch from faea45a to be6b4cc Jul 31, 2017

@tsg

This comment has been minimized.

Copy link
Collaborator

commented Aug 2, 2017

@monicasarbu, can you rebase this one, please? That should fix the intake job on Jenkins.

Increase the api version to 6 for monitoring
and add to the response body returned by Elasticsearch in case of an error

Fix comment

@monicasarbu monicasarbu force-pushed the monicasarbu:increase_api_version_monitoring branch from be6b4cc to be03af8 Aug 2, 2017

@monicasarbu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2017

done @tsg

// add the response body with the error returned by Elasticsearch
retErr = fmt.Errorf("%v: %s", resp.Status, obj)
}

return status, obj, retErr

This comment has been minimized.

Copy link
@urso

urso Aug 2, 2017

Collaborator

Reading the code, I just realized the old implementaion did return a nil error on IO errors, which could affect bulk inserts.

how about:

status := resp.StatusCode
obj, err := ioutil.ReadAll(resp.Body)
if err != nil {
  return status, nil, err
}

if status >= 300 {
  err = fmt.Errorf("%v: %s", resp.Status, obj)
}

return status, obj, err

This comment has been minimized.

Copy link
@monicasarbu

monicasarbu Aug 3, 2017

Author Contributor

@urso Done, thanks for the review.

@tsg tsg merged commit 25fdff8 into elastic:master Aug 8, 2017

5 of 6 checks passed

beats-ci Build finished.
Details
CLA Commit author is a member of Elasticsearch
Details
codecov/patch 83.33% of diff hit (target 62.36%)
Details
codecov/project 62.39% (+0.03%) compared to 747cfd9
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tsg tsg added the needs_backport label Aug 8, 2017

ramon-garcia added a commit to ramon-garcia/beats that referenced this pull request Dec 5, 2017

Increase api version for Beats monitoring (elastic#4793)
* Increase the api version to 6 for monitoring
and add to the response body returned by Elasticsearch in case of an error

Fix comment

* Add requested change

athom added a commit to athom/beats that referenced this pull request Jan 25, 2018

Increase api version for Beats monitoring (elastic#4793)
* Increase the api version to 6 for monitoring
and add to the response body returned by Elasticsearch in case of an error

Fix comment

* Add requested change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.