Update Microsoft Store service to allow callers to set their own package install timeout #3897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bbonaby
requested review from
dhoehna,
huzaifa-d,
vineeththomasalex,
krschau,
sshilov7 and
AmelBawa-msft
dhoehna
approved these changes
Sep 24, 2024
krschau
reviewed
Sep 25, 2024
| @@ -82,17 +81,11 @@ public async Task<bool> TryInstallPackageAsync(string packageId) | |||
| { | |||
| var installTask = InstallPackageAsync(packageId); | |||
|
|
|||
| // Wait for a maximum of StoreInstallTimeout (60 seconds). | |||
| var completedTask = await Task.WhenAny(installTask, Task.Delay(_storeInstallTimeout)); | |||
| await installTask; | |||
There was a problem hiding this comment.
Could this go forever? Is there a benefit to removing the timeout vs making it longer?
There was a problem hiding this comment.
Yeah timeout sounds good, especially since user can't cancel the operation.
There was a problem hiding this comment.
Interestingly, everywhere I looked in ADO I didn't see anyone using a timeout but I'm cool with having one. What I did was just creat a new method that allowed callers to pass in their own timeout. Then moved the function body (everything within the try) into that method. Then called it from within TryInstallPackageAsync. I did this to allow the exception to bubble up to callers if they would like that behavior e.g the wslExtension would.
bbonaby
changed the title
krschau
approved these changes
Sep 30, 2024
huzaifa-msft
approved these changes
Sep 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the pull request
Now that the Wsl extension uses the Microsoft store service in Dev Home to download and install Wsl distributions. I noticed from some of the telemetry and a failure I was getting that the Microsoft Store service has an arbitrary timeout of 1 minute. A download and install of an app could take longer than a minute especially if there are other updates happening that makes the download go to the pending state.
The Microsoft Store Service in Dev Home uses this
References and relevant issues
Detailed description of the pull request / Additional comments
Validation steps performed
PR checklist