Skip to content

Adopt Bedrock Client abstractions in SignalR client #10872

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

Closed
analogrelay opened this issue Jun 5, 2019 · 3 comments
Closed

Adopt Bedrock Client abstractions in SignalR client #10872

analogrelay opened this issue Jun 5, 2019 · 3 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions area-signalr Includes: SignalR clients and servers 🥌 Bedrock Done This issue has been fixed

Comments

@analogrelay
Copy link
Contributor

Epic #10869

SignalR currently uses the "Bedrock" abstractions on the server side (ConnectionContext, etc.) but has a custom client-side abstraction (IConnectionFactory). Bedrock has evolved to support a client-side abstraction now so SignalR should update to use it (since it would be a breaking change).

The new client abstraction shape is:

public interface IConnectionFactory
{
    ValueTask<ConnectionContext> ConnectAsync(System.Net.EndPoint endpoint, CancellationToken cancellationToken = default);
}

Some elements to this:

  • Add IConnectionFactory (as above) somewhere (Connections.Abstractions? or a new library?)
  • Add HttpEndpont sub-class of System.Net.Endpoint (for use in the IConnectionFactory.ConnectAsync method).
  • Configure the TransferFormat in the constructor instead of in the ConnectAsync method?
  • Delete existing connection factory and change to use the new abstraction.
@analogrelay
Copy link
Contributor Author

TBH, this is another thing making me nervous :).

cc @davidfowl to review my assessment of the elements needed to finish this.

@analogrelay analogrelay added the area-signalr Includes: SignalR clients and servers label Jun 5, 2019
@davidfowl
Copy link
Member

This looks good. We need to rename the interface to something that says client.

@analogrelay analogrelay added this to the 3.0.0-preview7 milestone Jun 11, 2019
@davidfowl davidfowl added Epic Groups multiple user stories. Can be grouped under a theme. 🥌 Bedrock and removed Epic Groups multiple user stories. Can be grouped under a theme. labels Jun 23, 2019
@halter73 halter73 added Done This issue has been fixed and removed Working labels Jun 28, 2019
@analogrelay
Copy link
Contributor Author

A man standing in a mirror, we see his reflection. He raises his hand in a thumbs-up gesture and says "We did it"

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions area-signalr Includes: SignalR clients and servers 🥌 Bedrock Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

4 participants