-
Notifications
You must be signed in to change notification settings - Fork 721
feat: aspire exec #10061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: aspire exec #10061
Conversation
|
TODO:
|
playground/DatabaseMigration/DatabaseMigration.ApiModel/Migrations/MyDb1ContextModelSnapshot.cs
Show resolved
Hide resolved
| { | ||
| using var activity = _telemetry.ActivitySource.StartActivity(this.Name); | ||
|
|
||
| var passedAppHostProjectFile = parseResult.GetValue<FileInfo?>("--project"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Should this be apphost instead? I could see confusion for people thinking project means a project resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense, but I am following the convention other commands have. For example RunCommand also refers to apphost path as --project, see
| var passedAppHostProjectFile = parseResult.GetValue<FileInfo?>("--project"); |
I think it will only bring confusion to users if in exec apphost flag would change from --project to --apphost.
|
|
||
| // a bit hacky, but in order to pass a full command with possible quotes and etc properly without losing the signature | ||
| // we can wrap it in a string and pass it as a single argument | ||
| "--command", $"\"{string.Join(" ", commandTokens)}\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? Should there be a test with an argument with a quoted string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added test with quoted string - it is running dotnet build "MyRandom.csproj" and it works! see Exec_EchoWithQuotedString_ShouldProduceLogs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually if you have other idea on how to parse / send the command to apphost - let me know. Aspire apphost does not have commandLineParser installed on its side, so it can only take the args passed from cli. I decided to pack the command in such a way, and I know it is not pretty, but it works at least.
| return ExitCodeConstants.Success; | ||
| } | ||
| } | ||
| catch (OperationCanceledException ex) when (ex.CancellationToken == cancellationToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit gnarly, looks like it was copied from other commands. Wonder if we can turn it into a helper method that handles all these different exception types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mind doing it in the follow-up PR? I can refactor all other commands as well
| /// Additional info about type of the message. | ||
| /// Should be used for controlling the display style. | ||
| /// </summary> | ||
| public string? Type { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it should be an enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is hard to achieve - apphost will have its own type of enum, and i have to somehow tell both cli and apphost to serialize/deserialize the enum in the same manner.
Nowdays apphost does not have the serialization context, and there is an effort to provide better experience, but not yet merged + requires serialization context for apphost:
#10058
lets do in the follow-up PR if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filed #10201
This reverts commit 2c143e3.
is the result of experiments #9842 and #9912
Description
This pull request introduces a new
execcommand to the Aspire CLI, allowing users to execute commands on a target resource through the app host. It also includes supporting changes to the backchannel, interaction services, and overall CLI infrastructure to enable this functionality.Implementation
As discussed here feature is built fully on top of existing capabilities - aspire exec forces apphost on the startup to add a new
ExecutableResourceto the DCP resources, and then via streams the logs of the resource back to cli backchannel viaResourceLoggerServiceand manages the lifetime of the resource viaResourceNotificationService.Moreover, new resource inherits
Environment,ResourceRelationshipandWaitForannotations of the target resource in order to be launched exactly in the same configuration as the target resource.There are 2 types of events I am waiting for from
ResourceNotificationService:KnownResourceStates.Running- i am changing the type of the log from "waiting" to "running". This is useful to have more interaction from the UI side and show that command is not yet being executed, but is waiting for other resources to start up (yellow text in the video)KnownResourceStates.TerminalStates- we know there will be no logs coming, and we can complete the logger then. It will stop the async-foreach of watching the logs and cli will also understand that it can continue with the flow.Extra configuration
--resourceone can write--start-resourceforcing the target resource to start before the command execution.--apphost-keepaliveis a flag which will not force apphost shutdown from the cli, but simply exists the cli after command is executed.Demo
Video shows execution of such aspire exec:
WebApp does not have the connection string defined anywhere explicitly, it only has the DbContext registration like:
And AppHost only defines the pgSql resource and the webapp referencing and waiting for db:
That shows that connection string is coming from the apphost's runtime knowledge about how postgresDb resource is being built and launched.
aspire.exec.local.tool.migrations.add.mp4
aspire.exec.local.tool.database.update.mp4
Fixes #9859