Update go-getter client options #111
Conversation
nywilken
changed the title
There was a problem hiding this comment.
👍🏼
Since this one is a bigger change that might break existing templates, do you think we should add the changelog entry in this same PR with more information about the changes? Maybe mentioning the bump as well #110
| DisableSymlinks: true, | ||
| // The order of the Getters in the list may affect the result | ||
| // depending if the Request.Src is detected as valid by multiple getters | ||
| Getters: []getter.Getter{ |
nywilken
force-pushed
the
nywilken/update-go-getter-client
branch
from
faee09d
to
f0617fc
Compare
| Netrc: true, | ||
| XTerraformGetDisabled: true, | ||
| HeadFirstTimeout: 10 * time.Second, | ||
| ReadTimeout: 30 * time.Second, |
There was a problem hiding this comment.
The other getter timeouts are 5 minutes, while this one is essentially 30 seconds. Maybe it'd make more sense to have a uniform timeout across the getters, or somehow make these configurable?
There was a problem hiding this comment.
I like the idea of the uniform timeout. TBH 30 seconds seems really short given the use of go-getter in Packer. I struggled a bit to find good defaults. But ultimately think that going high 30 minutes is a good base line. We can make these configurable for the step so that plugins could override if needed.
That said there is an open issue for improving iso_config to support some addition go-getter Headers, which could also include setting some of these values. Taking a page out of Nomad's book here.
| &getter.HgGetter{ | ||
| Timeout: 5 * time.Minute, | ||
| }, | ||
| new(getter.SmbClientGetter), |
There was a problem hiding this comment.
🤔 Should this getter have a timeout? I see this getter only exists in v2, and doesn't have that option currently. Just curious to your thoughts.
There was a problem hiding this comment.
Yeah - I think it should. It was not caught in the go-getter updates. I will follow up with a separate PR for that change.
There was a problem hiding this comment.
PR opened for the SmbClientGetter hashicorp/go-getter#369
nywilken
force-pushed
the
nywilken/update-go-getter-client
branch
from
f0617fc
to
900cdcf
Compare
Related to: https://github.com/hashicorp/packer-internal-issues/issues/38 (internal only)