-
Notifications
You must be signed in to change notification settings - Fork 715
Description
Is there an existing issue for this?
- I have searched the existing issues
Describe the bug
Take the below two commands. In many places in Aspire, an overload that differs only by IResource and string typically accepts a resource name. So you may think the below two commands are equivalent..
Hoever they're not, the string version of ExecuteCommandAsync actually wants an instance id (i.e. cache-xyz rather than just cache).
The instance ID is a concept that aspire doesn't really expose - the only way you can see it is as a resource property if you show all properties.
Expected Behavior
The XML documentation should be clear about what the resourceId paramater is, and in particular that it is NOT the resoruce.Name someone may assume it is.
Personally I'm inclined to use a completely different method name rather than overloads to distinguish the two forms- e.g. ExecuteCommandAsync and ExecuteInstanceCommandAsync or ExecuteCommandAgainstInstanceAsync.
Perhaps the instance version should also throw an ArgumentException of some kind if the string passed in doesn't contain a - to fail fast (and clearly) if someone passes in a resource name that is never going to work..
Steps To Reproduce
var cache = builder.AddRedis("cache");
cache
.WithCommand("one", "one", async ctx =>
{
var commandService = ctx.ServiceProvider.GetRequiredService<ResourceCommandService>();
await commandService.ExecuteCommandAsync(cache.Resource, "resource-stop");
return CommandResults.Success();
})
.WithCommand("two", "two", async ctx =>
{
var commandService = ctx.ServiceProvider.GetRequiredService<ResourceCommandService>();
await commandService.ExecuteCommandAsync("cache", "resource-stop");
return CommandResults.Success();
});Exceptions (if any)
No response
.NET Version info
No response
Anything else?
No response