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

inputs.x509_cert: Fix timeout issue #8824

Merged
merged 4 commits into from
Feb 16, 2021
Merged

inputs.x509_cert: Fix timeout issue #8824

merged 4 commits into from
Feb 16, 2021

Conversation

viperstars
Copy link
Contributor

@viperstars viperstars commented Feb 6, 2021 •
edited
Loading

closes #8809

If timeout is not configured, it will be set to "5ns" when adding to inputs. This cause the timeout issue #8809. Test case for reproducing is added in commit ac4cda1.

I fix the issue by setting timeout to "0s".

telegraf-tiger[bot]
telegraf-tiger bot approved these changes Feb 6, 2021
View reviewed changes
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Feb 6, 2021
@srebhan srebhan self-assigned this Feb 7, 2021
@srebhan srebhan added fix pr to fix corresponding bug and removed feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Feb 7, 2021
@viperstars viperstars requested a review from srebhan February 8, 2021 02:44
srebhan
srebhan approved these changes Feb 8, 2021
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Feb 8, 2021
@sjwang90 sjwang90 changed the title Bugfix/8809 inputs.x509_cert: Fix timeout issue Feb 9, 2021
@helenosheaa
Copy link
Member

@viperstars looks good. Can you please add closes #8809 to your description so it will close the issue when we merge?

@viperstars viperstars requested a review from srebhan February 12, 2021 09:17
@viperstars
Copy link
Contributor Author

@viperstars looks good. Can you please add closes #8809 to your description so it will close the issue when we merge?

It is added in the new commit message now.

@srebhan
Copy link
Member

srebhan commented Feb 12, 2021

@viperstars I think github only evaluates the final commit message which is the PR description above. That's due to the fact that the changes are squashed before merge. So please simply open the PR at the github site and click edit in the top PR message. Then add the line closes #8809 and save. There is no need for a code change.

@viperstars
Copy link
Contributor Author

viperstars commented Feb 12, 2021 •
edited
Loading

@viperstars I think github only evaluates the final commit message which is the PR description above. That's due to the fact that the changes are squashed before merge. So please simply open the PR at the github site and click edit in the top PR message. Then add the line closes #8809 and save. There is no need for a code change.

It is updated now. Thanks for the tip.

srebhan
srebhan approved these changes Feb 15, 2021
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Perfect.

@ivorybilled ivorybilled merged commit fe16d56 into influxdata:master Feb 16, 2021
ssoroka pushed a commit that referenced this pull request Feb 17, 2021
@viperstars @ssoroka
inputs.x509_cert: Fix timeout issue (#8824)
341aca6 
* reproduce timeout issue #8809

* fix timeout issue #8809

* set default timeout to 5s

* closes #8809

Co-authored-by: 彭浩 <penghao@qiyi.com>
(cherry picked from commit fe16d56)
@viperstars viperstars deleted the bugfix/8809 branch February 18, 2021 05:52
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@viperstars
inputs.x509_cert: Fix timeout issue (influxdata#8824)
749a28d 
* reproduce timeout issue influxdata#8809

* fix timeout issue influxdata#8809

* set default timeout to 5s

* closes influxdata#8809

Co-authored-by: 彭浩 <penghao@qiyi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan approved these changes

@helenosheaa helenosheaa helenosheaa approved these changes

@ivorybilled ivorybilled ivorybilled approved these changes

@telegraf-tiger telegraf-tiger[bot] telegraf-tiger[bot] approved these changes

Assignees

@srebhan srebhan

Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

telegraf plugin x509_cert stopped working from 1.17.1 upwards with dial tcp: i/o timeout
4 participants
@viperstars @helenosheaa @srebhan @ivorybilled