Filter Aspire.Hosting bits out of sdk dump command#15291
Filter Aspire.Hosting bits out of sdk dump command#15291IEvangelist wants to merge 6 commits intorelease/13.2from
Aspire.Hosting bits out of sdk dump command#15291Conversation
…blies When running 'aspire sdk dump MyIntegration.csproj', the output now only shows capabilities, handle types, DTO types, and enums from the specified integration assembly instead of including the entire Aspire.Hosting surface. Changes: - Add assemblyNames parameter to getCapabilities RPC method - Add assemblyName parameter to generateCode RPC method - Apply AtsContextFilter.FilterByExportingAssembliesWithReferences on the server side to scope output to specified assemblies while including referenced supporting types - Add IntegrationAssemblyNameResolver to read AssemblyName from .csproj - Add end-to-end test scanning real assemblies and verifying filtering - Fix test assertion to check actual context entries, not phantom IDs Fixes #14822
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15291Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15291" |
There was a problem hiding this comment.
Pull request overview
Updates the aspire sdk dump / aspire sdk generate flows to scope ATS output to the user-specified integration assembly (instead of always dumping the full Aspire.Hosting surface), by resolving the integration’s assembly name and filtering the ATS context server-side.
Changes:
- Added
IntegrationAssemblyNameResolverand updated SDK commands to use assembly name (vs. project file name) when wiring project integrations. - Extended the AppHost JSON-RPC surface (
getCapabilities,generateCode) to accept optional assembly-scoping parameters and applied ATS context filtering inCodeGenerationService. - Introduced
AtsContextFilter(strict + “include referenced types” modes) and added unit tests covering the filtering and assembly-name resolution.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.RemoteHost.Tests/AtsContextFilterTests.cs | Adds coverage for strict filtering and “with references” behavior, including an end-to-end scan + filter scenario. |
| tests/Aspire.Cli.Tests/Commands/IntegrationAssemblyNameResolverTests.cs | Tests project-file AssemblyName resolution and fallback behavior. |
| src/Aspire.Hosting.RemoteHost/CodeGeneration/CodeGenerationService.cs | Adds optional assembly scoping parameters to RPC methods and applies ATS context filtering before returning data / generating code. |
| src/Aspire.Hosting.RemoteHost/AtsContextFilter.cs | New internal helper that filters ATS contexts by exporting assembly, optionally retaining referenced types needed by selected exports. |
| src/Aspire.Cli/Scaffolding/ScaffoldingService.cs | Updates GenerateCodeAsync call-site to use named arguments after RPC signature change. |
| src/Aspire.Cli/Projects/IAppHostRpcClient.cs | Extends typed RPC interface to accept optional assembly scoping parameters. |
| src/Aspire.Cli/Projects/GuestAppHostProject.cs | Updates GenerateCodeAsync call-site to avoid positional ambiguity after signature change. |
| src/Aspire.Cli/Projects/AppHostRpcClient.cs | Implements the new typed RPC parameters for getCapabilities / generateCode. |
| src/Aspire.Cli/Commands/Sdk/SdkGenerateCommand.cs | Resolves integration assembly name from .csproj and scopes code generation to it. |
| src/Aspire.Cli/Commands/Sdk/SdkDumpCommand.cs | Uses resolved assembly name for project integrations and passes selected assembly names to the RPC call for filtering. |
| src/Aspire.Cli/Commands/Sdk/IntegrationAssemblyNameResolver.cs | New resolver that reads <AssemblyName> from SDK-style project files (with fallback). |
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #23252295798 |
|
The transient CI rerun workflow requested reruns for the following jobs after analyzing the failed attempt.
|
tests/Aspire.Cli.Tests/Commands/IntegrationAssemblyNameResolverTests.cs
Outdated
Show resolved
Hide resolved
…ory for improved path handling
|
The transient CI rerun workflow requested reruns for the following jobs after analyzing the failed attempt.
|
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Made a suggestion for API shape, but weakly held opinion. Really depends on whether we expect this API to evolve much. If it does evolve we might be better introducing some different method names to avoid overload confusions (since its string, string, ct which smells a bit of primitive obsession).
|
Hey @copilot - please address the peer review feedback in a PR targeting updates to the |
|
@IEvangelist I've opened a new pull request, #15353, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Refactor RPC client API to use distinct method names instead of nullable parameters Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> * Clarify that paired client methods intentionally share RPC endpoint names Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com>
|
The transient CI rerun workflow requested reruns for the following jobs after analyzing the failed attempt.
|
|
|
||
| try | ||
| { | ||
| var document = XDocument.Load(projectFile.FullName); |
There was a problem hiding this comment.
hmm I don't like us doing it via XDocument. What happens if this is in a Directory.Build.props file somewhere? I wonder if an alternative is to try to call the GetTargetPath target on the csproj and print it out or something. Other alternative is of couse loading the project using msbuild dlls but that is not ideal as we would have to bring the dependency.
There was a problem hiding this comment.
Yeah, I'd like to avoid msbuild for the dependency, as you mentioned...but for Directory.Build.props is it ever common to have an assembly name in there?
There was a problem hiding this comment.
Other alternative is of couse loading the project using msbuild dlls but that is not ideal as we would have to bring the dependency.
This is probably not possible since we need to Native AOT.
|
|
||
| namespace Aspire.Cli.Tests.Commands; | ||
|
|
||
| public class IntegrationAssemblyNameResolverTests |
There was a problem hiding this comment.
Do we need to add test cases for malformed projects?
There was a problem hiding this comment.
That's a good idea, assuming we keep using the XDocument for the parsing - to ensure failure mode flows as expected.
|
Looks like there are some merge conflicts on this one, and probably at this point it is late to take this change, so let's see if this is something we can instead take in servicing (13.2.x) or otherwise re-target to main. Let me know if you have any concerns with this. |
No concerns, this isn't a high priority one anyway. |
|
Does that mean we should retarget this to main instead then? |
Filter
Aspire.Hostingbits out ofsdk dumpcommandIntegrationAssemblyNameResolverto resolve assembly names from project files.SdkDumpCommandandSdkGenerateCommandto utilize the new resolver.AppHostRpcClientandIAppHostRpcClientto support assembly name parameters.AtsContextFilterfor filtering ATS contexts based on exporting assemblies.IntegrationAssemblyNameResolverandAtsContextFilter.Fixes #14822
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: