Improve the publish/deploy output #9981
Merged
+389
−82
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidfowl
changed the title
…case from PublishingTests
davidfowl
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR improves the publish/deploy output by refining progress reporting, logging messages, and image building logic across multiple publishing contexts and tests. Key changes include:
- Removal of a deploying callback test from the PublishingTests.
- Integration of a singleton registration for IResourceContainerImageBuilder in several Docker tests.
- Enhancements to progress reporting and error messaging in both Docker and Azure publishing flows.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/PublishingTests.cs | Removed a test for deploying callbacks, likely in favor of updated behavior. |
| tests/Aspire.Hosting.Docker.Tests/* | Added singleton registration for IResourceContainerImageBuilder and minor adjustments to logging. |
| src/Aspire.Hosting/Publishing/ResourceContainerImageBuilder.cs | Refactored container runtime handling and updated log messages. |
| src/Aspire.Hosting/Publishing/Publisher.cs | Enhanced progress reporting with additional steps and improved log messages. |
| src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs | Revised .env file generation logic and added progress reporting around file output. |
| src/Aspire.Hosting.Azure/* | Improved progress reporting for publishing Azure Bicep templates. |
| src/Aspire.Cli/Commands/PublishCommandBase.cs | Updated UI interaction handling for progress status and removed a redundant display message. |
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs:146
- The condition for writing the .env file was inverted compared to the previous logic; please verify that the .env file should be written only when environment.CapturedEnvironmentVariables.Count is greater than zero.
if (environment.CapturedEnvironmentVariables.Count > 0)
|
|
||
| if (taskInfo.State == TaskCompletionState.CompletedWithError) | ||
| { | ||
| // TOOD: This should be automatically handled (if any steps fail) |
There was a problem hiding this comment.
Typo detected; please correct 'TOOD' to 'TODO' for clarity.
Suggested change
| // TOOD: This should be automatically handled (if any steps fail) | |
| // TODO: This should be automatically handled (if any steps fail) |
Copilot uses AI. Check for mistakes.
captainsafia
approved these changes
Jun 23, 2025
| @@ -146,7 +144,11 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell | |||
| _interactionService.DisplayMessage("bug", InteractionServiceStrings.WaitingForDebuggerToAttachToAppHost); | |||
| } | |||
|
|
|||
| var backchannel = await backchannelCompletionSource.Task.ConfigureAwait(false); | |||
| var backchannel = await _interactionService.ShowStatusAsync($":hammer_and_wrench: {GetProgressMessage()}", async ()=> | |||
There was a problem hiding this comment.
What does this help achieve?
There was a problem hiding this comment.
Shows some UX before publishing starts then it goes away.
| @@ -58,4 +59,45 @@ public async Task BuildImageAsync(string contextPath, string dockerfilePath, str | |||
| throw new DistributedApplicationException($"Docker build failed with exit code {exitCode}."); | |||
| } | |||
| } | |||
|
|
|||
| public Task<bool> CheckIfRunningAsync(CancellationToken cancellationToken) | |||
There was a problem hiding this comment.
Not blocking feedback but do we want to add more granularity here? For example, if the container runtime is running and not healthy we want to surface why in the logs.
I realize this might be expose the same "you have to grep the logs to find the error" problem but something to think about.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Demos:
Publish failed:
Publish successful:
Running publish:
Screen.Recording.2025-06-21.173308.mp4