Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fs interface needs to support context.Context for all API calls #27

Open
nt opened this issue Dec 5, 2015 · 4 comments
Open

Fs interface needs to support context.Context for all API calls #27

nt opened this issue Dec 5, 2015 · 4 comments

Comments

@nt
Copy link

nt commented Dec 5, 2015

Proper implementations of Afero for rpc backed filesystems will require passing a context.Context.

See: https://godoc.org/golang.org/x/net/context

@mbertschler
Copy link
Collaborator

@nt could you point us to an example of a similar project where a Context is used? Would all the Fs functions need a second version where a Context is passed or how would the API look like?

@nt
Copy link
Author

nt commented Dec 6, 2015

Here is an example: https://godoc.org/google.golang.org/api/storage/v1#BucketsListCall.Context
see this blog post for details: https://blog.golang.org/context.

To generalize your API you would do:

type Fs interface {
    // Create creates a file in the filesystem, returning the file and an
    // error, if any happens.
    Create(ctx context.Context, name string) (File, error)

        ...
}

That would allow user implementing a rpc or http handler supporting contexts such as grpc or gorrilla to passit through as is. If the parent call is cancelled, fanned out rpcs to a remote Afero backend can be cancelled as well and resources can be released.

This is critical if you want to set deadline on calls and you do not know how long fs.Create("foo") will take.

@spf13
Copy link
Owner

spf13 commented Dec 10, 2015

I understand the need for this. However most of the backends wouldn't need or be able to use this. One of the initial goals of afero was to be interoperable with the os package as much as possible. Our function signatures mirror that of the os pacakge. This would be a dramatic shift.

Additionally wouldn't the Context be better set on the entire Fs rather than per function. This would enable compatibility with current backends.

I see Sameer's note in his post that "At Google, we require that Go programmers pass a Context parameter as the first argument to every function on the call path between incoming and outgoing requests.".

Just trying to figure out how to incorporate this without breaking everything.

@prochac
Copy link

prochac commented Sep 1, 2024

What about something like this?
golang/go#20280 (comment)

fs := NewAWSS3FS( /* ... */ ) // good 'ol XMLHttpRequest 😅
afs := &afero.AferoCtx{Fs: fs}
f, err := afs.Context(ctx).Create("foo.txt")
f.Close()
info, err := afs.Context(ctx).Stat("foo.txt")

BTW Go's context has been criticized precisely for this :/
https://faiface.github.io/post/context-should-go-away-go2/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants