.Net: Expose response and response content headers #10065
Conversation
…consumers to access them.
SergeyMenshykh
added
.NET
SergeyMenshykh
had a problem deploying
to
integration
GitHub Actions
Error
SergeyMenshykh
had a problem deploying
to
integration
GitHub Actions
Error
…/SergeyMenshykh/semantic-kernel into allow-access-to-response-headers
SergeyMenshykh
temporarily deployed
to
integration
GitHub Actions
Inactive
| /// A dictionary containing the headers of the HTTP response message, including content headers. | ||
| /// The dictionary keys are header names, and the values are collections of header values. | ||
| /// </returns> | ||
| private static Dictionary<string, IEnumerable<string>>? GetResponseHeaders(HttpResponseMessage responseMessage) |
There was a problem hiding this comment.
May consider a ReadOnlyDictionary or the original HttpResponseHeaders if it is accessible?
| private static Dictionary<string, IEnumerable<string>>? GetResponseHeaders(HttpResponseMessage responseMessage) | |
| private static ReadOnlyDictionary<string, IEnumerable<string>>? GetResponseHeaders(HttpResponseMessage responseMessage) |
There was a problem hiding this comment.
Agreed, is there a reason for this dictionary to be mutable?
There was a problem hiding this comment.
Having it mutable will allow users to add new headers or remove existing ones if needed.
| /// </summary> | ||
| [Experimental("SKEXP0040")] | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] | ||
| public IDictionary<string, IEnumerable<string>>? Headers { get; set; } |
There was a problem hiding this comment.
| public IDictionary<string, IEnumerable<string>>? Headers { get; set; } | |
| public IReadOnlyDictionary<string, IEnumerable<string>>? Headers { get; set; } |
There was a problem hiding this comment.
It should be possible to remove an existing header from the dictionary in auto function invocation filters or add a new one if needed.
There was a problem hiding this comment.
For response? Not sure if I'm following, isn't this after the effect (request)?
There was a problem hiding this comment.
Yep, in response, yes, after the effect. The auto function invocation filters allow you to mutate the function invocation result in the way that is required for your scenario. Some headers may not need to be sent to the AI model, some may need to be added if they were not in the response, and some may need to be normalized or sanitized, etc.
| // Add the content headers as well. | ||
| foreach (var item_ in responseMessage.Content.Headers) | ||
| { | ||
| headers[item_.Key] = item_.Value; |
There was a problem hiding this comment.
Can we have a conflict with the key names?
There was a problem hiding this comment.
Technically, it's possible, but it should not be the case because response and content represent different types of headers that should not have the same names. For the unusual scenarios where it might be the case, we can introduce RestApiOperationResponseResultFactory that will have access to the HttpResponseMessage and will allow customization of functionality for reading headers.
| /// A dictionary containing the headers of the HTTP response message, including content headers. | ||
| /// The dictionary keys are header names, and the values are collections of header values. | ||
| /// </returns> | ||
| private static Dictionary<string, IEnumerable<string>>? GetResponseHeaders(HttpResponseMessage responseMessage) |
There was a problem hiding this comment.
Agreed, is there a reason for this dictionary to be mutable?
RogerBarreto
temporarily deployed
to
integration
GitHub Actions
Inactive
Motivation, Context and Description
Today, it's difficult to access response and response content headers returned by OpenAPI plugins. This PR exposes the
Headersproperty, allowing easy access to both response and response content headers.Closes - #9986