Add HttpVersion parameter to Invoke-WebRequest cmdlet by hayhay27 · Pull Request #15853 · PowerShell/PowerShell

Maksim Ivanyuk (hayhay27) · 2021-08-01T13:33:50Z

PR Summary

Add HttpVersionCompletionsAttribute with completions 1.0, 1.1, 2.0, 3.0
Handle input conversions from string, int and double to Version (generalize ArgumentToVersionTransformationAttribute from Set-StrictMode)
Enrich Verbose mesasge for request with http version and uri
Add pester tests for HttpVersion completions
Add xUnit tests for ArgumentToVersionTransformationAttribute

PR Context

This PR addressed to issue #12641
At the moment impossible make requests to network resources served only with HTTP/2 cause of default http version in HttpRequestMessage.Version=1.1

Changes in this PR allow to set http version during invocation Invoke-RestMethod or Invoke-WebRequest

This is my first PR to PowerShell. I have a couple of questions about tests. How and where to implement (at least point me right place please).

PR Checklist

Deleted user (ghost) · 2021-08-01T13:34:03Z

All CLA requirements met.

Ilya (iSazonov)

As for test, I think we have no need to do real HTTP requests with specified version since we utilize .Net Runtime features. I think it would enough if you did manual checks and shared results.
Nevertheless, we must verify that (1) the parameter accepts well-know values without throw, (2) transformation attribute works (it could be xUnit tests), (3) tab-completion works.

Maksim Ivanyuk (hayhay27) · 2021-08-01T20:37:56Z

As for test, I think we have no need to do real HTTP requests with specified version since we utilize .Net Runtime features. I think it would enough if you did manual checks and shared results.
Nevertheless, we must verify that (1) the parameter accepts well-know values without throw, (2) transformation attribute works (it could be xUnit tests), (3) tab-completion works.

do you mean a pester test?
done with xunit
done with xunit

Maksim Ivanyuk (hayhay27) · 2021-08-25T07:41:20Z

I have no idea about failed tests )
What is that? )

Ilya (iSazonov) · 2021-08-27T05:13:54Z

Maksim Ivanyuk (@hayhay27) Please update the PR description to address latest code changes.

Maksim Ivanyuk (hayhay27)

Please update the PR description to address latest code changes.

done

Maksim Ivanyuk (hayhay27) · 2021-09-02T13:20:20Z

while we keep silence

I thought with your blessing I can update docs for this PR

add pester tests

skip tests for HttpVersion on mac and linux

fix code smells

fixes by review - remove HttpVersionUtils - refactor pester test

Maksim Ivanyuk (hayhay27) · 2021-10-08T17:00:12Z

Rob Holt (@rjmholt) Thanks a lot!
I applied all your comments

Dongbo Wang (daxian-dbw) · 2021-10-15T22:18:09Z

Rob Holt (@rjmholt) Can you please take another look?

fixes by review - extend version transformation via method override instead of ctor injection - clean up code

pull-request-quantifier-deprecated · 2021-11-02T21:30:07Z

This PR has 254 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Large
Size       : +186 -68
Percentile : 65.4%

Total files changed: 10

Change summary by file extension:
.cs : +145 -66
.resx : +2 -2
.ps1 : +39 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Dongbo Wang (daxian-dbw)

LGTM

Dongbo Wang (daxian-dbw) · 2021-11-02T23:54:18Z

Maksim Ivanyuk (@hayhay27) Thank you for your contribution!

Deleted user (ghost) · 2021-12-16T19:59:42Z

🎉v7.3.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

ghost assigned Dongbo Wang (daxian-dbw) Aug 1, 2021

Ilya (iSazonov) reviewed Aug 1, 2021

View reviewed changes

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Aug 1, 2021

Ilya (iSazonov) reviewed Aug 2, 2021

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Aug 5, 2021

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch from f2c9643 to 5cd00b7 Compare August 9, 2021 08:13

Ilya (iSazonov) reviewed Aug 9, 2021

View reviewed changes

Comment thread src/Microsoft.PowerShell.Commands.Utility/resources/WebCmdletStrings.resx Outdated

Ilya (iSazonov) reviewed Aug 9, 2021

View reviewed changes

Comment thread ...rosoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/HttpVersionCompleter.cs Outdated

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 19, 2021

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch from 6fd9a05 to cb4cb12 Compare August 21, 2021 11:25

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 21, 2021

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch from 4645764 to 7897ad8 Compare August 21, 2021 12:06

Maksim Ivanyuk (hayhay27) changed the title ~~WIP: Add HttpVersion parameter to Invoke-WebRequest cmdlet~~ Add HttpVersion parameter to Invoke-WebRequest cmdlet Aug 21, 2021

Ilya (iSazonov) reviewed Aug 23, 2021

View reviewed changes

Comment thread src/System.Management.Automation/engine/ArgumentToVersionTransformationAttribute.cs Outdated

Comment thread test/xUnit/csharp/test_ArgumentToVersionTransformationAttribute.cs Outdated

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch from 7897ad8 to 753ab24 Compare August 25, 2021 06:58

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch 2 times, most recently from d3a3a78 to 84e6695 Compare August 25, 2021 17:47

Maksim Ivanyuk (hayhay27) marked this pull request as ready for review August 25, 2021 18:19

Maksim Ivanyuk (hayhay27) requested a review from Ilya (iSazonov) August 26, 2021 16:03

Dongbo Wang (daxian-dbw) requested review from James Truher (JamesWTruher), Aditya Patwardhan (adityapatwardhan) and Rob Holt (rjmholt) August 27, 2021 04:48

Maksim Ivanyuk (hayhay27) commented Aug 29, 2021

View reviewed changes

Comment thread test/xUnit/csharp/test_ArgumentToVersionTransformationAttribute.cs Outdated

Ilya (iSazonov) approved these changes Aug 30, 2021

View reviewed changes

Maksim Ivanyuk (hayhay27) added 3 commits October 8, 2021 19:10

Add HttpVersion parameter to Invoke-WebRequest

546ca85

add pester tests

Add HttpVersion parameter to Invoke-WebRequest

2bca829

skip tests for HttpVersion on mac and linux

Add HttpVersion parameter to Invoke-WebRequest

83ce173

fix code smells

Maksim Ivanyuk (hayhay27) force-pushed the add-http-version-parameter branch from 04bcb4f to 83ce173 Compare October 8, 2021 16:10

Add HttpVersion parameter to Invoke-WebRequest

27b46b3

fixes by review - remove HttpVersionUtils - refactor pester test

Maksim Ivanyuk (hayhay27) requested a review from Rob Holt (rjmholt) October 8, 2021 17:00

Dongbo Wang (daxian-dbw) requested changes Nov 2, 2021

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Nov 2, 2021

Add HttpVersion parameter to Invoke-WebRequest

2fb1df8

fixes by review - extend version transformation via method override instead of ctor injection - clean up code

pull-request-quantifier-deprecated Bot added the Large label Nov 2, 2021

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Nov 2, 2021

Maksim Ivanyuk (hayhay27) requested review from Dongbo Wang (daxian-dbw) and removed request for Rob Holt (rjmholt) November 2, 2021 21:35

Dongbo Wang (daxian-dbw) approved these changes Nov 2, 2021

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit 674ef88 into PowerShell:master Nov 2, 2021

Steve Lee (SteveL-MSFT) mentioned this pull request Nov 18, 2021

Add [ArgumentToPSVersionTransformationAttribute] to list of patterns to remove for generated ref assembly #16489

Merged

22 tasks

ghost mentioned this pull request Dec 16, 2021

invoke-restmethod does not support http/2 #12641

Closed

Kevin Locke (kevinoid) mentioned this pull request Dec 28, 2021

Remove URI from Invoke-RestMethod's Verbose Output (Optionally) #14417

Closed

TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this pull request Jan 19, 2022

Add -HttpVersion parameter to web cmdlets (PowerShell#15853)

cc27f06

Dongbo Wang (daxian-dbw) added a commit to daxian-dbw/PowerShell that referenced this pull request May 16, 2022

Redo the change that was reverted by PowerShell#15853

cd1f770

Dongbo Wang (daxian-dbw) mentioned this pull request May 16, 2022

Redo the change that was reverted by #15853 #17357

Merged

7 tasks

Travis Plunk (TravisEz13) pushed a commit that referenced this pull request May 17, 2022

Redo the change that was reverted by #15853 (#17357)

391e080

Ryan Yates (kilasuit) removed the WG-Cmdlets-Utility label Jul 17, 2026

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

Add -HttpVersion parameter to web cmdlets (PowerShell#15853)

388fa9f

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

Redo the change that was reverted by PowerShell#15853 (PowerShell#17357)

a7b8e05

Uh oh!

Conversation

Maksim Ivanyuk (hayhay27) commented Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Deleted user (ghost) commented Aug 1, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Maksim Ivanyuk (hayhay27) commented Aug 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Maksim Ivanyuk (hayhay27) commented Aug 25, 2021

Uh oh!

Ilya (iSazonov) commented Aug 27, 2021

Uh oh!

Maksim Ivanyuk (hayhay27) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Maksim Ivanyuk (hayhay27) commented Sep 2, 2021

Uh oh!

Maksim Ivanyuk (hayhay27) commented Oct 8, 2021

Uh oh!

Dongbo Wang (daxian-dbw) commented Oct 15, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pull-request-quantifier-deprecated Bot commented Nov 2, 2021

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) commented Nov 2, 2021

Uh oh!

Deleted user (ghost) commented Dec 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Maksim Ivanyuk (hayhay27) commented Aug 1, 2021 •

edited

Loading

Deleted user (ghost) commented Aug 1, 2021 •

edited by ghost

Loading

Maksim Ivanyuk (hayhay27) left a comment •

edited

Loading