network: Can't have ConnectionState in grpc service #258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instances of NodeService (renamed back from NodeServer) get created for each request by the tower-h2 server. There is not enough information passed to NodeService on creation or with requests to distinguish between different socket connections and manage per-connection state. Which means we need some other way to identify nodes along with subscription requests.
NicolasDP
approved these changes
Apr 5, 2019
mzabaluev
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Making service instances to share a
ConnectionStatewas a mistake. The tower-h2 services are not associated with transport connections and can't currently distinguish between connections from which incoming requests are made. See tower-rs/tower-h2#64 for discussion.Removed
ConnectionState::new_listenas we no longer meaningfully can have that in gRPC.