Support for non-Amazon S3 services #356
Conversation
adamstruck
force-pushed
the
generic-s3
branch
from
c02f9f7 to
6620711
Compare
adamstruck
force-pushed
the
generic-s3
branch
from
c7d2ff1 to
aa2f2c9
Compare
storage/local.go
Outdated
| if !ok { | ||
| return fmt.Errorf("local storage does not support put on %s", url) | ||
| return fmt.Errorf("Must provided an absolute path") |
There was a problem hiding this comment.
Also, please ensure the rawurl is included in the final error message, whether here or by the worker code.
storage/local.go
Outdated
| if !isAllowed(path, local.allowedDirs) { | ||
| return fmt.Errorf("Can't access file, path is not in allowed directories: %s", url) | ||
| return fmt.Errorf("Can't access file, path is not in allowed directories: %s", path) |
There was a problem hiding this comment.
This should stay url (or rawurl) so that the error relates directly to the task message, without transformation.
| } | ||
|
|
||
| if !l.Supports("/path/to/foo.txt", "/host/path", tes.FileType_FILE) { | ||
| t.Fatal("Expected normal file path to be supported") | ||
| err = l.Supports("/path/to/foo.txt") |
There was a problem hiding this comment.
This is surprising. I thought we were requiring file://?
There was a problem hiding this comment.
I didn't change any behavior here.
There was a problem hiding this comment.
Is this test contrary to this comment? #291 (review)
There was a problem hiding this comment.
Not necessarily. We can support both file URIs and linux style absolute paths. Windows users are just restricted to file URIs.
Either way, we have supported this for a long time now. This PR isn't changing any behavior in the local backend.
There was a problem hiding this comment.
Gotcha, thanks. Filed #364 to clean this up.
storage/swift.go
Outdated
| var writer io.WriteCloser | ||
| var err error | ||
|
|
||
| // TODO turn on checkHash |
There was a problem hiding this comment.
Yes, please do turn this on. If I understand correctly, it was previously on and was critical for preventing corrupted files in smc-het.
| case Directory: | ||
| // Create a done channel. | ||
| doneCh := make(chan struct{}) | ||
| defer close(doneCh) |
There was a problem hiding this comment.
That is a strange API choice by minio...
|
A few minor issues, but otherwise tons of great work here. Also, this: rabix/bunny#382 (comment) |
adamstruck
force-pushed
the
generic-s3
branch
from
d33f8eb to
d07e2db
Compare
New features:
s3://<endpoint>/<bucket>/<key>Fixes issues:
Known issues:
Example config: