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

improve the documentation for the p2p feature #4894

Closed
wants to merge 1 commit into from

Conversation

Stebalien
Copy link
Member

No description provided.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien requested a review from Kubuxu as a code owner March 29, 2018 06:52
@ghost ghost assigned Stebalien Mar 29, 2018
@ghost ghost added the status/in-progress In progress label Mar 29, 2018
Copy link
Member

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

LGTM,

One thing to note is that when you p2p stream dial, the listening endpoint is one-shoot (unlike ssh's -L). We might want to change this at some point (either by --keep flag or by just changing default behavior)

@Stebalien
Copy link
Member Author

One thing to note is that when you p2p stream dial, the listening endpoint is one-shoot (unlike ssh's -L).

Are you sure? It looks like listen dials the listener for every inbound stream (although it looks like there are many obvious bugs...).

@magik6k
Copy link
Member

magik6k commented Mar 29, 2018

I meant the listening endpoint on the dialer side (the one on client side) - https://github.com/ipfs/go-ipfs/blob/master/p2p/p2p.go#L87.

The code is messy/weird there, I'll revisit it eventually to fix some of this stuff (likely when I'll get to creating coreapi for it).

@Stebalien
Copy link
Member Author

I meant the listening endpoint on the dialer side (the one on client side) - https://github.com/ipfs/go-ipfs/blob/master/p2p/p2p.go#L87.

Ah... I see. Really, we should support an HTTP upgrade from a command (although supporting both makes sense).

@Stebalien Stebalien mentioned this pull request Mar 29, 2018
@Stebalien
Copy link
Member Author

We should make it work like ssh -L (that's what users likely expect)`. Blocked on #4895.

@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Mar 29, 2018
@whyrusleeping
Copy link
Member

@Stebalien is this unblocked yet?

cc @magik6k

@magik6k
Copy link
Member

magik6k commented Jun 13, 2018

I've cherry-picked this in #4929 and adjusted to changes there, so if the refactor pr is going to be merged soon I'd say this can be closed.

@Stebalien Stebalien closed this Jun 13, 2018
@ghost ghost removed the status/in-progress In progress label Jun 13, 2018
@Stebalien Stebalien deleted the feat/improve-p2p-docs branch February 28, 2019 22:29
@Stebalien Stebalien restored the feat/improve-p2p-docs branch May 30, 2019 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/blocked Unable to be worked further until needs are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants