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

[API Proposal]: [QUIC] Make CompleteWritesAsync #103434

Closed
ManickaP opened this issue Jun 13, 2024 · 4 comments
Closed

[API Proposal]: [QUIC] Make CompleteWritesAsync #103434

ManickaP opened this issue Jun 13, 2024 · 4 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic
Milestone

Comments

@ManickaP
Copy link
Member

ManickaP commented Jun 13, 2024

Background and motivation

System.Net.Quic is still in Preview, before we un-preview it, we'd like to change this method to an async one.
https://learn.microsoft.com/en-us/dotnet/api/system.net.quic.quicstream.completewrites?view=net-8.0

API Proposal

namespace System.Net.Quic;

// Existing class.
public class QuicStream : Stream
{
-    void CompleteWrites();
+    ValueTask CompleteWritesAsync();
}

API Usage

await using var quicStream = await connection.OpenStreamAsync(...);
// Use the stream...
await quicStream.CompleteWritesAsync();

Alternative Designs

Keeping it not-async and use existing Task WritesClosed property.

Risks

No response

@ManickaP ManickaP added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 13, 2024
@ManickaP ManickaP added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Jun 13, 2024
Copy link
Contributor

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

@ManickaP ManickaP added this to the 9.0.0 milestone Jun 13, 2024
@MihaZupan
Copy link
Member

Alternative 2: Delete the method? 😆

@bartonjs
Copy link
Member

bartonjs commented Jun 13, 2024

Video

  • There were questions as to why the WritesClosed property needs to still exist, but there is a legitimate scenario.
namespace System.Net.Quic;

// Existing class.
public class QuicStream : Stream
{
-    public void CompleteWrites();
+    public ValueTask CompleteWritesAsync();
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 13, 2024
@ManickaP ManickaP self-assigned this Jun 13, 2024
@ManickaP
Copy link
Member Author

ManickaP commented Jul 2, 2024

Closing as we decided to keep the API as-is.

@ManickaP ManickaP closed this as completed Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants