pkg/storage: add External implementation #1587
Conversation
|
@marwan-at-work this PR will close these issues in addition to #1110 :
Can you put those 3 in the OP? Edit: it can also close #1551 |
marwan-at-work
force-pushed
the
external
branch
from
ceed6bc
to
41f5890
Compare
There was a problem hiding this comment.
@marwan-at-work this looks really good!! 🚀 
I think this is good as-is, and my comments could be be for follow-ups if you like. One other thing that we can also leave for a follow-up would be writing down the external storage API in docs, in case someone else wants to roll their own server
| srv := httptest.NewServer(handler) | ||
| defer srv.Close() | ||
| externalStrg := NewClient(srv.URL, nil) | ||
| clear := strg.(interface{ Clear() error }).Clear |
There was a problem hiding this comment.
worth having an explicit named Clearer (or something) interface?
There was a problem hiding this comment.
@arschles this method is only defined/used inside _test.go files and you cannot import it if you define it in a _test.go file anyway. So I'd keep it this way, but if we find ourselves doing this casting in other places, then it would def be worth it to define it in a none-test file and use it from within tests similar to our compliance package.
There was a problem hiding this comment.
@arschles that is where the method is defined, sorry i meant the interface-casting is only defined/used in _test.go
The reason that method is defined (and got moved from _test.go) to non-test Go file, is exactly because _test.go files in other packages don't get imported even during tests.
There was a problem hiding this comment.
I got ya, this is good for me then 👍
|
@arschles addressed all the comments, let me know if it looks good. thanks! |
|
I'm looking forward to this going in! I'll start writing some storage servers after it does 😄 Let's plan to do some docs after this is merged, and before it's released 🛩 🚀 |
This PR adds an http external storage implementation. It re-uses a lot of what we have already and optimizes "save" operations by uploading all 3 files (info/mod/zip) in one endpoint as a piped multi-writer.
Basic documentation and example is included, but I'll add more docs in follow ups.
Fixes #1110
Fixes #1353
Fixes #1130
Fixes #1131
Closes #1551