zeroshade commented on code in PR #286:
URL: https://github.com/apache/iceberg-go/pull/286#discussion_r1942723893
##########
io/s3.go:
##########
@@ -106,10 +106,26 @@ func ParseAWSConfig(ctx context.Context, props
map[string]string) (*aws.Config,
return awscfg, nil
}
+type ctxkey string
+
+const AwsCfgCtxKey ctxkey = "awsConfig"
+
Review Comment:
As specified in the context documentation:
> The provided key must be comparable and should not be of type string or
any other built-in type to avoid collisions between packages using context.
Users of WithValue should define their own types for keys. To avoid allocating
when assigning to an interface{}, context keys often have concrete type
struct{}.
The better route here would be to create an exported function and use that:
```go
type ctxkey struct{}
func WithAwsConfig(ctx context.Context, cfg *aws.Config) context.Context {
return context.WithValue(ctx, ctxkey{}, cfg)
}
func getAwsConfig(ctx context.Context) *aws.Config {
if v := ctx.Value(ctxkey{}); v != nil {
return v.(*aws.Config)
}
return v
}
```
This has the added benefit of ensuring that the only way anything would be
placed in the context as a value with that key is as an `*aws.Config` so you
don't need to do the type assertion check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
