Implement the command "ipfs repo has" by MichaelMure · Pull Request #4541 · ipfs/kubo

MichaelMure · 2018-01-03T00:01:09Z

Following #4028, I implemented a ipfs repo has command that tell if something is available locally or not.

USAGE
  ipfs repo has <key>... - Show if an object is available locally

SYNOPSIS
  ipfs repo has [--recursive | -r] [--] <key>...

ARGUMENTS

  <key>... - Key(s) to check for locality.

OPTIONS

  -r, --recursive bool - Check recursively the graph of objects to build more precise stats.

Results look like this:

non-recursive
$ ipfs repo has QmYGVhjTfVvBAAf2SAWMJsTDheo7UuyjQigGahmB8YU3ZH
QmYGVhjTfVvBAAf2SAWMJsTDheo7UuyjQigGahmB8YU3ZH	local
$ ipfs repo has QmPXvegq26x982cQjSfbTvSzZXn7GiaMwhhVPHkeTEhrPT
QmPXvegq26x982cQjSfbTvSzZXn7GiaMwhhVPHkeTEhrPT	non local

recursive
$ ipfs repo has -r QmQFnam7qMXfF8R1D3qrETCgESnVP8gt6iEnAbyG73gyqm
QmQFnam7qMXfF8R1D3qrETCgESnVP8gt6iEnAbyG73gyqm	unknow
$ ipfs repo has -r QmQLXHs7K98JNQdWrBB2cQLJahPhmupbDjRuH1b9ibmwVa
QmQLXHs7K98JNQdWrBB2cQLJahPhmupbDjRuH1b9ibmwVa	incomplete	4.1 MB of 6.9 MB (60.31%)
$ ipfs repo has -r QmeUKqHMr5r6wJzt3AVScnUAyC4TkdZGjYJNaFBCL1BmAv
QmeUKqHMr5r6wJzt3AVScnUAyC4TkdZGjYJNaFBCL1BmAv	complete	3.2 MB of 3.2 MB (100.00%)

License: MIT
Signed-off-by: Michael Muré batolettre@gmail.com

magik6k · 2018-01-03T00:04:31Z

core/commands/repo.go

+var repoHasCmd = &cmds.Command{
+	Helptext: cmdkit.HelpText{
+		Tagline:          "Show if an object is available locally",
+		ShortDescription: ``,


Some description would be nice

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

magik6k

Thanks! Other than few comments this looks mostly good.

magik6k · 2018-01-03T00:08:56Z

core/commands/repo.go

+					state = "incomplete"
+				}
+				if lo.SizeTotal <= 0 {
+					state = "unknow"


Typo: "unknown"

magik6k · 2018-01-03T00:14:10Z

core/commands/repo.go

+				if lo.Local {
+					state = "local"
+				} else {
+					state = "non local"


local/non local sounds a bit weird to me, not sure what would sound better though.

I'm open to suggestion ¯\_(ツ)_/¯

I think unknown instead of non local would be appropriate as we do not have any guarantees the key exists elsewhere/remotely.

But that's the point of this command, to tell if something is stored in the node, regardless of what happen elsewhere.
To illustrate, my usecase is a file sharing application. I want to know if this particular data has finished downloading or if the data the user want to share is still properly available locally.

"found", "missing"?

magik6k · 2018-01-03T00:16:58Z

core/commands/repo.go

+	Type: localityOutput{},
+}
+
+func walkBlock(ctx context.Context, dagserv dag.DAGService, merkleNode node.Node) (bool, int, int, error) {


I'd just call merkleNode a nd as that's what it's usually called

This should also probably get recursive or depth pram as it looks like that we traverse the tree even if --recursive is not specified

No, this function is not called without recursive

magik6k · 2018-01-03T00:26:50Z

core/commands/repo.go


 	Subcommands: map[string]*cmds.Command{
 		"stat": repoStatCmd,
+		"has":  repoHasCmd,


Imo this doesn't fit into ipfs repo. ipfs block or ipfs dag would be a better place (I'm not sure which one is better)

I choose ipfs repo because we are interested about what is inside the repo and nothing else. The question is 'is this stored on my node ?'.

Both ipfs block and ipfs dag will do network request and treat the graph of object as something that exist somewhere, regardless of the repo state.

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

MichaelMure · 2018-01-03T14:28:29Z

Some tests would be nice, but I'm not sure how. Is there some graph of file/directory that I could use ? The webui ?

whyrusleeping · 2018-01-03T22:49:48Z

@MichaelMure you should just be able to add stuff in a sharness test and test the command out. You can also manually craft a dag that you don't have parts of

Stebalien

So, I'm going to raise a bit of a fuss here. This is unixfs specific (download sizes) and should probably be a part of ipfs files ls and ipfs ls. (with --local, --size, and --tree flags). See my comment below.

It would be useful to also have a dag specific command (although I'd put that under ipfs dag but that wouldn't be able to give you all the information you want.

Stebalien · 2018-01-05T00:39:09Z

core/commands/repo.go

+		}
+
+		// Decode all the keys
+		var keys []*cid.Cid


Might as well preallocate this.

Stebalien · 2018-01-05T00:41:34Z

core/commands/repo.go

+		cmdkit.StringArg("key", true, true, "Key(s) to check for locality.").EnableStdin(),
+	},
+	Options: []cmdkit.Option{
+		cmdkit.BoolOption("recursive", "r", "Check recursively the graph of objects to build more precise stats"),


Should probably have a quiet option (exits with a zero status if we have everything).

Stebalien · 2018-01-05T00:41:46Z

core/commands/repo.go

+`,
+	},
+	Arguments: []cmdkit.Argument{
+		cmdkit.StringArg("key", true, true, "Key(s) to check for locality.").EnableStdin(),


Stebalien · 2018-01-05T01:01:17Z

core/commands/repo.go

+}
+
+func walkBlock(ctx context.Context, dagserv dag.DAGService, nd node.Node) (bool, int, int, error) {
+	stat, err := nd.Stat()


So... time to air a dirty secret... This only exists for unixfs. Don't use it. You can't know how much of a dag is missing (only how much of a unixfs filesystem is missing).

Have you considered using the ipfs files feature and extending the ls command? I think this functionality may fit better there. Also, MFS (ipfs files) is a much nicer way to work with IPFS.

Example:

> ipfs files cp /ipfs/QmId /some/path > ipfs files ls --local --tree --size /some/path /some/path └── (1MiB/2MiB) foo ├── (2MiB) bar.go └── (2MiB) baz

Yeah, i'm on-board with this functionality being in ipfs files. Especially since we should be able to do: ipfs files ls --local --tree --size /ipfs/QmFooBar (skipping the copy step)

(really, we should merge ipfs files {ls,etc.} and ipfs {ls,}.

Especially since we should be able to do...

I agree but we can't currently. Is there an issue for this?

So... time to air a dirty secret... This only exists for unixfs. Don't use it. You can't know how much of a dag is missing (only how much of a unixfs filesystem is missing).

I'm confused. If I'm looking at the code of Stat(), I don't see anything specific to unixfs. This does works as well:

$ echo 'test' | ipfs add added QmeomffUNfmQy76CQGy9NdmqEnnHU9soCexBnGU3ezPHVH QmeomffUNfmQy76CQGy9NdmqEnnHU9soCexBnGU3ezPHVH $ ipfs object stat QmeomffUNfmQy76CQGy9NdmqEnnHU9soCexBnGU3ezPHVH NumLinks: 0 BlockSize: 13 LinksSize: 2 DataSize: 11 CumulativeSize: 13

What am I missing ?
Do you mean that Stat() is not implemented properly for every kind of ipld block ? If so, why ?

More questions:

ipfs files is supposed to be MFS, right ? So if ipfs repo has is moved to ipfs files ls --local --tree --size, it will only works inside MFS. I'm not using mfs for my project so it's not helpful for me.

what would the --size parameter really mean ? Using it alone would intuitively means 'give me various size information you have'. But adding --local would somehow change it's meaning to 'check that everything is local properly and report how much if not' ?

Moving the feature to 'ipfs files ls' feels like shoe-horning to me. I think there is space for a proper 'node administrator' command to inspect what is inside a repository (and in my case help me drive ipfs properly).

Do you mean that Stat() is not implemented properly for every kind of ipld block ? If so, why ?

Yes. To be precise, Stat, etc. only apply to DagPB IPLD nodes (so, technically not only unixfs). One thing on my todo list is to fix the go interfaces and get rid of that method.

Why? IPLD came after unixfs and the merkledag (DagPB). We wanted to generalize the concept and create a system that:

Allowed structured linked data. DagPB does not allow much structure. We wanted something more like JSON.

Allowed us to interpret and treat all existing existing merkledags (like Ethereum and git) as one big merkleforest. That way, we could write one set of tools to work with any merkle-linked data.

That means we couldn't require any metadata fields like Size. We wouldn't have, e.g., been able to interpret a git repo as an IPLD dag if we did that.

I'm not using mfs for my project so it's not helpful for me.

Aside: IMO, you should. It makes managing files much simpler. Eventually, we'd like to using MFS for all local file management (where /ipfs and /ipns would be "virtual" directories within the MFS tree. Eventually, ipfs CMD will probably be aliases for ipfs files CMD (possibly with some path restrictions).

However, for now, it would probably be simpler to just add the flags to the ipfs ls command (we can work on unifying the commands later). That should work for your purpose.

what would the --size parameter really mean ? Using it alone would intuitively means 'give me various size information you have'. But adding --local would somehow change it's meaning to 'check that everything is local properly and report how much if not' ?

Sorry, that was poorly explained.

--size would mean "report the size of the file and how much has been downloaded so far". We could also just add a separate --local-size flag for that.

--local would mean "don't fetch anything when running this command". Ideally, we'd provide that as an argument to all commands. Otherwise, ls --tree would block until all directories (but not their files) have been fetched.

I think there is space for a proper 'node administrator' command to inspect what is inside a repository (and in my case help me drive ipfs properly).

It would be useful to have a command to check and see if we have locally something without fetching it. However, we can't make a general purpose command for getting size information about arbitrary IPLD nodes.

Thank you, that's helpful.

Aside: IMO, you should. It makes managing files much simpler. Eventually, we'd like to using MFS for all local file management (where /ipfs and /ipns would be "virtual" directories within the MFS tree. Eventually, ipfs CMD will probably be aliases for ipfs files CMD (possibly with some path restrictions).

For some context, my project is https://github.com/MichaelMure/Arbore. It's a social file sharing app. For a broader view on how I use IPFS, see #3866

Unless I miss something (entirely possible), I see MFS as more a problem than a help for me. I'm using the IPFS repository as a black box / buffer area. In particular, I need to:

import easily files/directory (ipfs add -r). I get a nice progress from the API. I don't understand how to do that with MFS. On top of that I would need to manage path inside of MFS where I really don't need to.

be able to trigger a download (ipfs pin add). MFS doesn't seems to support that (?). Each share in Arbore has it's own root CID, so it's very easy to manage. If I want to remove a share from IPFS, I just pin rm the root, trigger a garbage collect and IPFS will deal with everything.

be able to use the filestore. Not using twice the disk size when you handle big files is a nice touch. I don't think MFS is able to do that.

be able to encrypt files. Ideally it will be done by IPFS when encryption support is added. Will that be supported by MFS ?

It would be useful to have a command to check and see if we have locally something without fetching it. However, we can't make a general purpose command for getting size information about arbitrary IPLD nodes.

What about a command that return detailed size information only if the nodes are compatible ? If the cumulative size is not available, we are still able to tell if everything is local or not, and how much data is here (using Node.Size() recursively).

So, unless I got it wrong, MFS is not an option for me. I see two viable options for me:

move the feature to ipfs ls

fix the current PR for ipfs repo has to not use Node.Stat()

@Stebalien @whyrusleeping Thoughts ?

@MichaelMure

import easily files/directory (ipfs add -r). I get a nice progress from the API. I don't understand how to do that with MFS. On top of that I would need to manage path inside of MFS where I really don't need to.

Keep using ipfs add, Its really supposed to just be an alias for ipfs files add but that command doesnt exist yet (see the javascript api).

be able to trigger a download (ipfs pin add). MFS doesn't seems to support that (?). Each share in Arbore has it's own root CID, so it's very easy to manage. If I want to remove a share from IPFS, I just pin rm the root, trigger a garbage collect and IPFS will deal with everything.

For now, keep that flow. We need to add a command to ipfs files that lets you fully sync a given bit of content. Like ipfs files cp —full-sync /ipfs/QmFooBar /path/to/thing

be able to use the filestore. Not using twice the disk size when you handle big files is a nice touch. I don't think MFS is able to do that.

Keep using the filestore, ipfs files doesnt care where the blocks are.

be able to encrypt files. Ideally it will be done by IPFS when encryption support is added. Will that be supported by MFS ?

Yeah, It really won't get in your way at all for any of these concerns.

To be clear, what we're recommending (and as I said in my previous comment) the command would look like:

ipfs files ls —local —tree —size /ipfs/QmFooBar

Note that the content in question is not explicitly within the mfs namespace.

Stebalien · 2018-01-05T01:01:36Z

core/commands/repo.go

+				if lo.Local {
+					state = "local"
+				} else {
+					state = "non local"


"found", "missing"?

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

whyrusleeping · 2018-01-15T20:50:37Z

@Stebalien Heres a proposal:

USAGE
  ipfs files tree <path>... - Show information about a given subtree

SYNOPSIS
  ipfs files tree [--summary | --local] [--] <key>...

ARGUMENTS

  <path>... - Path to print information about.

OPTIONS

  --summary bool - Print a summary of tree's information.
  --local bool - Provide information about locality of data.

Stebalien · 2018-01-15T21:00:26Z

@whyrusleeping 👍 for not overloading the ls command.

What exactly would the summary include? Would the tree always include file sizes, types, etc?

eingenito · 2018-12-05T01:42:29Z

I think this PR was superseded by #4638. Closing and feel free to reopen if I got that wrong.

magik6k reviewed Jan 3, 2018

View reviewed changes

MichaelMure added 3 commits January 3, 2018 01:21

Implement the command "ipfs repo has"

5957f4a

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

fix codeclimate issue

e4ecbdf

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

Add a proper description for ipfs repo has

14b8dbd

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

MichaelMure force-pushed the has2 branch from 3ed133f to 14b8dbd Compare January 3, 2018 00:22

magik6k suggested changes Jan 3, 2018

View reviewed changes

address code review

b1430f1

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

whyrusleeping requested review from a user and Stebalien January 4, 2018 04:04

Stebalien requested changes Jan 5, 2018

View reviewed changes

address minor code review

8784d08

License: MIT Signed-off-by: Michael Muré <batolettre@gmail.com>

MichaelMure mentioned this pull request Jan 19, 2018

Add a 'ipfs files tree' command #4596

Closed

MichaelMure mentioned this pull request Feb 1, 2018

add a --with-local option to ipfs files stat #4638

Merged

eingenito closed this Dec 5, 2018

Uh oh!

Conversation

MichaelMure commented Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

magik6k left a comment

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!

MichaelMure Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelMure commented Jan 3, 2018

Uh oh!

whyrusleeping commented Jan 3, 2018

Uh oh!

Stebalien left a comment

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!

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!

whyrusleeping commented Jan 15, 2018

Uh oh!

Stebalien commented Jan 15, 2018

Uh oh!

eingenito commented Dec 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

MichaelMure commented Jan 3, 2018 •

edited

Loading

MichaelMure Jan 3, 2018 •

edited

Loading