-
-
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
add ipfs dag stat command #7553
Conversation
1301846
to
3699694
Compare
if len(rp.Remainder()) > 0 { | ||
return fmt.Errorf("cannot return size for anything other than a DAG with a root 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.
This is supportable, but didn't seem necessary for an initial release of this command. Open to counter opinions.
3699694
to
948348b
Compare
This is really useful and something I use It would be great if this could be a top level command, e.g. |
We have |
@achingbrain for reasons... we also have I don't really have strong feelings about these aliases (other then I don't know why we have them at all), so if you'd like |
e7b35d2
to
3a09089
Compare
dagStats = out | ||
fmt.Fprintf(os.Stderr, "%v\r", out) | ||
} | ||
return re.Emit(dagStats) | ||
}, | ||
}, | ||
Encoders: cmds.EncoderMap{ | ||
cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, event *DagStat) error { | ||
_, err := fmt.Fprintf( | ||
w, | ||
"%v\n", | ||
event, | ||
) |
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.
This seems like the pattern we use for emitting updates as well as a final result, but would like some confirmation that this is reasonable.
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.
It is... I just wish we had a better way.
3a09089
to
d702fe7
Compare
|
||
test_expect_success "dag stat of simple IPLD object" ' |
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.
Added a few tests here, do we need more before shipping this?
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.
This seems reasonable.
core/commands/dag/dag.go
Outdated
|
||
dagstats := &DagStat{} | ||
err = traverse.Traverse(obj, traverse.Options{ | ||
DAG: api.Dag(), |
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.
We should probably try to create a bitswap session here, but I'm fine punting that if necessary (this API will usually be called on local DAGs, I assume).
dagStats = out | ||
fmt.Fprintf(os.Stderr, "%v\r", out) | ||
} | ||
return re.Emit(dagStats) | ||
}, | ||
}, | ||
Encoders: cmds.EncoderMap{ | ||
cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, event *DagStat) error { | ||
_, err := fmt.Fprintf( | ||
w, | ||
"%v\n", | ||
event, | ||
) |
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.
It is... I just wish we had a better way.
|
||
test_expect_success "dag stat of simple IPLD object" ' |
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.
This seems reasonable.
Adds a command to get simple stats out from a DAG (e.g. how big is it and how many blocks does it have).
This PR certainly has room for improvement, but adding this command is probably better than not.