Fix aspire wait for JSON-emitted resource names#16224
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16224Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16224" |
There was a problem hiding this comment.
Pull request overview
This PR fixes aspire wait so it accepts both canonical resource Name values (as emitted by aspire ps --format json --resources) and unique displayName values, by resolving the user input against the current AppHost resource snapshots before invoking the wait RPC.
Changes:
- Resolve the user-supplied resource token to a canonical resource
Nameusing resource snapshots before calling the AppHost wait RPC. - Update wait argument help text to indicate it accepts a resource name or identifier.
- Add unit tests covering display-name resolution, canonical-name pass-through, and ambiguous display-name failure.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Commands/WaitCommand.cs | Adds snapshot-based resolution from user input to canonical resource name before waiting. |
| src/Aspire.Cli/Resources/WaitCommandStrings.resx | Updates the resource argument description string to “name or identifier”. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.cs.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.de.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.es.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.fr.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.it.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.ja.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.ko.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.pl.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.pt-BR.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.ru.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.tr.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.zh-Hans.xlf | Updates localized string source/target for the resource argument description. |
| src/Aspire.Cli/Resources/xlf/WaitCommandStrings.zh-Hant.xlf | Updates localized string source/target for the resource argument description. |
| tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs | Captures the last wait call inputs for assertions in unit tests. |
| tests/Aspire.Cli.Tests/Commands/WaitCommandTests.cs | Adds unit tests for display-name resolution, canonical-name pass-through, and ambiguous-name failure. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…re-wait-resource-name
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
This client-side resolution in
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
There was a problem hiding this comment.
I think the wait command should solve this problem the same way the resource command does. The partial resource name should be resolved inside the app host
Addressed in |
|
But I still see resource name resolution logic in WaitCommand... |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixed in |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Everything is moved to the |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…re-wait-resource-name
JamesNK
left a comment
There was a problem hiding this comment.
1 issue flagged: misleading error message when resource name is ambiguous (multiple replicas) vs. genuinely not found.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24573554565 |
|
Did you test this with an older apphost? |
|
@davidfowl do you mean we should see if older apphosts had the issue? If so how old? We didn't have json results that long ago. |
|
new cli old apphost is what needs to always work. |
|
1- we are not touching the protocol definition, |
| }); | ||
|
|
||
| var response = await waitTask.WaitAsync(TimeSpan.FromSeconds(10)); | ||
| var response = await waitTask.WaitAsync(TestConstants.DefaultTimeoutTimeSpan); |
There was a problem hiding this comment.
No, use DefaultTimeout in tests. A shared file may need to be added to the project. It adapts waiting time based on the environment, no ops when debugger is attached, provides better error message.
Description
aspire waitwas forwarding the user-supplied resource token directly to the AppHost wait RPC. That RPC only accepts the canonical resourceName, which meant the generatednameemitted byaspire ps --format json --resourcescould fail even though the friendly display name worked.This updates
waitto resolve the input against current resource snapshots before invoking the backchannel. Exact canonical names still pass through, unique display names are mapped to the canonicalName, and ambiguous replica display names still fail instead of guessing. The PR also updates the wait help text to say it accepts a resource name or identifier and adds unit coverage for the resolution behavior.Fixes #15842
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: