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

Allow resty backend timeout to be configurable #3637

Merged
merged 2 commits into from
Mar 27, 2020

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Mar 20, 2020

What is this change?

Allow resty backend timeout to be configurable

Why is this change necessary?

Closes #3635

The timeout isn't configurable, as gets triggered on highly used installations.

Does your change need a Changelog entry?

Yes, the changelog was updated in the "Unreleased" section

Have you reviewed and updated the documentation for this change? Is new documentation required?

Documentation likely needs to be updated about this change. Could someone point me to the right location?

How did you verify this change?

Build sensuctl binary locally, and verified the setting was adjusted, and persisted in configuration file

Is this change a patch?

No

Update config commands to allow getting/setting timeout
Update basic config tests to show read/write success
Update command tests to show getting/setting timeout
Update changelog

Signed-off-by: naemono <[email protected]>
Copy link
Contributor

@echlebek echlebek left a comment

Choose a reason for hiding this comment

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

Looks like some of the tests are failing, and need to have mock expectations set. Something like

m.On("Timeout").Return(time.Second * 15)

cli/client/config/mock.go Outdated Show resolved Hide resolved
cli/client/testing/mock_config.go Outdated Show resolved Hide resolved
@naemono
Copy link
Contributor Author

naemono commented Mar 22, 2020

@echlebek these issues should be resolved now with latest commit.

Please be aware of etcd-io/bbolt#187 as I was running go 1.14, and ran directly into this when running unit tests. Mass failure.

@naemono naemono force-pushed the make-timeout-configurable branch from 48cb398 to f9a39cf Compare March 22, 2020 23:34
Copy link
Contributor

@echlebek echlebek 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 well-made PR!

@echlebek
Copy link
Contributor

To be merged following the 5.19.0 release.

@echlebek echlebek merged commit 7a9f9f4 into sensu:master Mar 27, 2020
echlebek pushed a commit that referenced this pull request Mar 27, 2020
* Update config commands to allow getting/setting timeout
* Update basic config tests to show read/write success
* Update command tests to show getting/setting timeout
* Update changelog

Signed-off-by: naemono <[email protected]>
echlebek pushed a commit that referenced this pull request Mar 27, 2020
* Update config commands to allow getting/setting timeout
* Update basic config tests to show read/write success
* Update command tests to show getting/setting timeout
* Update changelog

Signed-off-by: naemono <[email protected]>
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.

[Proposal] Allowing Resty HTTP Timeout to be configurable
2 participants