EVG-18372: Reimplement gridfs bucket #105
Conversation
julianedwards
changed the title
| @@ -25,6 +25,10 @@ func consistentJoin(elems []string) string { | |||
| return strings.Join(out, "/") | |||
| } | |||
|
|
|||
| func consistentTrimPrefix(key, prefix string) string { | |||
| return strings.TrimPrefix(key, prefix+"/") | |||
There was a problem hiding this comment.
Should this trim trailing slashes from prefix before appending a trailing slash to it to avoid double slashes in the prefix? I've had some ambiguity before where I was specifying the prefix and wasn't certain if the prefix was supposed to include the trailing slash or not.
There was a problem hiding this comment.
I think this is ok to leave, we can always change it later but this is only used internally.
gridfs_bucket.go
Outdated
| // it exists. This function is called by each operation that needs to read or | ||
| // write from the bucket to avoid read and write deadline conflicts—custom | ||
| // deadlines cannot be set on a GridFS bucket instance concurrently with other | ||
| // read or write operations that also require a custom deadlines. Setting the |
There was a problem hiding this comment.
nit: maybe phrase the last sentence as This function sets the read and write deadlines to allow it to respect the context timeouts passed in by the caller, since the phrasing seems to assume you already know it sets the read/write deadlines.
gridfs_bucket.go
Outdated
| @@ -115,6 +99,13 @@ func (b *gridfsBucket) Exists(ctx context.Context, key string) (bool, error) { | |||
|
|
|||
| func (b *gridfsBucket) Join(elems ...string) string { return consistentJoin(elems) } | |||
|
|
|||
| // bucket returns a new GridFS bucket configured with the context timeout, if | |||
| // it exists. This function is called by each operation that needs to read or | |||
| // write from the bucket to avoid read and write deadline conflicts—custom | |||
There was a problem hiding this comment.
I'm not sure what this sentence about custom deadlines is saying. Are there deadlines other than the context deadline in play here? What do custom deadlines and other read or write operations that also require custom deadlines refer to in this case?
Would help understanding if we can clarify this sentence a little bit. 🙏
gridfs_bucket.go
Outdated
| // it exists. This function is called by each operation that needs to read or | ||
| // write from the bucket to avoid read and write deadline conflicts—custom | ||
| // deadlines cannot be set on a GridFS bucket instance concurrently with other | ||
| // read or write operations that also require a custom deadlines. Setting the |
There was a problem hiding this comment.
nit: a custom deadline or custom deadlines
JIRA: https://jira.mongodb.org/browse/EVG-17662
Adding back the gridfs bucket implementation to facilitate support for logs on local evergreen.
I also fixed up some tests and improved consistent usage of path separators.