Conversation
|
Oh I misread, thought this was based on the issue, not the PR I just opened. |
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="NuGet.Configuration" /> |
There was a problem hiding this comment.
This reference isn't needed after moving from dotnet tool install to dotnet tool exec. Remove it, and the reference in Directory.Packages.props
|
@copilot Implement the PR feedback |
…location - Changed app URLs from 18001/18002 to 16179/16180 (standard app port range) - Updated OTLP endpoints from 18003/18005 to 17119/17120 (standard OTLP range) - Added MCP endpoints 18036/18037 (standard MCP range) - Updated Resource Service endpoints to match OTLP endpoints - Removed launchBrowser from generate-manifest profile for consistency Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Updated launchSettings.json to align with PR #13155 port allocation scheme in commit d2a0efe. Changes:
|
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13169Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13169" |
| @@ -0,0 +1,7 @@ | |||
| namespace DotnetTool.AppHost; | |||
|
|
|||
| internal sealed class DotnetToolInstaller(string name, string command) : | |||
There was a problem hiding this comment.
Remove this class and file - I should have been removed for the dotnet tool exec version.
| .WithPackageIgnoreFailedSources() | ||
| ; |
There was a problem hiding this comment.
| .WithPackageIgnoreFailedSources() | |
| ; | |
| .WithPackageIgnoreFailedSources(); |
|
@davidfowl Anything I can do to help move this along. |
|
@copilot DotnetToolInstaller - Remove this class and file - I should have been removed for the dotnet tool exec version. Also write tests for this in a new test project. |
- Removed DotnetToolInstaller.cs (leftover from dotnet tool install approach) - Created new test project Aspire.Hosting.DotnetTool.Tests with 22 tests - All tests pass and validate AddDotnetTool extension methods - Tests cover: basic usage, version specs, prerelease, custom sources, args, and manifest generation - Added using statements to playground files (needed for test linking) - Fixed DotnetToolResource to use "." as working directory (fixes manifest generation) - Added test project to solution file in alphabetical order Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Removed
All tests pass successfully. |
| x.Args.Add("--verbosity"); | ||
| x.Args.Add("detailed"); |
There was a problem hiding this comment.
Verbose output made sense when dotnet tool install was it's own resource, but for dotnet tool exec, it just adds noise a user may not be expecting.
| x.Args.Add("--verbosity"); | |
| x.Args.Add("detailed"); |
|
It would be awesome if we could render the Currently the source renders like the top example, but it would be nicer if we could render it more like the second - similar to how project resources render
Similar to how projects are displayed I was able to hack this to work, but DCP does it's darndest to undo any custom values, so I had to fight aspire/src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs Lines 165 to 174 in 93abfaf .OnInitializeResource(async (resource,evt,ct) =>
{
var rns = evt.Services.GetRequiredService<ResourceNotificationService>();
_ = Task.Run(async () =>
{
await foreach(var x in rns.WatchAsync(ct))
{
if (x.Resource != resource)
{
continue;
}
var expectedPath = resource.ToolConfiguration.PackageId;
var existingPathProp = x.Snapshot.Properties.FirstOrDefault(p => p.Name == "executable.path");
if (existingPathProp == null || existingPathProp.Value as string != expectedPath)
{
var args = ImmutableArray.Create(await resource.GetArgumentValuesAsync());
var toolSplitIndex = args.IndexOf("--");
if (toolSplitIndex > 0)
{
args = args[(toolSplitIndex + 1)..];
}
await rns.PublishUpdateAsync(resource, x => x with
{
//TODO: `Use KnownProperties.Executable.Path` & `KnownProperties.Resource.AppArgsSensitivity`
//TODO: Merge with existing entry, not appending
Properties = x.Properties
.RemoveAll(x => x.Name == "resource.appArgs" || x.Name == "executable.path")
.AddRange(
new ResourcePropertySnapshot("resource.appArgs", args),
//TODO: set args sensitivity appropiately
//new(KnownProperties.Resource.AppArgsSensitivity, TODO)
new ResourcePropertySnapshot("executable.path", resource.ToolConfiguration.PackageId))
});
}
}
}, ct);
});Related to the above, should we differentiate between args provided to the tool itself, vs args we provide to |



Description
Ports the .NET tool integration playground from #13168 by @afscrome. This adds support for running .NET global tools (dotnet-ef, dotnet-dump, etc.) as Aspire resources using
dotnet tool exec.Credit: All implementation code authored by @afscrome.
Changes
playground/DotnetTool/demonstrating .NET tool resources withAddDotnetTool()extension methodsNuGet.Configurationv6.14.0 toDirectory.Packages.propslaunchSettings.jsonto align with PR Fix playground AppHost port conflicts, add MCP endpoints, and ensure generate-manifest profiles - assign unique ports to all 52 projects #13155 port allocation scheme:DotnetToolInstaller.cs(leftover fromdotnet tool installapproach that was not needed fordotnet tool execimplementation)Aspire.Hosting.DotnetTool.Testswith 22 unit tests covering:Example Usage
Relates to #13077
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplateOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.