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

Simplify upgrader components? #313

Open
ralexstokes opened this issue Sep 24, 2019 · 1 comment
Open

Simplify upgrader components? #313

ralexstokes opened this issue Sep 24, 2019 · 1 comment
Labels
discussion issue is meant to serve as a venue for discussion

Comments

@ralexstokes
Copy link
Contributor

The code in muxer_multistream.py and security_multistream.py is pretty duplicative and I think we will have less burden of maintenance over time if we consider consolidating the use of multistream-select in each one.

I opened this issue as a discussion: the decision to make is whether the specialization in each of the above classes warrants its own class, or if we can simply have two instances of Multiselect in the TransportUpgrader. I'm currently of the opinion that the specialization in each class does not hold its own weight so we should remove them; the only thing really make this up for debate is how we handle the inbound/outbound sense of the action and the fact that we work w/ particular abstractions (in particular, MuxerMultistream.new_conn takes an ISecureConn -- can we just provide an io.ReadWriteCloser instead?).

@ralexstokes ralexstokes added the discussion issue is meant to serve as a venue for discussion label Sep 24, 2019
@mhchia
Copy link
Contributor

mhchia commented Sep 26, 2019

I think inlining muxer_multistream and security_multistream in TransportUpgrader is doable, though I guess we will lose some extent of abstraction. SecurityMultistream, which implements SecureTransport, seems to be the "Upgrader" which uses "Multistream-select" as the negotiation protocol. MuxerMultistream is similar as well. I think it's fine to remove them If we don't have protocols other than "Multistream-select" for protocol negotiation to upgrade connections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion issue is meant to serve as a venue for discussion
Projects
Status: No status
Development

No branches or pull requests

2 participants