Skip to content

Conversation

@ManickaP
Copy link
Member

@ManickaP ManickaP commented Jul 13, 2022

Backport of #71783, #71969 and #72031 to release/7.0-preview7

/cc @ManickaP

Customer Impact

Some of the QUIC APIs were finally approved last evening and we were not able to make them in even if the team worked late night.
They are all QUIC changes and not related or dependent on anything else.
This will allow Kestreal to consume the final API shape and we may be able to get at least some public use.

Testing

There are existing System.Net.Quic and System.Net.Http functional tests.

Risk

Low. The whole library is in preview. And HTTP/3 is still not enabled by default.

* Listener comment; PreviewFeature attribute

* Feedback

* QuicConnection new API including compilable implementation

* Fixed logging

* Fixed S.N.Quic and S.N.Http tests

* Options now correspond to the issue

* Feedback

* Comments, PreviewFeature attribute and RemoteCertificate disposal.

* Preview feature attribute is assembly wide

* Some typos

* Fixed test with certificate

* Default values as constants

* Event handlers split into methods called via switch expression.

* Some more comments

* Unified unsafe usage

* Fixed some more tests

* Cleaned up some exceptions and resource strings.

* Feedback

* Latest greatest API proposal.

* Fixed Http solution

* Feedback
@ManickaP ManickaP requested review from CarnaViire, rzikm and wfurt and removed request for rzikm and wfurt July 13, 2022 16:24
@ghost ghost assigned ManickaP Jul 13, 2022
@ghost
Copy link

ghost commented Jul 13, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ManickaP ManickaP requested review from rzikm and wfurt and removed request for CarnaViire July 13, 2022 16:25
@ghost
Copy link

ghost commented Jul 13, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #71783 to release/7.0-preview7

/cc @ManickaP

Customer Impact

Some of the QUIC APIs were finally approved last evening and we were not able to make them in even if the team worked late night.
They are all QUIC changes and not related or dependent on anything else.
This will allow Kestreal to consume the final API shape and we may be able to get at least some public use.

Testing

There are existing System.Net.Quic and System.Net.Http functional tests.

Risk

Low. The whole library is in preview. And HTTP/3 is still not enabled by default.

Author: ManickaP
Assignees: ManickaP
Labels:

new-api-needs-documentation, area-System.Net.Quic

Milestone: -

@ManickaP ManickaP changed the title [QUIC] API QuicConnection (#71783) [QUIC] API QuicConnection (#71783), QuicStream (#71969), QUIC public (#72031) Jul 13, 2022
@ManickaP ManickaP requested a review from CarnaViire July 13, 2022 16:32
ManickaP added 2 commits July 13, 2022 20:03
* Quic stream API surface

* Fixed test compilation

* Fixed http test compilation

* HttpLoopbackConnection Dispose -> DisposeAsync

* QuicStream implementation

* Fixed some tests

* Fixed all QUIC and HTTP tests

* Fixed exception type for stream closed by connection close

* Feedback

* Fixed WebSocket.Client test build

* Feedback, test fixes

* Fixed build on framework and windows

* Fixed winhandler test

* Swap variable based on order in defining class

* Post merge fixes

* Feedback and build

* Reverted connection state to pass around abort error code

* Fixed exception type.
* System.Net.Quic removed from ASP transport package and made part of SDK ref

* Removed manual references to System.Net.Quic.csproj
@ManickaP ManickaP added the Servicing-consider Issue for next servicing release review label Jul 13, 2022
@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 13, 2022
@carlossanlop
Copy link
Contributor

Approved via email. Will merge once the CI passes. cc @ManickaP

Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

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

LGTM

@ManickaP
Copy link
Member Author

The failing test is #71233, unrelated. It's been also noted on the original PRs into main.

@carlossanlop carlossanlop merged commit 060de1c into dotnet:release/7.0-preview7 Jul 13, 2022
@ManickaP ManickaP deleted the mapichov/bp-71783 branch July 14, 2022 16:10
@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants