Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filestore util: Add basic implementation of 'filestore get' command. #4261

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Sep 21, 2017

This will get a hash and store the results into the filestore. Afterwards the file is duplicated in both the filestore and normal blockstore.

The next step would be to bypass the normal blockstore when fetching the hash from the network.

When offline this is equivalent to what I envisioned 'filestore mv' doing and thus closes #4193.

Related to #3981

Tagline: "Download IPFS objects into filestore.",
},
Arguments: []cmds.Argument{
cmds.StringArg("cid", true, false, "The cid of the IPFS object to be outputted."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe s/outputted/fetched/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it may not necessary fetch it if its already in local storage.

res.SetError(err, cmds.ErrNormal)
return
}
println("here we go")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

filestore/get.go Outdated
dag "github.com/ipfs/go-ipfs/merkledag"
pi "github.com/ipfs/go-ipfs/thirdparty/posinfo"
unixfs "github.com/ipfs/go-ipfs/unixfs"
cid "gx/ipfs/QmNp85zy9RLrQ5oQD4hPyS39ezrrXpcaa7R4Y9kxdWQLLQ/go-cid"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to always leave a space between go-ipfs imports and gx imports

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. Oversight, will fix.

@whyrusleeping
Copy link
Member

@kevina so this would be the equivalent of ipfs get <cid> && ipfs add --nocopy <cid> ?

@kevina
Copy link
Contributor Author

kevina commented Sep 21, 2017

@kevina so this would be the equivalent of ipfs get && ipfs add --nocopy ?

Basically, although slightly more efficient. This version doesn't pin though.

With a node getter that bypasses the normal block store it will get a block directly to the filestore without duplicating it (assuming it's not already in the local blockstore).

Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM after fixing Codeclimate issues.
Coverage is a bit low due to high density of error checking.

@whyrusleeping
Copy link
Member

Should be rebased, but LGTM. We should start the conversation for how to fetch things from the network to the filestore, entirely skipping the normal datastore.

@whyrusleeping whyrusleeping added this to the go-ipfs 0.4.14 milestone Nov 16, 2017
@ghost ghost assigned kevina Nov 16, 2017
@ghost ghost added the status/in-progress In progress label Nov 16, 2017
@kevina
Copy link
Contributor Author

kevina commented Nov 16, 2017

Rebased and circleci is green.

@whyrusleeping
Copy link
Member

@kevina Got a merge conflict here, sorry. Mind fixing that up and rebasing?

cc @keks in case there are any issues

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@kevina
Copy link
Contributor Author

kevina commented Nov 19, 2017

@keks except for errors this command has no output yet, please let me know if I am doing things correctly.

@keks
Copy link
Contributor

keks commented Nov 19, 2017

Just took a quick look and this seems correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement "ipfs filestore mv"
4 participants