dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.82k stars 458 forks source link

KnownResourceStates.TerminalStates should include KnownResourceStates.Hidden #6468

Open DamianEdwards opened 1 week ago

DamianEdwards commented 1 week ago

The KnownResourceStates class includes members for known resource states including 'Running', 'Exited', etc., and a field TerminalStates that's contains all known states considered terminal, i.e. once resources move to that state they won't move to any other state.

The TerminalStates field should include the KnownResourceStates.Hidden value as we still have some resources that end up in that state, e.g. parameters. If one is writing test code today that waits for all resources to reach the KnownResourceStates.Running state or any terminal state before continuing, they can't simply rely on the KnownResourceStates.TerminalStates member to do so, e.g. the following code will never complete if there are any resources that move to the KnownResourceStates.Hidden state:

public static Task WaitForResources(this DistributedApplication app, IEnumerable<string>? targetStates = null, CancellationToken cancellationToken = default)
{
    targetStates ??= [KnownResourceStates.Running, ..KnownResourceStates.TerminalStates];

    var applicationModel = app.Services.GetRequiredService<DistributedApplicationModel>();
    var resourceNotificationService = app.Services.GetRequiredService<ResourceNotificationService>();

    return Task.WhenAll(applicationModel.Resources.Select(r => resourceNotificationService.WaitForResourceAsync(r.Name, targetStates, cancellationToken)));
}
afscrome commented 1 week ago

Does Hidden mean terminal? The dashboard enters and remains in the Hidden state whilst it's running.

dbug: Aspire.Hosting.ApplicationModel.ResourceNotificationService[0] Resource aspire-dashboard/aspire-dashboard-zkqsvpzj changed state: Starting -> Hidden

It does feel odd to me that Hidden is a state in it's own right - hiding a resource feels independent of the state it's in - a resource could be finished and hidden, or running and hidden. The overloading of resource state for hidden also makes it tricky if you want to hide a resource from the dashboard as you have to intercept every state change of the resource and change the state back to hidden - see #6138 .

DamianEdwards commented 1 week ago

Does Hidden mean terminal? The dashboard enters and remains in the Hidden state whilst it's running.

I guess that depends on our definition of "terminal". Now that we have stop/start/restart support in the dashboard, it could be argued nothing is terminal, in that a resource can move from "Finished" back to "Starting" and "Running". Do we have any resources that move from "Hidden" to something else? If they're hidden, you can't send a stop/start/restart command to it, but can it go from "Hidden" to "Finished"?

It does feel odd to me that Hidden is a state in it's own right - hiding a resource feels independent of the state it's in - a resource could be finished and hidden, or running and hidden. The overloading of resource state for hidden also makes it tricky if you want to hide a resource from the dashboard as you have to intercept every state change of the resource and change the state back to hidden - see https://github.com/dotnet/aspire/issues/6138 .

👍