-
Notifications
You must be signed in to change notification settings - Fork 693
Update scripts to use the new Quality mappings to urls #10479
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
Conversation
Co-authored-by: Copilot <[email protected]>
Hiding whitespace changes is useful for the review. |
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.
Pull Request Overview
This PR updates the Aspire CLI download scripts to support new quality-based URL mappings and adds dry-run functionality. The changes standardize the URL structure across PowerShell and Bash scripts, with staging
becoming the default quality level instead of daily
.
Key changes:
- Updated URL mappings for quality levels (
dev
,staging
,ga
) to use aka.ms URLs - Added
--dry-run
and-WhatIf
support for testing without performing actual operations - Improved error handling, parameter validation, and PowerShell coding practices
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
eng/scripts/get-aspire-cli.sh | Updated Bash script with new quality-based URL mappings, added dry-run support, and improved error handling |
eng/scripts/get-aspire-cli.ps1 | Enhanced PowerShell script with configuration-based URL construction, WhatIf support, and better PowerShell practices |
eng/scripts/README.md | Updated documentation to reflect new quality options and default values |
|
||
$expectedChecksum = (Get-Content $ChecksumFile -Raw).Trim().ToLower() | ||
$actualChecksum = (Get-FileHash -Path $ArchiveFile -Algorithm SHA512).Hash.ToLower() | ||
$expectedChecksum = (Get-Content $ChecksumFile -Raw -ErrorAction Stop).Trim().ToLowerInvariant() |
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 validation to ensure the checksum file contains valid content before processing. An empty or malformed checksum file could cause unexpected behavior.
$expectedChecksum = (Get-Content $ChecksumFile -Raw -ErrorAction Stop).Trim().ToLowerInvariant() | |
$expectedChecksum = (Get-Content $ChecksumFile -Raw -ErrorAction Stop).Trim().ToLowerInvariant() | |
# Validate the checksum content | |
if (-not $expectedChecksum) { | |
throw "Checksum file '$ChecksumFile' is empty. Please provide a valid checksum file." | |
} | |
if ($expectedChecksum -notmatch '^[a-f0-9]{128}$') { | |
throw "Checksum file '$ChecksumFile' contains invalid content. Expected a 128-character hexadecimal string." | |
} | |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Verified so far on MacOS. Are we going to change the default quality level in each branch? |
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.
tested; would be great if others can test on linux/macos
I did 😄 |
Testing in a codespace now (for linux) |
@DeagleGross did you try different staging and dev? |
Works on linux! (in bash) |
@davidfowl yep, works for staging and dev |
/backport to release/9.4 |
Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16435355366 |
@radical backporting to "release/9.4" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Update scripts to use the new Quality mappings to urls
Applying: Rename qualities to dev, staging, and ga. And make staging the default
Applying: Cleanup the scripts making the ps1 more idiomatic. And add support for -WhatIf in ps1, and --dry-run in sh
Applying: Update eng/scripts/get-aspire-cli.ps1
Applying: Fix parameter validation
Applying: Update eng/scripts/get-aspire-cli.ps1
Applying: Update eng/scripts/get-aspire-cli.sh
Applying: rename ga to release
Applying: change default quality to release
Applying: Add examples of using the script with arguments
Using index info to reconstruct a base tree...
M docs/using-latest-daily.md
Falling back to patching base and 3-way merge...
Auto-merging docs/using-latest-daily.md
CONFLICT (content): Merge conflict in docs/using-latest-daily.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0010 Add examples of using the script with arguments
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
## Key Changes ### 1. **Quality Level Behavior** - **`dev`**: Downloads latest development builds from the `main` branch - **`staging`**: Downloads release candidate builds (rc/daily) - **`ga`**: Downloads generally available release builds (ga/daily) - **Default**: `staging` quality is used when no quality is specified ### 2. **Updated URL Mappings for Quality Levels** - **PowerShell Script (`get-aspire-cli.ps1`)**: - `dev` quality now maps to: `https://aka.ms/dotnet/9/aspire/daily` - `staging` quality now maps to: `https://aka.ms/dotnet/9/aspire/rc/daily` - `ga` quality now maps to: `https://aka.ms/dotnet/9/aspire/ga/daily` - **Bash Script (`get-aspire-cli.sh`)**: - Updated with corresponding URL mappings to match PowerShell script - Consistent quality-to-URL mapping across both script implementations ### 3. **Support for -WhatIf and --dry-run** - This will be useful for testing ### 4. **Code Quality Improvements** The code has been made more idiomatic in many ways: - Better PowerShell coding practices and conventions - Improved error handling patterns - Enhanced cross-platform compatibility - More robust parameter validation - Cleaner function organization and separation of concerns Co-authored-by: Copilot <[email protected]> (cherry picked from commit a5d2813)
Key Changes
1. Quality Level Behavior
dev
: Downloads latest development builds from themain
branchstaging
: Downloads release candidate builds (rc/daily)ga
: Downloads generally available release builds (ga/daily)staging
quality is used when no quality is specified2. Updated URL Mappings for Quality Levels
PowerShell Script (
get-aspire-cli.ps1
):dev
quality now maps to:https://aka.ms/dotnet/9/aspire/daily
staging
quality now maps to:https://aka.ms/dotnet/9/aspire/rc/daily
ga
quality now maps to:https://aka.ms/dotnet/9/aspire/ga/daily
Bash Script (
get-aspire-cli.sh
):3. Support for -WhatIf and --dry-run
4. Code Quality Improvements
The code has been made more idiomatic in many ways: