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 python to REQUIREMENTS.md #65

Merged
merged 5 commits into from Feb 12, 2019
Merged

Add python to REQUIREMENTS.md #65

merged 5 commits into from Feb 12, 2019

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Jan 31, 2019

The python implementation can be found here :
github.com/zixuanzh/py-libp2p

@ghost
Copy link

ghost commented Feb 4, 2019

@zixuanzh This LGTM, but you're in a better position than me to double check for accuracy relative to the current state of py-libpo2p.

@ghost ghost requested a review from zixuanzh February 4, 2019 16:49
Copy link

@zixuanzh zixuanzh left a comment

Choose a reason for hiding this comment

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

@Jorropo Thanks for the PR, features status looks accurate. We also have multistream-select implemented for protocol muxing if you want to include that for other clients as well. Can you also add a link to py-libp2p repo in the main readme section here? Thanks

CC @mgoelzer

@Jorropo
Copy link
Contributor Author

Jorropo commented Feb 11, 2019

@zixuanzh what do you mean by :

We also have multistream-select implemented for protocol muxing

I don't find multisteam-select in https://github.com/zixuanzh/py-libp2p/tree/master/libp2p/stream_muxer
Only mplex and yamux (yamux only contain an empty init (reason for red))
And mplex is yellow
https://github.com/libp2p/libp2p/pull/65/files#diff-c989be102856fc7df91ef19f04516e27R60
should I switch it to green ?
If I not checked it and it should be, please confirm that he isn't in the current list pls (not present in an other name).

Can you also add a link to py-libp2p repo in the main readme section here? Thanks

Done ! 😄

This is to according to change done in libp2p/py-libp2p#117
@zixuanzh
Copy link

@Jorropo we implemented multistream-select as protocol-muxer as we find that name more descriptive. mplex should still be lemon since we are going through some fixes right now. We should roll out the changes soon. Thanks!

README.md Outdated
@@ -30,6 +30,7 @@ Meanwhile, learn more about libp2p at [**libp2p.io**](https://libp2p.io)
- [go-libp2p](//github.com/libp2p/go-libp2p) in Go
- [js-libp2p](//github.com/libp2p/js-libp2p) in Javascript, for Node and the Browser
- [rust-libp2p](//github.com/libp2p/rust-libp2p) in Rust
- [py-libp2p](//github.com/zixuanzh/py-libp2p) in Python 3

Choose a reason for hiding this comment

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

let's specify Python 3.7 to avoid confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May I should precise async, this is for me important to know if the lib support that or not ?

Choose a reason for hiding this comment

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

Are you talking about asyncio? I think specifying 3.7 should be sufficient.

Choose a reason for hiding this comment

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

@Jorropo we are moving into libp2p org today, so lets use //github.com/libp2p/py-libp2p instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For async I think you are right. That made it unclear.

@Jorropo
Copy link
Contributor Author

Jorropo commented Feb 12, 2019

@zixuanzh I'm felling very idiot because I don't understand what you are talking about, that surely obvious but I can't get it.

In REQUIREMENT.md I find these category :

  • libp2p Node
  • Identify Protocol
  • Transport Protocols
  • Stream Muxers
  • Switch
  • NAT Traversal
  • Peer Discovery
  • Content Routing
  • Peer Routing
  • Exchange
  • Consensus

And protocol muxer isn't in.
But I find mplex in Stream Muxers named multiplex.

So please can you help me to catch it ?
Should I add a protocol muxer section ? (If yes why python need it and all other implementation seems to work without ?)

@robzajac
Copy link

Hi @Jorropo, indeed multistream-select and our implementation as protocol-muxing is not a typical libp2p module. As you can tell by this PR in py-libp2p, it will eventually be factored out. Note that multistream is also implemented in Go, and seems to be an idea from multiformats instead of libp2p.

I think it is okay to leave it out of the REQUIREMENTS, and we may eventually remove it ourselves too. So this PR looks ready to me.

@zixuanzh
Copy link

@Jorropo sorry for the confusion, @robzajac is correct. Stream Muxers allow two nodes to agree on a stream id to communicate on and Protocol Muxers/Multistreams allow two nodes to agree on a protocol for a particular stream. Let's change the link in README since the repo was just transferred today and it should be good to go. Thank you!

@Jorropo
Copy link
Contributor Author

Jorropo commented Feb 12, 2019

@robzajac ok thx a lot.

@zixuanzh np, the goal of protocol and stream muxer was clear for me 😉, for me protocol muxer was a very new functionality implemented by python only and maybe planned in other implementation.

@ghost ghost assigned robzajac Feb 12, 2019
@ghost ghost added the in progress label Feb 12, 2019
@robzajac robzajac merged commit 38f5639 into libp2p:master Feb 12, 2019
@ghost ghost removed the in progress label Feb 12, 2019
@Jorropo Jorropo deleted the addPython branch February 12, 2019 22:36
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