feat: files (mfs) api #54
Conversation
There was a problem hiding this comment.
Thanks! For some context, I think we punted on this as we wanted a unified interface for unixfs/files. However, given that we haven't made much progress on that, I'm happy to move this forward in the meantime and refactor later.
- Could you add some documentation to the interface?
- Document that it's incomplete.
- Document that that /ipfs and /ipns files can be copied in
Copy.
- Could you also add a
Removemethod? That'll cover the key methods we actually need.
|
@Stebalien oh, this is a WIP, I haven't yet added all methods I'm thinking of adding 😄 nor the comments! I will do that and ping you again! |
License: MIT Signed-off-by: Henrique Dias <[email protected]>
License: MIT Signed-off-by: Henrique Dias <[email protected]>
|
@Stebalien I actually implemented almost everything from the API as far as I know! Please take a look! |
There was a problem hiding this comment.
First pass.
If possible, could we pare down the scope of this? Move, Rename, and Remove are likely to make the final cut but the rest of the functions (read, write, etc.) were designed as a CLI API, not a programmatic API. Users should be able to open file-like objects with this API.
If necessary, we can try to tackle that right now but we'll have to sit down and try to figure out the right API.
| // FilesAPI specifies an interface to interact with the Mutable File System | ||
| // layer. | ||
| type FilesAPI interface { | ||
| Copy(ctx context.Context, src, dst string, opts *FilesCopyOptions) error |
There was a problem hiding this comment.
This should use the functional options pattern we use in the other APIs. It allows us to add new complicated options (with logic) while retaining backwards compatibility.
See: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
| type FilesAPI interface { | ||
| Copy(ctx context.Context, src, dst string, opts *FilesCopyOptions) error | ||
| Move(ctx context.Context, src, dst string, opts *FilesMoveOptions) error | ||
| List(ctx context.Context, path string, opts *FilesListOptions) ([]mfs.NodeListing, error) |
There was a problem hiding this comment.
Let's make this streaming from the start. See #49
There was a problem hiding this comment.
Let's re-use the return type from Unixfs().Ls(). I'd rather not expose internal mfs types in this API.
| CidBuilder cid.Builder | ||
| } | ||
|
|
||
| type FileInfo struct { |
| Size uint64 | ||
| CumulativeSize uint64 | ||
| Blocks int | ||
| Type string |
There was a problem hiding this comment.
Please use the file types from unixfs.go.
| Type string | ||
| WithLocality bool `json:",omitempty"` | ||
| Local bool `json:",omitempty"` | ||
| SizeLocal uint64 `json:",omitempty"` |
There was a problem hiding this comment.
Hm. I remember not liking this when it first came up. Can we come up with a better (internal) API and translate back at the edges? I'll think about this a bit but proposals are very welcome. For historical context: ipfs/kubo#4638
| Hash string | ||
| Size uint64 | ||
| CumulativeSize uint64 | ||
| Blocks int |
There was a problem hiding this comment.
I have no idea why we called this "blocks". Let's call it what it is, Children (NumChildren?).
| "github.com/ipfs/go-mfs" | ||
| ) | ||
|
|
||
| type FilesCopyOptions struct { |
License: MIT Signed-off-by: Henrique Dias <[email protected]>
License: MIT Signed-off-by: Henrique Dias <[email protected]>
License: MIT Signed-off-by: Henrique Dias <[email protected]>
|
@Stebalien before continuing, I think it's best to define which functions to keep 😄 You said What if we keep:
With that, we would remove What do you think? |
|
Status update: I believe we paused this in favor of trying to revive #19. Closing for now. |
This adds support for Files API.