Add the URLFor optional method to the storagedriver api #51
Conversation
We now also have a storagedriver error variable for identifying api calls that are not implemented by drivers (the URLFor method is not implemented by either the filesystem or inmemory drivers)
| return "", storagedriver.InvalidPathError{Path: path} | ||
| } | ||
|
|
||
| return d.Bucket.SignedURL(d.s3Path(path), time.Now().Add(24*time.Hour)), nil |
There was a problem hiding this comment.
For whatever reason the old registry created S3 URLs which were valid for 20 minutes (reference). I'm not sure what the recommended time frame should be, but I feel like 1 day is probably excessive.
There was a problem hiding this comment.
We can add that as an argument to URLFor(), but certain drivers will ignore the TTL. As long as we define the semantic properly and stick with it then it's ok.
There was a problem hiding this comment.
So, we are agreed on adding it as an argument to the URLFor?
There was a problem hiding this comment.
I added that to the discussion over at #49. Leave it out for now, and we can add it when a decision is made.
There was a problem hiding this comment.
I've suggested the URLFor rpc call take an options argument on #49 .
|
Looks good overall, but we'll wait for some more review of #49. |
| @@ -265,6 +265,12 @@ func (d *Driver) Delete(subPath string) error { | |||
| return err | |||
| } | |||
|
|
|||
| // URLFor returns a URL which may be used to retrieve the content stored at the given path. | |||
| // May return an UnsupportedMethodErr in certain StorageDriver implementations. | |||
| func (d *Driver) URLFor(path string) (string, error) { | |||
There was a problem hiding this comment.
I am starting to think we need a default implementation of the storage driver, with method stubs and simple implementations for URLFor, PutContent, GetContent and Move. Implementors can embed and replace as the driver dictates.
|
This is looking good. Is there some documentation that needs updating with this addition? |
|
Ok, LGTM! |
Add the URLFor optional method to the storagedriver api
…e-errors [ENGDTR-2139] Add Enforcement Policy specific errors to manifest handler
…e-errors [ENGDTR-2139] Add Enforcement Policy specific errors to manifest handler
…e-errors [ENGDTR-2139] Add Enforcement Policy specific errors to manifest handler
…e-errors [ENGDTR-2139] Add Enforcement Policy specific errors to manifest handler
This implements part of proposal #49