zeroshade commented on code in PR #2079:
URL: https://github.com/apache/iceberg-go/pull/2079#discussion_r4168187042
##########
catalog/rest/vended_creds_test.go:
##########
@@ -928,3 +933,214 @@ func TestPrefixScopedIOPreservesReadContextCancellation(t
*testing.T) {
cancel()
require.ErrorIs(t, p.ctx.Err(), context.Canceled)
}
+
+// contextCapturingScheme is a filesystem that behaves like the gocloud
+// backends: every load returns a blobfs.FileIO over its own in-memory bucket,
+// opened on the context it is given, which blobfs keeps and uses for every
+// operation. It records each context and counts closes; closing an IO closes
+// its bucket.
+type contextCapturingScheme struct {
+ scheme string
Review Comment:
Nit: nothing reads `scheme`, because the factory closure already captures
the parameter. It can be removed.
##########
catalog/rest/vended_creds.go:
##########
@@ -171,33 +176,60 @@ func (v *vendedCredentialRefresher) loadFS(ctx
context.Context) (iceio.IO, error
maps.Copy(config, freshCreds)
}
+ // The IO is cached and shared by every later caller, so it must not
+ // inherit this caller's cancellation: a filesystem may keep the
context it
+ // is opened with (blobfs does, so every gocloud backend), and a cached
IO
+ // built on a per-operation context would fail every later operation
with
+ // context.Canceled once that context is done. The refresher owns the
IO's
+ // lifetime instead, through ioCancel. fetchCreds above still honours
ctx.
+ //
+ // WithoutCancel keeps ctx's values, so the first caller's
request-scoped
Review Comment:
Nit: the IO depends on the ctx values that `WithoutCancel` keeps; they
aren't just a cost. The S3 factory takes its base AWS config from
`utils.GetAwsConfig(ctx)` (`io/gocloud/s3/s3.go:192`) on `ioCtx`. Please say so
in the comment so a later cleanup doesn't switch this to `context.Background()`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
