Add CreateMessage method to the pipeline #7636
Conversation
| @@ -60,5 +60,76 @@ public void SetProperty(string name, object value) | |||
|
|
|||
| _properties[name] = value; | |||
| } | |||
|
|
|||
| public Stream? PreserveResponseContent() | |||
There was a problem hiding this comment.
The name PreserveResponseContent makes me think this should be a bool. I'm not sure of a better name though. AcquireResponseContentOwnership? GetAndLockResponseContent? ClaimResponseContent? Maybe @KrzysztofCwalina has an idea for this one.
There was a problem hiding this comment.
ExtractResponseContent?
|
|
||
| private static Exception CreateException() | ||
| { | ||
| return new InvalidOperationException("This operation returns Stream as part of the model type. It should be used instead of the response content stream."); |
There was a problem hiding this comment.
I worry about users understanding this error message because "model type" isn't as obvious to new users. Maybe we say this operation has called PreserveResponseContent and will provide the stream as part of its response type? Then they can quickly search for PreserveResponseContent in the docs and we have a chance to make them read a few sentences and see some examples.
| throw CreateException(); | ||
| } | ||
|
|
||
| public override bool CanRead => throw CreateException(); |
There was a problem hiding this comment.
Nit - maybe do this for all the methods too?
There was a problem hiding this comment.
I think others would eventually call into one of these ones
| _response?.Dispose(); | ||
| } | ||
|
|
||
| private class ResponseShouldNotBeUsedStream: Stream |
There was a problem hiding this comment.
Would it be easier to modify Response.ContentStream's getter so you see the error as soon as you touch it instead of creating a Stream that you might hand off somewhere else for processing?
There was a problem hiding this comment.
The response is abstract so every transport would have to implement this. Not sure if we want to go in that direction.
| @@ -440,6 +440,7 @@ directive: | |||
| transform: > | |||
| $.get.responses["200"]["x-az-response-name"] = "FlattenedDownloadProperties"; | |||
| $.get.responses["200"]["x-az-public"] = false; | |||
| $.get.responses["200"]["x-az-stream"] = true; | |||
There was a problem hiding this comment.
Nit - it's probably good to put this on the 206 response as well because if the order ever gets shuffled, it'd be good if any success type that gets merged together has the flag set.
Fixes: #7519
Breaking change
SendAsync with
bufferResponseparameter is removed, CreateMessage should be used for advanced scenarios.Before:
After: