-
-
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
core: add context.Context param to core.Resolve() #1189
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,12 +91,12 @@ output is the raw data of the object. | |
} | ||
|
||
fpath := path.Path(req.Arguments()[0]) | ||
output, err := objectData(n, fpath) | ||
node, err := core.Resolve(req.Context().Context, n, fpath) | ||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
} | ||
res.SetOutput(output) | ||
res.SetOutput(bytes.NewReader(node.Data)) | ||
}, | ||
} | ||
|
||
|
@@ -121,7 +121,12 @@ multihash. | |
} | ||
|
||
fpath := path.Path(req.Arguments()[0]) | ||
output, err := objectLinks(n, fpath) | ||
node, err := core.Resolve(req.Context().Context, n, fpath) | ||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
} | ||
output, err := getOutput(node) | ||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
|
@@ -176,7 +181,7 @@ This command outputs data in the following encodings: | |
|
||
fpath := path.Path(req.Arguments()[0]) | ||
|
||
object, err := objectGet(n, fpath) | ||
object, err := core.Resolve(req.Context().Context, n, fpath) | ||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
|
@@ -242,7 +247,7 @@ var objectStatCmd = &cmds.Command{ | |
|
||
fpath := path.Path(req.Arguments()[0]) | ||
|
||
object, err := objectGet(n, fpath) | ||
object, err := core.Resolve(req.Context().Context, n, fpath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this feels strange to me... not sure whether or not i like the pattern There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please see my comment below (on gh..). I have no strong feelings on this, can change it back as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, removed all of the helpers. |
||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
|
@@ -343,42 +348,6 @@ Data should be in the format specified by the --inputenc flag. | |
Type: Object{}, | ||
} | ||
|
||
// objectData takes a key string and writes out the raw bytes of that node (if there is one) | ||
func objectData(n *core.IpfsNode, fpath path.Path) (io.Reader, error) { | ||
dagnode, err := core.Resolve(n, fpath) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
log.Debugf("objectData: found dagnode %s (# of bytes: %d - # links: %d)", fpath, len(dagnode.Data), len(dagnode.Links)) | ||
|
||
return bytes.NewReader(dagnode.Data), nil | ||
} | ||
|
||
// objectLinks takes a key string and lists the links it points to | ||
func objectLinks(n *core.IpfsNode, fpath path.Path) (*Object, error) { | ||
dagnode, err := core.Resolve(n, fpath) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
log.Debugf("objectLinks: found dagnode %s (# of bytes: %d - # links: %d)", fpath, len(dagnode.Data), len(dagnode.Links)) | ||
|
||
return getOutput(dagnode) | ||
} | ||
|
||
// objectGet takes a key string from args and a format option and serializes the dagnode to that format | ||
func objectGet(n *core.IpfsNode, fpath path.Path) (*dag.Node, error) { | ||
dagnode, err := core.Resolve(n, fpath) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
log.Debugf("objectGet: found dagnode %s (# of bytes: %d - # links: %d)", fpath, len(dagnode.Data), len(dagnode.Links)) | ||
|
||
return dagnode, nil | ||
} | ||
|
||
// ErrEmptyNode is returned when the input to 'ipfs object put' contains no data | ||
var ErrEmptyNode = errors.New("no data or links in this node") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ import ( | |
"io" | ||
"strings" | ||
|
||
context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" | ||
|
||
cmds "github.com/ipfs/go-ipfs/commands" | ||
core "github.com/ipfs/go-ipfs/core" | ||
crypto "github.com/ipfs/go-ipfs/p2p/crypto" | ||
|
@@ -89,7 +91,8 @@ Publish an <ipfs-path> to another public key (not implemented): | |
} | ||
|
||
// TODO n.Keychain.Get(name).PrivKey | ||
output, err := publish(n, n.PrivateKey, p) | ||
// TODO(cryptix): is req.Context().Context a child of n.Context()? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the top level context stuff around here is really confusing.. it uses this context group thing if i remember correctly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jbenet can you tell us? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apparently There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and yes it's very confusing :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jbenet ohai cmdlib 😞 .. but fixed 😄. Do I need to mirror this change on the offline mode, too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
probably? i can think about this more carefully tomorrow. |
||
output, err := publish(req.Context().Context, n, n.PrivateKey, p) | ||
if err != nil { | ||
res.SetError(err, cmds.ErrNormal) | ||
return | ||
|
@@ -106,14 +109,14 @@ Publish an <ipfs-path> to another public key (not implemented): | |
Type: IpnsEntry{}, | ||
} | ||
|
||
func publish(n *core.IpfsNode, k crypto.PrivKey, ref path.Path) (*IpnsEntry, error) { | ||
func publish(ctx context.Context, n *core.IpfsNode, k crypto.PrivKey, ref path.Path) (*IpnsEntry, error) { | ||
// First, verify the path exists | ||
_, err := core.Resolve(n, ref) | ||
_, err := core.Resolve(ctx, n, ref) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
err = n.Namesys.Publish(n.Context(), k, ref) | ||
err = n.Namesys.Publish(ctx, k, ref) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,12 @@ import ( | |
) | ||
|
||
func Pin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) { | ||
// TODO(cryptix): do we want a ctx as first param for (Un)Pin() as well, just like core.Resolve? | ||
ctx := n.Context() | ||
|
||
dagnodes := make([]*merkledag.Node, 0) | ||
for _, fpath := range paths { | ||
dagnode, err := core.Resolve(n, path.Path(fpath)) | ||
dagnode, err := core.Resolve(ctx, n, path.Path(fpath)) | ||
if err != nil { | ||
return nil, fmt.Errorf("pin: %s", err) | ||
} | ||
|
@@ -43,7 +45,7 @@ func Pin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) { | |
return nil, err | ||
} | ||
|
||
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute) | ||
ctx, cancel := context.WithTimeout(ctx, time.Minute) | ||
defer cancel() | ||
err = n.Pinning.Pin(ctx, dagnode, recursive) | ||
if err != nil { | ||
|
@@ -61,10 +63,12 @@ func Pin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) { | |
} | ||
|
||
func Unpin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) { | ||
// TODO(cryptix): do we want a ctx as first param for (Un)Pin() as well, just like core.Resolve? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. most likely, yeah. I'd also be inclined to move Unpin towards taking only a single path and moving the iteration logic upwards. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. Next PR though, this is getting big. |
||
ctx := n.Context() | ||
|
||
dagnodes := make([]*merkledag.Node, 0) | ||
for _, fpath := range paths { | ||
dagnode, err := core.Resolve(n, path.Path(fpath)) | ||
dagnode, err := core.Resolve(ctx, n, path.Path(fpath)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -75,7 +79,7 @@ func Unpin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) { | |
for _, dagnode := range dagnodes { | ||
k, _ := dagnode.Key() | ||
|
||
ctx, cancel := context.WithTimeout(context.TODO(), time.Minute) | ||
ctx, cancel := context.WithTimeout(ctx, time.Minute) | ||
defer cancel() | ||
err := n.Pinning.Unpin(ctx, k, recursive) | ||
if err != nil { | ||
|
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.
Yeah, this is super sketchy. I think its probably best to put the context into the request object instead of here on the handler context
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.
👍
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 is put into the request object, but it's put here: line 104. maybe we can find another place to put it, earlier? (nbd for me -- i want to rewire how all this works already)