Skip to content

PowerShellForGitHub: Suggest Disabling ProgressBar when Calling Invoke-WebRequest #227

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

Closed
X-Guardian opened this issue Jun 8, 2020 · 3 comments · Fixed by #229
Closed
Labels
bug This relates to a bug in the existing module. in progress Work on this issue is already underway. Please don't work start new work on it.

Comments

@X-Guardian
Copy link
Contributor

Issue Details

It is a known issue that the speed of Invoke-WebRequest is affected badly by displaying the progress bar, especially for larger transfers in PowerShell 5. See Progress bar can significantly impact cmdlet performance.

It is suggested to disable the progress bar for Invoke-WebRequest to improve the speed of the Pester tests and also general module performance. This would also remove progress bar 'flickering' when using NoStatus.

Steps to reproduce the issue

(Measure-Command { Invoke-Pester .\Tests\GitHubRepositories.tests.ps1 -Show None}).TotalSeconds

Results

PowerShell Version Progress Bar State Run Time in Seconds
5.1 Enabled 62
5.1 Disabled 52
7.0.1 Enabled 56
7.0.1 Disabled 57

Suggested solution to the issue

Add the following lines before both Invoke-WebRequest calls in the Invoke-GHRestMethod function:

$tempProgressPreference = $ProgressPreference
$ProgressPreference = 'SilentlyContinue'

and the following line after both calls:

$progressPreference = $tempProgressPreference

This change has no affect on the Status progress bar.

Requested Assignment

  • If possible, I would like to fix this.

Operating System

OsName               : Microsoft Windows 10 Pro
OsOperatingSystemSKU : 48
OsArchitecture       : 64-bit
WindowsVersion       : 1903
WindowsBuildLabEx    : 18362.1.amd64fre.19h1_release.190318-1202
OsLanguage           : en-GB
OsMuiLanguages       : {en-GB, en-US, th-TH}

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.0.1
PSEdition                      Core
GitCommitId                    7.0.1
OS                             Microsoft Windows 10.0.18362
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Module Version

Running: 0.14.0
Installed: 0.14.0

@X-Guardian X-Guardian added bug This relates to a bug in the existing module. triage needed An issue that needs to be reviewed by a member of the team. labels Jun 8, 2020
@HowardWolosky
Copy link
Member

Interesting. Very interesting. Thanks for the background.

I agree that based on that thread, we should consider a change.

That thread also points out that the issue was fixed in PS 7 (which your repro also confirms), so I don't want all users to lose out on potentially helpful status information, simply because of an issue in the earlier platform.

So...my thought is to add a new configuration property: EnableWebRequestStatus (or something similar). It can default to false. But then users can choose to re-enable it if they so choose (either because they're on PS5 and don't care about the performance hit, or because they're on PS7 and don't have a performance hit to worry about).

The one thing to keep in mind when implementing a config property read within Invoke-GHRestMethod is that you need to pass-in the config property as a parameter to the scriptblock. Don't read it directly within the scriptblock. This is mainly because someone can change the value as -SessionOnly to impact the current PS session, but the scriptblock will actually be a separate session that we need to treat as the same one as far as configurations are concerned.

Approving the suggestion, and marking it as in progress since you want to take it on.

@HowardWolosky HowardWolosky added in progress Work on this issue is already underway. Please don't work start new work on it. and removed triage needed An issue that needs to be reviewed by a member of the team. labels Jun 8, 2020
@X-Guardian
Copy link
Contributor Author

Are you sure that is not over-engineering the solution? If you have Status enabled, you don't even see the Invoke-WebRequest progress, as it is within a job. If you have Status disabled, you didn't want status, so perhaps we shouldn't show any?

@HowardWolosky
Copy link
Member

You raise great points. Agreed. Forget the new config property, and proceed as you initially described. Thanks!

HowardWolosky pushed a commit that referenced this issue Jun 10, 2020
Disables the PowerShell progress bar for the `Invoke-WebRequest` cmdlet calls in the `Invoke-GHRestMethod` and `Invoke-SendTelemetryEvent` functions due to known performance issues in PowerShell 5.1.

Reference: 
[Progress bar can significantly impact cmdlet performance.](PowerShell/PowerShell#2138)

Fixes #227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This relates to a bug in the existing module. in progress Work on this issue is already underway. Please don't work start new work on it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants