feat(io/s3): support connect timeout property#1141
Conversation
5fdb93c to
f145e50
Compare
|
@laskoviymishka @tanmayrauth |
| } | ||
|
|
||
| if httpClient == nil { | ||
| httpClient = awshttp.NewBuildableClient() |
There was a problem hiding this comment.
Not a blocker for this PR since the same pattern already exists for s3.proxy-uri, but worth flagging: createS3Bucket (lines 167-176) applies tuned transport options (MaxIdleConnsPerHost: 256, MaxIdleConns: 256, etc.) only when awscfg.HTTPClient == nil. Setting s3.connect-timeout now populates HTTPClient here, so callers fall back to the AWS SDK's awshttp.NewBuildableClient() defaults — notably MaxIdleConnsPerHost: 10 instead of 256, which can hurt highly parallel scans. Could you open a follow-up issue to apply the same transport tuning inside ParseAWSConfig (or factor a shared newBuildableClient() helper) so the proxy and connect-timeout paths don't silently regress connection pooling?
f145e50 to
a28aa17
Compare
Fixes #941
Summary
s3.connect-timeoutproperty for S3 clients.60and60.0, plus Go duration strings such as5s.s3.proxy-uriby sharing the same buildable HTTP client.Supersedes #950 and addresses all three items from laskoviymishka's review: float-seconds parsing, zero/negative rejection, and proxy+timeout composition test.
Testing
go test ./io/gocloud/...Release note
S3 now supports the
s3.connect-timeoutproperty, accepting numeric seconds like60/60.0and Go duration strings like5s.