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.

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

Try to close HTTP connections and wait for completion on output reload/close #10599

Closed
wants to merge 1 commit into from
Closed

Try to close HTTP connections and wait for completion on output reload/close #10599

wants to merge 1 commit into from

Conversation

urso
Copy link

@urso urso commented Feb 6, 2019 •
edited by ph
Loading

No description provided.

@urso urso requested a review from a team as a code owner February 6, 2019 00:55
houndci-bot
houndci-bot reviewed Feb 6, 2019
View reviewed changes
libbeat/outputs/elasticsearch/client.go
@@ -725,8 +739,15 @@ func (conn *Connection) Ping() (string, error) {
return response.Version.Number, nil
}

func (client *Client) Close() error {
Copy link

Choose a reason for hiding this comment

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

exported method Client.Close should have comment or be unexported

@ph ph self-assigned this Feb 6, 2019
ph
ph approved these changes Feb 6, 2019
View reviewed changes
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM I have tested it with my case in #10567

@ph ph added review needs_backport PR is waiting to be backported to other branches. labels Feb 6, 2019
@ph ph mentioned this pull request Feb 6, 2019
Closed
@urso urso force-pushed the pipeline-output-client-close branch from 2eb1aa9 to bf963ba Compare February 6, 2019 01:58
@urso urso mentioned this pull request Feb 6, 2019
Closed
@urso urso added v6.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Feb 6, 2019
@ph
Copy link
Contributor

ph commented Feb 11, 2019

I will take another look at this.

@urso
Copy link
Author

urso commented Jan 17, 2020

@ph I think this is still an issue affecting the Agent we have to solve, right?

@urso urso mentioned this pull request Jan 17, 2020
Closed
@urso urso closed this Jan 17, 2020
@ph
Copy link
Contributor

ph commented Jan 20, 2020

Yes this is still an issue, let me remove myself and assign the beats team.

@ph ph removed their assignment Jan 20, 2020
@ph
Copy link
Contributor

ph commented Jan 20, 2020

I guess we could either merge this or wait for bigger refactoring?

@ycombinator
Copy link
Contributor

@urso Now that #11068 is merged, did you want to resurrect this PR here? Or if you want I can take a stab at it too. Just let me know which way you prefer to go!

@andresrc andresrc added the Team:Integrations Label for the Integrations team label Mar 6, 2020
@ycombinator ycombinator mentioned this pull request Apr 1, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@ph ph ph approved these changes

Assignees
No one assigned
Labels
libbeat Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@urso @ph @ycombinator @houndci-bot @andresrc