[RFC] MFS interface draft by magik6k · Pull Request #19 · ipfs/interface-go-ipfs-core

magik6k · 2019-03-12T01:35:43Z

Quick draft of the interface, few things missing / need discussion:

Flush / Sync
- Provide Sync method, open files with --flush=false unless file is opened with O_SYNC?
ChCID / SetPrefix / ?
Streaming ReadDir? (file.Readdir?)
Dedicated copy call? (We could abuse the fact that io.Copy can use WriteTo/ReadFrom, but dedicated call would be more clear about what happens)

djdv · 2019-03-12T15:17:38Z

Provide Sync method, open files with --flush=false unless file is opened with O_SYNC?

If this is done, it may be worth repeating the warning from the files api (the one in ipfs files --help) in the documentation for Open.
Since the only real concern here is sudden node failure, this really shouldn't be an issue as long as the user is aware of it.
Likewise, a note saying that (at least currently) files are synced implicitly on Close via the MFS pkg.
Although maybe users should be encouraged to write code that syncs prior to closing anyway in case this changes. (unlikely?)
Or wrapped in Close here as well.
Not sure how best to handle that one.

ChCID / SetPrefix / ?

I suppose these are kind of necessary to allow for migrations / prevent stagnation of MFS trees.
Although I'm not positive the implications here. Would the target for these functions be single nodes, or entire trees/roots?
I'm not sure what scenarios you might want to target single nodes or have a mixed tree (or if that's even valid). But I lack familiarity in that area. In general it seems like we'd like to at least have the option to move away from CIDv0, and this would likely help enable that.

Streaming ReadDir?

I'm a fan of this, in the same way Ls returns a channel. I don't see much value in waiting for the entire directory to be resolved up front, instead of processing entries as you need them. Obviously if you need the entire thing you can just construct it from the channel, and sort however is desired, etc.
Currently, in the fuse package, I resolve MFS directories to unixfs Directories and call EnumLinksAsync on them to get this behaviour.
(Side note: mfs.ForEachEntry is synchronous, and probably shouldn't be?)

Dedicated copy call?

This is kind of a tangent, but I should mention that in the fuse pkg's rename function, I plan to check if the src+dst strings allow for reference/cid operations, to avoid doing actual IO copies where possible.
If a dedicated copy call is implemented here, it may be worth inspecting the reader+writer's types to allow for the same thing.

For example, if the source reader is a DagReader|files.File, etc. and the writer is an MFS file, we could do the equivalent of ipfs files cp /ipfs/CID /whatever reducing the io copy to a cid copy. Falling back to wrapping io.Copy when this isn't possible or the types are unexpected.
Not sure if this is sensible or not though in a copy context.

type File interface {
Name() MfsPath

I wonder about consistency here. Some of the other objects have a .Name() which returns a string and a .Path() which returns a Path compatible object. Not weighing in on this, just pointing it out.

mfs.go

magik6k · 2019-03-13T15:18:07Z

mfs.go

+	Open(ctx context.Context, path MfsPath) (File, error)
+	OpenFile(ctx context.Context, path MfsPath, flag int, perm os.FileMode) (File, error)
+
+	Stat(ctx context.Context, path MfsPath) (os.FileInfo, error)


TODO: change this to match ipfs files stat better

Maybe but I'd consider not materializing the CID here for performance reasons. IMO, files stat command should call Flush and Stat.

(although... there are some atomicity issues there...)

Stebalien · 2019-03-14T16:57:46Z

Tell me when you'd like me to review this.

Stebalien · 2019-03-18T18:01:54Z

mfs.go

+//
+// This interface is inspired by https://godoc.org/github.com/src-d/go-billy,
+// but doesn't implement it
+type MfsPath interface {


We don't have to do this now, but I'd like to prefix all MFS paths with /local. If we do that, we won't have to have this special type (and we'll get rid of a bunch of namespace conflict issues.

(the upgrade path will be interesting but we may have to save this for an API refactor)

Should we support resolving /local outside Files API?

Yes but disable on gateway api?

How should ipfs files commands handle those?

Just prefix everything with /local?

Might be confusing for users (i.e. why those commands have to be different)

Does /ipfs equal /local/ipfs everywhere?

I slowly starting to think that using fancy types for paths isn't the best way to handle them (#16, #15), but using raw strings introduces another set of problems (CID serialization to name one). There is some trade-off to be made here..

FWIW, in the mount implementation I have the Files API mounted under /files, I almost feel like /local is a bit of a misnomer given that files placed there are still published/accessible to the network.

(for a bit of context, I think /local initially appeared in ipfs/kubo#4666 (comment))

"files" is fine, really. or maybe just "file" to match the file:/// schema? My goal is just to disambiguate paths.

Should we support resolving /local outside Files API?

See ipfs/specs#98 and ipfs/specs#98 (comment). The hope was to eventually find a way to unify these APIs.

For now, let's just push forward with what we have. I think this is a bikeshed for another day.

Stebalien · 2019-03-18T18:08:33Z

mfs.go

+	Open(ctx context.Context, path MfsPath) (File, error)
+	OpenFile(ctx context.Context, path MfsPath, flag int, perm os.FileMode) (File, error)
+
+	Stat(ctx context.Context, path MfsPath) (os.FileInfo, error)


Maybe but I'd consider not materializing the CID here for performance reasons. IMO, files stat command should call Flush and Stat.

(although... there are some atomicity issues there...)

Stebalien · 2019-03-18T18:08:56Z

mfs.go

+	// already a directory, MkdirAll does nothing and returns nil.
+	MkdirAll(ctx context.Context, path MfsPath, perm os.FileMode) error
+
+	Flush(ctx context.Context, path MfsPath) error


We might want to make this return the CID.

mfs.go

hacdias · 2019-12-11T19:01:04Z

Oh, I didn't notice thiss! I was building #54 but perhaps I can take over this. What do you think @magik6k @Stebalien?

Stebalien · 2019-12-13T13:47:39Z

@hacdias please do! I had forgotten about this. Also, take a look at https://github.com/spf13/afero for inspiration (looks like it may have inspired this interface as well). The main difference is that we need to pass contexts.

lidel · 2023-06-19T21:24:58Z

This repository is no longer maintained and has been copied over to boxo/mfs. In an effort to avoid noise and crippling in the Boxo repo from the weight of issues of the past, we are closing most issues and PRs in this repo. Please feel free to open a new PR in Boxo (and reference this issue) if resolving this issue is still critical for unblocking or improving your usecase.

You can learn more in the FAQs for the Boxo repo copying/consolidation effort.

magik6k changed the title ~~MFS interface draft~~ [RFC] MFS interface draft Mar 12, 2019

MFS interface draft

0c19a87

magik6k force-pushed the feat/mfs branch from 2202e43 to 0c19a87 Compare March 12, 2019 01:36

magik6k added 2 commits March 13, 2019 15:15

mfs: add contexts

d8751b5

mfs: attach to the core interface

39b8d13

magik6k commented Mar 13, 2019

View reviewed changes

mfs.go Show resolved Hide resolved

magik6k commented Mar 13, 2019

View reviewed changes

This was referenced Mar 14, 2019

CoreAPI: Files API ipfs/kubo#6081

Draft

CoreAPI progress tracking issue ipfs/kubo#4498

Open

magik6k added 5 commits March 14, 2019 18:42

mfs: drop ReaderAt from File, add WriterAt

c6ece89

mfs: expose as Files on CoreAPI

177e67f

mfs: implement FilePath func

26f5aa1

mfs: Flush call

9b1d915

mfs: Setup tests

cbc70f4

Stebalien reviewed Mar 18, 2019

View reviewed changes

magik6k added 2 commits March 19, 2019 17:53

mfs: Copy Call

ea1ef0f

mfs: remove Name from File

48d6eda

magik6k mentioned this pull request Mar 19, 2019

Return node from FlushPath ipfs/go-mfs#72

Merged

[WIP] drop MfsPath

91883fc

hacdias mentioned this pull request Dec 12, 2019

WIP (DO NOT LOOK AT THIS) #56

Closed

Stebalien mentioned this pull request Jan 17, 2020

feat: files (mfs) api #54

Closed

djdv mentioned this pull request Aug 29, 2021

[BOUNTY] Implement UnixFSv1.5 in go-ipfs ipfs/kubo#6920

Closed

lidel closed this Jun 19, 2023

Conversation

magik6k commented Mar 12, 2019

Uh oh!

djdv commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Stebalien commented Mar 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hacdias commented Dec 11, 2019

Uh oh!

Stebalien commented Dec 13, 2019

Uh oh!

lidel commented Jun 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

djdv commented Mar 12, 2019 •

edited

Loading