-
Notifications
You must be signed in to change notification settings - Fork 647
Update PublishingActivityProgressReporter to support step/task views #9971
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
Conversation
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.
Pull Request Overview
This PR introduces a hierarchical publishing progress model by splitting the flat activity structure into steps and tasks. Key changes include:
- Refactoring of the progress reporter API to use PublishingStep and PublishingTask.
- Updates to tests, CLI commands, and backchannel data types in order to propagate and display structured progress information.
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/Aspire.Hosting.Tests/Publishing/NullPublishingActivityProgressReporterTests.cs | Updated tests to exercise step and task creation and completion. |
tests/Aspire.Cli.Tests/TestServices/TestAppHostBackchannel.cs | Adjusted backchannel callbacks to return structured PublishingActivity items. |
src/Aspire.Hosting/Publishing/ResourceContainerImageBuilder.cs | Replaced flat activity creation with task creation for image build reporting. |
src/Aspire.Hosting/Publishing/PublishingActivityProgressReporter.cs | Major refactoring to support steps and tasks, updating methods and state channels. |
src/Aspire.Hosting/Publishing/NullPublishingActivityProgressReporter.cs | Adapted the null reporter to the new step/task API. |
src/Aspire.Hosting/DistributedApplicationRunner.cs | Replaced legacy activity updates with complete publish signals. |
src/Aspire.Hosting/CompatibilitySuppressions.xml | Updated suppressions to reflect the refactored publishing activity types. |
src/Aspire.Hosting/Backchannel/* | Modified backchannel data types to use the new PublishingActivity model. |
src/Aspire.Cli/Commands/PublishCommandBase.cs | Adjusted the CLI progress reporting to work with the hierarchical progress model. |
src/Aspire.Cli/Backchannel/* | Updated CLI backchannel interfaces and implementations to yield PublishingActivity items. |
Comments suppressed due to low confidence (2)
src/Aspire.Hosting/Publishing/PublishingActivityProgressReporter.cs:215
- Consider adding an inline comment explaining the rationale for locking on 'parentStep' to ensure thread safety while checking the parent's completion status. This would help future maintainers understand the concurrency design in task creation.
lock (parentStep)
src/Aspire.Cli/Commands/PublishCommandBase.cs:356
- [nitpick] Consider adding inline documentation clarifying the distinction between when the progress context is ready (i.e. currentProgressContext.Ctx is non-null) versus when tasks are queued as pending. This clarification would improve readability of the progress task handling logic.
if (currentStepId == stepId && currentProgressContext.Ctx != null)
3d399da
to
e3e86c8
Compare
f628fbd
to
288ade9
Compare
Contributes towards #9867.
This pull request introduces a hierarchical model (steps => tasks) the publishing progress UI to replace the previous flat activity structure.
aspire-cli-steps-tasks.mov
Open Questions and Follow-ups
PublishComplete
activity we can figure out a way for the user to control these messages.BuildImageAsync
. We likely want to rework this so there is a single step for all calls associated with ResourceContainerImageBuilder and each image build appears as a step. Some interesting things to sort through there around sequencing for the steps/tasks depending on when ResourceContainerImageBuilder is called.