Skip to content

Commit

Permalink
Reimagining status for the module (#253)
Browse files Browse the repository at this point in the history
* Reimagining status for the module

Status for commands was originally added to this module based on my experience with other REST API's where individual commands could easily take 10-20 seconds.

Practical usage has shown that most GitHub requests in reality take under one second.  The additional work that PowerShell has to do in order to display progress to the user can easily make the overall command take 4-6 times longer than its actual execution time.

Therefore, status is being ripped out of this module (for the most part).  `Invoke-GHRestMethod` and `Invoke-SendTelemetryEvent` no longer have bifurcating execution paths based on the value of `$NoStatus`.  Everything runs synchronously now on the command prompt.

* `DefaultNoStatus` has been deprecated.  Its value will be ignored.
* The `NoStatus` switch has not been removed from the module commands in order to avoid a breaking change.  It may be removed in a future update.
* `Invoke-GHRestMethod -ExtendedResult` has been updated to include the next page's number and the total number of pages for the REST request.
* A new configuration value has been added: `MultiRequestProgressThreshold
  `Invoke-GHRestMethodMultipleResult` will display a ProgressBar to the user tracking the number of remaining requests for the overall execution of the requested command based on this threshold value.  It will only display the progress bar if the number of requets needed meets or exceeds this threshold value.  This defaults to 10, and can be disabled with a value of 0.  
  Practical usage has shown that this adds less than a second of additional time to the overall execution of a multi-request command (quite different than the previous status).
* `Wait-JobWithAnimation` has been removed since it's no longer used.

Fixes #247
  • Loading branch information
HowardWolosky committed Jun 30, 2020
1 parent f31d791 commit 2740026
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 482 deletions.
16 changes: 16 additions & 0 deletions GitHubConfiguration.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ function Set-GitHubConfiguration
.PARAMETER DefaultNoStatus
Control if the -NoStatus switch should be passed-in by default to all methods.
The -NoStatus switch has been deprecated. Commands in this module no longer display status
on the console, thus passing in -NoStatus to a command no longer has any impact.
Therefore, the value of this configuration setting also no longer has any impact on
command execution.
.PARAMETER DefaultOwnerName
The owner name that should be used with a command that takes OwnerName as a parameter
Expand Down Expand Up @@ -132,6 +136,14 @@ function Set-GitHubConfiguration
.PARAMETER LogTimeAsUtc
If specified, all times logged will be logged as UTC instead of the local timezone.
.PARAMETER MultiRequestProgressThreshold
Some commands may require sending multiple requests to GitHub. In some situations,
getting the entirety of the request might take 70+ requests occurring over 20+ seconds.
A progress bar will be shown (displaying which sub-request is being executed) if the number
of requests required to complete this command is greater than or equal to this configuration
value.
Set to 0 to disable this feature.
.PARAMETER RetryDelaySeconds
The number of seconds to wait before retrying a command again after receiving a 202 response.
Expand Down Expand Up @@ -212,6 +224,8 @@ function Set-GitHubConfiguration

[switch] $LogTimeAsUtc,

[int] $MultiRequestProgressThreshold,

[int] $RetryDelaySeconds,

[switch] $SuppressNoTokenWarning,
Expand Down Expand Up @@ -296,6 +310,7 @@ function Get-GitHubConfiguration
'LogProcessId',
'LogRequestBody',
'LogTimeAsUtc',
'MultiRequestProgressThreshold',
'RetryDelaySeconds',
'SuppressNoTokenWarning',
'SuppressTelemetryReminder',
Expand Down Expand Up @@ -640,6 +655,7 @@ function Import-GitHubConfiguration
'logProcessId' = $false
'logRequestBody' = $false
'logTimeAsUtc' = $false
'multiRequestProgressThreshold' = 10
'retryDelaySeconds' = 30
'suppressNoTokenWarning' = $false
'suppressTelemetryReminder' = $false
Expand Down

0 comments on commit 2740026

Please sign in to comment.