GSP-749: Unify Path Behavior #749
Conversation
Codecov Report
@@ Coverage Diff @@
## master #749 +/- ##
=======================================
Coverage 12.34% 12.34%
=======================================
Files 22 22
Lines 1434 1434
=======================================
Hits 177 177
Misses 1250 1250
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Let's start a quick discussion in room: https://matrix.to/#/#beyondstorage@gsp-749:matrix.org
|
docs/rfcs/749-unify-path-behavior.md
Outdated
|
|
||
| - Absolute Path: | ||
| - For Unix, the absolute path starts with `/`. | ||
| - For Windows, the absolute path starts with a drive letter. |
There was a problem hiding this comment.
Maybe only go-service-fs will support absolute path starts with a drive letter?
There was a problem hiding this comment.
I thought we will not support Read("C:\\xxxx") ?
docs/rfcs/749-unify-path-behavior.md
Outdated
|
|
||
| `path` is the file path for file system, or an object key for object storage. Also, it could be a prefix filter for `List` operation. | ||
|
|
||
| - For file system services on Windows, `path` MUST be the relative path for `WorkDir`. For file system on Unix, `path` could be an absolute path or a relative path. |
There was a problem hiding this comment.
For go-service-fs on windows, absolute path is OK which means a path under current volume. But absolute path with driver letter is not allowed.
docs/rfcs/749-unify-path-behavior.md
Outdated
|
|
||
| - Absolute Path: | ||
| - For Unix, the absolute path starts with `/`. | ||
| - For Windows, the absolute path starts with a drive letter. |
There was a problem hiding this comment.
I thought we will not support Read("C:\\xxxx") ?
docs/rfcs/749-unify-path-behavior.md
Outdated
| - For Unix, the absolute path starts with `/`. | ||
| - For Windows, the absolute path starts with a drive letter. | ||
| - Relative Path: | ||
| - The relative path is for the working directory `WorkDir`. |
There was a problem hiding this comment.
A bit strange. What's the meaning of for? How about make it more clear?
docs/rfcs/749-unify-path-behavior.md
Outdated
|
|
||
| ## Implementation | ||
|
|
||
| N/A |
There was a problem hiding this comment.
Please give more detail of the implementation?
- Will we change the definitions?
- Will we add new integration tests?
docs/rfcs/749-unify-path-behavior.md
Outdated
| The field `path` is used as an input parameter for operation, indicating the file path for file system, object key for object storage or prefix filter for `List` operator. | ||
|
|
||
| - `path` could be absolut path or relative path based on `work_dir`. | ||
| - `path` MUST be Unix style path but SHOULD NOT contain the prefix `/` for object storage services. |
There was a problem hiding this comment.
When users call store.Read, they don't know whether this storage is object storage or not. So I think it's not a good rule.
And it conflicts with the example in the following.
|
Mostly LGTM, let's create a tracking issue for this GSP! |
ref: #744