Generalize Http2Cat #17438
Generalize Http2Cat #17438
Conversation
Tratcher
added
the
api-suggestion
Tratcher
force-pushed
the
tratcher/h2cat
branch
from
7d4203e to
d88639f
Compare
| @@ -26,3 +26,13 @@ public partial class SocketTransportOptions | |||
| public bool NoDelay { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } | |||
| } | |||
| } | |||
| namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Client | |||
| { | |||
| public partial class SocketConnectionFactory : Microsoft.AspNetCore.Connections.IConnectionFactory, System.IAsyncDisposable | |||
There was a problem hiding this comment.
I'm fine with making this public in 5.0 for now, but is there any other option besides this? Does InternalsVisibleTo not work here?
There was a problem hiding this comment.
InternalsVisibleTo isn't really an option when we need to consume this across several projects, it doesn't scale. Stephen said they wanted to make this public soon anyways.
There was a problem hiding this comment.
I know we wanted to improve this API to take in a propertybag/featurecollection. That may be something we want to consider before making this public?
There was a problem hiding this comment.
We'll run it by API review Monday and see. If it's just additions then there's no need to block this PR.
There was a problem hiding this comment.
Scratch that, Servers aren't subject to API review yet. I'll check the current API with @halter73 and file any follow up issues. We have the entire release to work out the API surface and I want to get this in to unblock HttpSys feature development.
There was a problem hiding this comment.
Given the namespace soup here (that it's deep within .Kestrel.Transport.Sockets.Client) I feel OK making this public since it won't collide with a more "real" client factory.
Tratcher
force-pushed
the
tratcher/h2cat
branch
from
09b8332 to
1c20ce4
Compare
Tratcher
removed
the
api-suggestion
amcasey
added
area-networking
#14894 This is going to be fun.
We want to re-use Kestrel's low level Http2 test infrastructure to test HttpSys and other servers. Stephen already got it partially separated into a sample project, but that relied heavily on InternalsVisibleTo. This change takes the next step change from InternalsVisibleTo to shared src files. Moving content to the repo Shared folder gives us a clear separation of what's shared and why.
There are now three types of shared src:
There is one API that did need to become public: The client SocketConnectionFactory.
I've included two new HttpSys tests for the 3.1 GoAway feature. I even found a bug in the process #17420.
I did add some more OS version test infrastructure (dotnet/extensions#2715) that needs a dependency update, but that can happen later because the affected test is already skipped due to a product bug.