System.ClientModel: Provide APIs for message types to transfer dispose ownership of response#41764
Merged
annelo-msft merged 17 commits intoAzure:mainfrom Feb 7, 2024
Merged
Conversation
Collaborator
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
christothes
reviewed
Feb 5, 2024
christothes
reviewed
Feb 5, 2024
christothes
reviewed
Feb 5, 2024
christothes
reviewed
Feb 5, 2024
KrzysztofCwalina
approved these changes
Feb 7, 2024
This file contains hidden or 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
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.
Unblock generator work to generate protocol methods for streaming APIs where message.BufferResponse is set to false. This addresses a problem described by @AlexanderSher in https://gist.github.com/AlexanderSher/df22ed21acaa938a4371c188f0699b11.
To the end-user who calls a protocol method, what happens in the message's Dispose function is an implementation detail they're not aware of. We need to be able to hide this detail and allow the end-user to dispose the network stream by disposing Response -- i.e. we must be able to return an un-disposed Response to the end-user.
This PR adds an ExtractResponse method to PipelineMessage in ClientModel. When the Azure.Core/ClientModel integration occurs, this method will move to Azure.Core's HttpMessage as well, which will newslot it to return Response instead of PipelineResponse.
Protocol methods that implement streaming APIs must now:
Convenience methods that call into these protocol methods should call ExtractResponse on the message if they are able to return the network stream to the end-user such that the end-user is able to dispose it. If the convenience method cannot pass dispose ownership to someone else (e.g. because it throws an exception or does not return the network stream because of the response status code), it should dispose the response itself.