update: add the ability to specify a http-timeout #233
Conversation
| factoryConfigs := make(map[string]driver.ConfigUnmarshaler) | ||
| factoryConfigs["rhel-vex"] = func(v interface{}) error { | ||
| cfg := v.(*vex.FactoryConfig) | ||
| cfg.CompressedFileTimeout = claircore.Duration(httpTimeout) |
There was a problem hiding this comment.
Could also just fix claircore to use the passed client's timeout instead of a default
There was a problem hiding this comment.
Speak on this; what do you mean
There was a problem hiding this comment.
So, right now the VEX fetcher has the ability to configure the timeout manually falling back to a default of 2 mins. But that makes situations like this annoying when the client's timeout is potentially set above 2 mins (because we're defaulting to less), it's like maybe the default timeout should just be the passed client's timeout and the CompressedFileTimeout should override that if needed.
There was a problem hiding this comment.
"The client's timeout" meaning like, a timeout on the passed context.Context?
There was a problem hiding this comment.
Well, whatever someone has set the Timeout to be in the http.Client they pass to Claircore
There was a problem hiding this comment.
The Timeout setting on the http.Client is a pretty big hammer, as it covers the whole time of making the request to calling http.Response.Body.Close(); I wouldn't expect that it'd be set for the passed Client.
There was a problem hiding this comment.
Hmmm fair enough. Now I'm really confused how to proceed; if I remove the configurable Client.Timeout then the arg http-timeout is kind of misleading, I could change the arg to be specific to VEX but it's future usefulness becomes kind of lessened.
Another option is to not set the Client.Timeout unless it's explicitly passed as an argument so it's infinite by default but I still think we'd need some way to specify the CompressedFileTimeout.
There was a problem hiding this comment.
I guess this might be fine as a big kludge? We should probably think this through in a new interface.
There was a problem hiding this comment.
Do you think it ever makes sense to have a default Deadline for all updater http calls and then individual levers for each updater as needed? Or should we take the "infinite by default" approach and allow users to set Deadlines as they wish?
Previously the timeout was set to 2 mins which is usually plenty for most scenarios but some users want to be able to specify a custom timeout for more latent environments. This adds a --http-timeout flag to the `update` mode and sets it on the client used to request data from security sources. Signed-off-by: crozzy <joseph.crosland@gmail.com>
crozzy
force-pushed
the
add-timeout-setting-ability
branch
from
a62827b to
c00172e
Compare
2 participants
Previously the timeout was set to 2 mins which is usually plenty for most scenarios but some users want to be able to specify a custom timeout for more latent environments. This adds a --http-timeout flag to the
updatemode and sets it on the client used to request data from security sources.