Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Implement the jsipfs ping command #928

Closed
daviddias opened this issue Jul 23, 2017 · 9 comments
Closed

Implement the jsipfs ping command #928

daviddias opened this issue Jul 23, 2017 · 9 comments
Assignees
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue P3 Low: Not priority right now

Comments

@daviddias
Copy link
Member

Following: https://github.com/ipfs/js-ipfs/pull/925/files

We want to support the same command that go-ipfs supports: jsipfs ping.

From go-ipfs:

» ipfs ping --help
USAGE
  ipfs ping <peer ID>... - Send echo request packets to IPFS hosts.

SYNOPSIS
  ipfs ping [--count=<count> | -n] [--] <peer ID>...

ARGUMENTS

  <peer ID>... - ID of peer to be pinged.

OPTIONS

  -n, --count int - Number of ping messages to send. Default: 10.

DESCRIPTION

  'ipfs ping' is a tool to test sending data to other nodes. It finds nodes
  via the routing system, sends pings, waits for pongs, and prints out round-
  trip latency information.
@daviddias daviddias added exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue labels Jul 23, 2017
@daviddias daviddias added P3 Low: Not priority right now status/deferred Conscious decision to pause or backlog labels Jul 26, 2017
@daviddias daviddias added status/ready Ready to be worked and removed status/deferred Conscious decision to pause or backlog labels Sep 13, 2017
@trendsetter37
Copy link

@diasdavid I can take a stab at this

@daviddias
Copy link
Member Author

Thank you @trendsetter37 🌟

@melvin0008
Copy link

Hey @diasdavid,

Thanks a lot for the IPFS project. I really enjoyed reading about the project and have been blown away with the vision <3.
I would like to start contributing to this project.

I gave this issue a shot and got the ping implementing but had few implementation detail questions

  1. In short I called the ping exposed by libp2p . However I noticed although the documentation says it takes in an option. The underlying implementation doesn't take in options. So would you like me to change the underlying implementation in and help it take options (--count)

  2. I would like to update the curent API for ipfs.ping() from not taking anything to take in key, option and callback. I see that we have two folders one called core and other commands. Since commands is where we would be logging the ping times and average latency. What is expected from the ipfs.ping() in core to return?
    List of times for all ping operations?

Once, these doubts are cleared I would like to submit a PR if needed.

Thanks :)

@daviddias
Copy link
Member Author

Thanks a lot for the IPFS project. I really enjoyed reading about the project and have been blown away with the vision <3.

Thank you ❤️❤️❤️

I would like to start contributing to this project.

YEAAS, Welcome! :)

  1. In short I called the ping exposed by libp2p ...

You should not have to change the implementation. The implementation gives you a event emitter and a stop call. You can get the --count to function with just that.

What is expected from the ipfs.ping() in core to return?

Return the ping object. See usage at https://github.com/libp2p/js-libp2p-ping#usage

image

@melvin0008
Copy link

Return the ping object.

My second question was regarding js-ipfs library. Shouldn't the ipfs.ping() implemented in link take in peerId and options? In the README it indicates there are no parameters.

Just to confirm this ipfs.ping() call would return the ping object and be used by jsipfs ping implementation in commands folder to console the ping results?

@daviddias
Copy link
Member Author

In the README it indicates there are no parameters.

I see where the confusion is from. The README has just a reference to the func, but it is not yet "documented". The func full definition should be added to https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/MISCELLANEOUS.md and then that table of contents should be updated.

Sorry for the confusion. I would love some help in going through all the methods and making sure that docs are up to date. //cc @hacdias

@melvin0008
Copy link

No worries. Just wanted to clear that out. I will send in a PR for the ping implementation and also will update the doc with it. :)

melvin0008 pushed a commit to melvin0008/js-ipfs that referenced this issue Feb 2, 2018
melvin0008 added a commit to melvin0008/js-ipfs that referenced this issue Feb 2, 2018
@daviddias
Copy link
Member Author

Work happening at #1202 :)

JGAntunes pushed a commit to JGAntunes/js-ipfs that referenced this issue Mar 29, 2018
@vasco-santos vasco-santos self-assigned this Apr 5, 2018
@hacdias hacdias added status/in-progress In progress and removed status/ready Ready to be worked labels May 3, 2018
@hacdias
Copy link
Member

hacdias commented May 3, 2018

Work happening on #1299 now.

@JGAntunes JGAntunes mentioned this issue May 3, 2018
2 tasks
@ghost ghost removed the status/in-progress In progress label May 4, 2018
JGAntunes pushed a commit to JGAntunes/js-ipfs that referenced this issue May 6, 2018
daviddias pushed a commit that referenced this issue May 8, 2018
* feat: implement `ipfs ping` flags #928

* feat: first shot at ping implementaion

* fix: ETOOMANYSTREAMS 😢

* chore: cleanup on the ping component

* chore: ping component linting

* chore: bump js-ipfs-api and fix http ping validation

* chore: add test to ping cli command

* chore: add ping cli test

* chore: refactor ping component and some cleanup

* chore: add tests to the ping http API

* fix: no need to check for peerRouting method in ping

* chore: add tests for ping core functionality
daviddias pushed a commit that referenced this issue May 8, 2018
* feat: implement `ipfs ping` flags #928

* feat: first shot at ping implementaion

* fix: ETOOMANYSTREAMS 😢

* chore: cleanup on the ping component

* chore: ping component linting

* chore: bump js-ipfs-api and fix http ping validation

* chore: add test to ping cli command

* chore: add ping cli test

* chore: refactor ping component and some cleanup

* chore: add tests to the ping http API

* fix: no need to check for peerRouting method in ping

* chore: add tests for ping core functionality
alanshaw pushed a commit that referenced this issue May 16, 2018
* feat: implement `ipfs ping` flags #928

* feat: first shot at ping implementaion

* fix: ETOOMANYSTREAMS 😢

* chore: cleanup on the ping component

* chore: ping component linting

* chore: bump js-ipfs-api and fix http ping validation

* chore: add test to ping cli command

* chore: add ping cli test

* chore: refactor ping component and some cleanup

* chore: add tests to the ping http API

* fix: no need to check for peerRouting method in ping

* chore: add tests for ping core functionality
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue P3 Low: Not priority right now
Projects
None yet
Development

No branches or pull requests

5 participants