Add context support
Hi,
I'm working on a similar project, but for OneDrive.
The thing that kinda bothers me, is a lack of context.Context support.
I see you resort to context.Background().
The solution I settled on, without violating fs.Fs interface is:
// ContextFS is the interface implemented by a file system that is aware of context.
type ContextFS interface {
fs.FS
// Context returns a new FS with the given context. If the fs.FS implements
// additional interfaces, such as [ReadFileFS], they must be implemented by the
// returned FS.
Context(context.Context) fs.FS
}
For work with multiple fs.ContextFS implementation, one can use this helper:
func FSWithContext[T ContextFS](ctx context.Context, fs T) T {
return fs.Context(ctx).(T)
}
WDYT?
Then you can set context.Context before any fs.Fs use:
f, _ := fs.Context(ctx).Open("extractor_test/foo.json")
defer f.Close()
fileStat, err := fs.Stat(fileSystem.Context(ctx), filePath)
My current workaround is to wrap *s3.Client with my custom s3fs.Client implementation, that holds the context.
But it sets one context for all fs.FS operations.
I'd say - let's just add the following method
+ // WithContext returns a shallow copy of the S3FS that
+ // runs all its operations on the provided Context.
+ func (f S3FS) WithContext(ctx context.Context) *S3FS {
+ f.ctx = ctx
+ return &f
+}
usage:
fsys := s3fs.New(s3.New(s), bucket)
ctx := context.Background()
f, err := fsys.WithContext(ctx).Open("foo.txt")
_, _ = f, err
so, the change to the struct would be:
type S3FS struct {
cl Client
bucket string
readSeeker bool
+ ctx context.Context
}
when ctx == nil, then we use context.Background()
I'd like to avoid adding ContextFS interface to this package. It feels like it can be easily added (or wrapped) by the caller when needed. One reason is that if something similar gets added to the std lib one day we will have a deprecated code here. Second is that if s3fs defines this interface, it is still local to s3fs, so it doesn't solve the problem.
I'm glad you like the idea, or at least, you aren't against 🙃
Yes, the interface is unnecessary. I just mentioned it for a full picture, like the helper function.
I agree that the WithContext is a better name, longer, but better.
I'm thinking, WithContext(ctx context.Context) fs.FS may be more generic, although a bit against "accept interface, return types"
The usage of extra interfaces like fs.StatFS can be done the following way:
fs.Stat(fsys.WithContext(ctx), filePath)