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

Added a verbose option for swarm peers. #2713

Merged
merged 2 commits into from
May 18, 2016

Conversation

csasarak
Copy link
Contributor

This is used like 'ipfs swarm peers -v'. It prints out the normal peers
information, but also the latency of each peer

License: MIT
Signed-off-by: Chris Sasarak chris.sasarak@gmail.com

This is used like 'ipfs swarm peers -v'. It prints out the normal peers
information, but also the latency of each peer

License: MIT
Signed-off-by: Chris Sasarak <chris.sasarak@gmail.com>
Options: []cmds.Option{
cmds.BoolOption("verbose", "v",
`Also display latency along with peer information in the following form:
<peer address> <latency>
Copy link
Member

Choose a reason for hiding this comment

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

You are using raw string here. Meaning that now this string includes two new lines and also indentation. Please change it to one line.

If you want to split line you can do it like that

"line1" +
"still line1"

@Kubuxu Kubuxu added the kind/support A question or request for support label May 17, 2016
@Kubuxu
Copy link
Member

Kubuxu commented May 17, 2016

@whyrusleeping might be able to respond to your questions better than I am.

If latency isn't stored or exposed easily then it would be worth pinging in batches of few tenths of peers at a time.

@ghost
Copy link

ghost commented May 17, 2016

It might be worth to make pinging a separate command, since it doesn't just make the output more verbose, but actually alters the behaviour.

pContext, cancelFn := context.WithTimeout(req.Context(), timeout)
defer cancelFn()

ch, _ := pingService.Ping(pContext, pid)
Copy link
Member

Choose a reason for hiding this comment

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

We can grab this information from n.Peerstore.LatencyEWMA(peer) no need to ping each peer for it

@csasarak
Copy link
Contributor Author

That's significantly easier than what I was doing, thanks! I'll edit this tonight.

@RichardLitt RichardLitt added need/author-input Needs input from the original author and removed kind/support A question or request for support labels May 17, 2016
License: MIT
Signed-off-by: Chris Sasarak <chris.sasarak@gmail.com>
@csasarak
Copy link
Contributor Author

I've incorporated the changes everyone discussed. I failed one of the tests, but haven't merged in master. Do you all prefer that I squash/rebase my commits, I merge master and push to my branch with a merge commit?

@Kubuxu Kubuxu added need/review Needs a review and removed need/author-input Needs input from the original author labels May 18, 2016
@whyrusleeping
Copy link
Member

@csasarak i can squash the commits here on github. And teamcity always tests the merge commit of your branch and latest master.

@whyrusleeping
Copy link
Member

This LGTM

@whyrusleeping whyrusleeping merged commit 2482dc8 into ipfs:master May 18, 2016
@csasarak csasarak deleted the swarm_verbose branch May 19, 2016 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants