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

Feature request: parametrise timeout of HttpHelper #902

Closed
darmach opened this issue May 17, 2021 · 3 comments
Closed

Feature request: parametrise timeout of HttpHelper #902

darmach opened this issue May 17, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@darmach
Copy link

darmach commented May 17, 2021

Default 10 seconds of timeout of http.Client here:

Timeout: 10 * time.Second,
is a bit too short for me - it's used on part of the infrastructure that has some intermittent lags, and I can't do anything about it (vendor issue)

HttpDo is referenced by other HttpDo* request functions, so this would be useful in all of them - can the timeout be parametrized, and default to 10 if not defined otherwise?

@brikis98 brikis98 added enhancement New feature or request help wanted labels May 24, 2021
@brikis98
Copy link
Member

It can, but it's not easy to do so in Go in a backwards compatible way. I'm also worried about adding yet another parameter to the already long function signatures in http_helper.

Probably what we need to do is refactor the http_helper methods to take in an Options struct. That struct can contain tlsConfig, retries, sleepBetweenRetries, this new timeout setting, etc. What's nice is that the struct would be extensible, so we could add new params in a backwards compatible way, without too much verbosity, in the future.

A PR to do this is welcome.

@darmach
Copy link
Author

darmach commented May 24, 2021

This makes a lot of sense, I will try to do that if I have some spare time.

@denis256
Copy link
Member

Options for HTTP requests were introduced in v0.40.11

https://github.com/gruntwork-io/terratest/releases/tag/v0.40.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants