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

Add back simple, optional keep-alive to libp2p-ping. #1088

Merged
merged 1 commit into from Apr 25, 2019

Conversation

romanb
Copy link
Contributor

@romanb romanb commented Apr 25, 2019

Admittedly a bit of a back-and-forth, but this is now a very simple option serving multiple purposes:

  • It allows for stable (integration) tests involving a Swarm, which
    are otherwise subject to race conditions due to the connection being
    allowed to terminate at any time with KeepAlive::No
    (which remains the default).

  • It makes for a more entertaining ping example which continuously
    sends pings.

  • Maybe someone wants to use the ping protocol for application-layer
    connection keep-alive after all.

See also the discussion in #1086.

This is now a very simple option serving multiple purposes:

  * It allows for stable (integration) tests involving a Swarm, which
    are otherwise subject to race conditions due to the connection being
    allowed to terminate at any time with `KeepAlive::No`
    (which remains the default).

  * It makes for a more entertaining ping example which continuously
    sends pings.

  * Maybe someone wants to use the ping protocol for application-layer
    connection keep-alive after all.
@ghost ghost assigned romanb Apr 25, 2019
@ghost ghost added the in progress label Apr 25, 2019
@romanb romanb merged commit 9a525d5 into libp2p:master Apr 25, 2019
@romanb romanb deleted the ping-opt-keepalive branch April 25, 2019 08:34
@ghost ghost removed the in progress label Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants