S3 Backend MVP #75
S3 Backend MVP #75
Conversation
pkg/storage/s3.go
Outdated
| sess := session.Must(session.NewSession()) | ||
|
|
||
| s3Client := s3.New(sess) | ||
| getOutput, err := s3Client.GetObject(&s3.GetObjectInput{ |
There was a problem hiding this comment.
Seems like there's an easily accessible GetObjectWithContext method [1]. Given that we already have a context passed in, could we call that instead? Here and below.
[1] https://github.com/aws/aws-sdk-go/blob/master/service/s3/api.go#L2969
There was a problem hiding this comment.
It's pretty great that they support this now 🎉
pkg/storage/s3.go
Outdated
| } | ||
|
|
||
| func (b *s3Backend) read(_ context.Context, id string) ([]byte, error) { | ||
| sess := session.Must(session.NewSession()) |
There was a problem hiding this comment.
From the docs [1] it seems that these sessions (or perhaps even the client?) are designed to be cached/reused. Should we perahps store one in the backend object rather than creating it on every call? Here and below.
There was a problem hiding this comment.
It seems like storing the client makes sense for now. That may change, but it's a detail that doesn't impact the interface.
* s/dinowernli.me/github.com\/dinowernli/g
…nto feature/s3_backend
|
Let me know when this is ready for review. I'm assuming it currently isn't because it still includes all the now-merged renaming changes. |
A simple implementation for an S3 backend. I've tested and it "works" on my AWS(Screenshots).
The diff should become easier to read with the import renaming landed.