-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
base: master
Are you sure you want to change the base?
Conversation
f492112
to
55d4ae4
Compare
core/commands/filestore.go
Outdated
Tagline: "Download IPFS objects into filestore.", | ||
}, | ||
Arguments: []cmds.Argument{ | ||
cmds.StringArg("cid", true, false, "The cid of the IPFS object to be outputted."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe s/outputted/fetched/ ?
There was a problem hiding this comment.
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.
core/commands/filestore.go
Outdated
res.SetError(err, cmds.ErrNormal) | ||
return | ||
} | ||
println("here we go") |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. Oversight, will fix.
@kevina so this would be the equivalent of |
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). |
55d4ae4
to
11b927c
Compare
There was a problem hiding this 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.
11b927c
to
18c43d0
Compare
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. |
18c43d0
to
78832d3
Compare
78832d3
to
a2e1aa6
Compare
Rebased and circleci is green. |
License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>
a2e1aa6
to
7b8303d
Compare
@keks except for errors this command has no output yet, please let me know if I am doing things correctly. |
Just took a quick look and this seems correct. |
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