-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Add ResponseHeadersVariable Parameter to Invoke-RestMethod #4888
Changes from all commits
6bc50d4
bb81e3d
a718058
2e5c81f
0073964
d4b5305
8971595
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,12 @@ internal override void ProcessResponse(HttpResponseMessage response) | |
{ | ||
StreamHelper.SaveStreamToFile(responseStream, QualifiedOutFile, this); | ||
} | ||
|
||
if (!String.IsNullOrEmpty(ResponseHeadersVariable)) | ||
{ | ||
PSVariableIntrinsics vi = SessionState.PSVariable; | ||
vi.Set(ResponseHeadersVariable, WebResponseHelper.GetHeadersDictionary(response)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we return ReadOnlyDictionary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have seen cases where users are taking the returned dictionary, updating it, and then supplying back to further calls. I don't see much benefit. Also, $Res = invoke-webrequest https://google.com
$Res.BaseResponse.Headers.TryAddWithoutValidation('testy','lala')
$Res.BaseResponse.Headers.GetValues('testy') results with As for the returning entire response, that would be somewhat pointless with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree that we should keep complex self-parsing usage to Invoke-WebRequest and target most common easy to use scenarios for Invoke-RestMethod. This seems fine to me. |
||
} | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the guideline is to still use singular
ResponseHeaderVariable
unless the output is always not singular. It also seems that HTTP 1.1 calls the whole thing aResponse Header
and the individual headers as fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose
Headers
because we are using the-Headers
parameter as well asWebResponseObject.Headers
(which this provides parity for) andHttpResponseMessage.Headers
&HttpResponseMessage.Content.Headers
(which the dictionary is generated from). I can change it, but it would be the odd-man-out.If the resulting variable provided a string with the raw Response Header, I would agree. But I think
ResponseHeadersVariable
makes clear that it is collection of some kind.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with it