Skip to content

docs(inputs.openntpd): Correct timeout config setting to the actually used default#16666

Merged
mstrandboge merged 4 commits intoinfluxdata:masterfrom
msimerson:patch-1
Mar 24, 2025
Merged

docs(inputs.openntpd): Correct timeout config setting to the actually used default#16666
mstrandboge merged 4 commits intoinfluxdata:masterfrom
msimerson:patch-1

Conversation

@msimerson
Copy link
Contributor

@msimerson msimerson commented Mar 20, 2025

up the config timeout from a ridiculous 5ms to 1s.

The timeout embedded in the code is 5 seconds, so perhaps that's what the config should be?

Summary

5 ms causes the collector to time out before ntpctl returns, and it returns very very fast. Also, it's too easy for humans reading the config to not notice the units are ms units and not s units.

Checklist

  • No AI generated code was used in this PR

Related issues

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@srebhan
Copy link
Member

srebhan commented Mar 20, 2025

@msimerson could you please sign the CLA and run make docs on the code so we can review your contribution!?

@msimerson
Copy link
Contributor Author

!signed-cla

@msimerson msimerson changed the title config: openntpd/sample.conf docs: openntpd/sample.conf Mar 20, 2025
@telegraf-tiger telegraf-tiger bot added the docs Issues related to Telegraf documentation and configuration descriptions label Mar 20, 2025
@telegraf-tiger
Copy link
Contributor

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.

@msimerson thanks for taking care. One comment. Furthermore, could you please rebase on the latest master to get CI fixed?

@srebhan srebhan changed the title docs: openntpd/sample.conf docs(inputs.openntpd): Correct timeout config setting to the actually used default Mar 21, 2025
@telegraf-tiger telegraf-tiger bot added the plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins label Mar 21, 2025
@srebhan srebhan self-assigned this Mar 21, 2025
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 the fix @msimerson!

@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 Mar 24, 2025
@srebhan srebhan assigned skartikey and mstrandboge and unassigned srebhan Mar 24, 2025
@mstrandboge mstrandboge merged commit f515d89 into influxdata:master Mar 24, 2025
27 checks passed
@github-actions github-actions bot added this to the v1.34.1 milestone Mar 24, 2025
@msimerson msimerson deleted the patch-1 branch March 24, 2025 19:09
srebhan pushed a commit that referenced this pull request Mar 24, 2025
justinwwhuang pushed a commit to justinwwhuang/telegraf_fork that referenced this pull request Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Issues related to Telegraf documentation and configuration descriptions plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins 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

Development

Successfully merging this pull request may close these issues.

4 participants