-
Notifications
You must be signed in to change notification settings - Fork 856
Fix .NET SDK CLI download timeout #493
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
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -48,6 +48,13 @@ | |||||
| <CopilotNpmRegistryUrl Condition="'$(CopilotNpmRegistryUrl)' == ''">https://registry.npmjs.org</CopilotNpmRegistryUrl> | ||||||
| </PropertyGroup> | ||||||
|
|
||||||
| <!-- Timeout in seconds for downloading the Copilot CLI tarball. | ||||||
| Override CopilotCliDownloadTimeout in your .csproj or Directory.Build.props if needed. | ||||||
| Default is 600 seconds (10 minutes) to handle slow or unreliable network conditions. --> | ||||||
| <PropertyGroup> | ||||||
| <CopilotCliDownloadTimeout Condition="'$(CopilotCliDownloadTimeout)' == ''">600</CopilotCliDownloadTimeout> | ||||||
| </PropertyGroup> | ||||||
|
|
||||||
| <!-- Download and extract CLI binary --> | ||||||
| <Target Name="_DownloadCopilotCli" BeforeTargets="BeforeBuild" Condition="'$(_CopilotPlatform)' != ''"> | ||||||
| <Error Condition="'$(CopilotCliVersion)' == ''" Text="CopilotCliVersion is not set. The GitHub.Copilot.SDK.props file may be missing from the NuGet package." /> | ||||||
|
|
@@ -68,6 +75,7 @@ | |||||
| <MakeDir Directories="$(_CopilotCacheDir)" Condition="!Exists('$(_CopilotCliBinaryPath)')" /> | ||||||
| <Message Importance="high" Text="Downloading Copilot CLI $(CopilotCliVersion) for $(_CopilotPlatform)..." Condition="!Exists('$(_CopilotCliBinaryPath)')" /> | ||||||
| <DownloadFile SourceUrl="$(_CopilotDownloadUrl)" DestinationFolder="$(_CopilotCacheDir)" DestinationFileName="copilot.tgz" | ||||||
| Timeout="$([System.Int32]::Parse('$(CopilotCliDownloadTimeout)'))" | ||||||
|
||||||
| Timeout="$([System.Int32]::Parse('$(CopilotCliDownloadTimeout)'))" | |
| Timeout="$([System.Int32]::Parse('$(CopilotCliDownloadTimeout)') * 1000)" |
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.
Consider adding input validation for the CopilotCliDownloadTimeout property to ensure it's a positive integer. Invalid values (negative numbers, zero, or non-numeric strings) could cause build failures with cryptic error messages. You could add a validation target or use MSBuild's Error condition, for example:
<Error Condition="$([System.Int32]::Parse('$(CopilotCliDownloadTimeout)')) <= 0" Text="CopilotCliDownloadTimeout must be a positive integer (in seconds)." />This would provide users with a clearer error message if they misconfigure the property.