Skip to content

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

Merged
merged 10 commits into from
Jun 21, 2025

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Jun 20, 2025

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

  • A failed task doesn't terminate the publish flow. Only failed steps or an entirely failed publish command do. Should failed tasks also force the completion of the publish flow?
  • Should completing a step complete all in-progress tasks or should it only be possible to complete a step when all tasks attached to it have been completed? We don't currently have any safety checks here.
  • Currently, the user has no way to control the top-level success/failure message. Now that we have a PublishComplete activity we can figure out a way for the user to control these messages.
  • The modeling of steps/tasks in the ResourceContainerImageBuilder isn't the final view. We currently create a new step for each invocation of 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.

@Copilot Copilot AI review requested due to automatic review settings June 20, 2025 18:58
@captainsafia captainsafia requested a review from mitchdenny as a code owner June 20, 2025 18:58
@captainsafia captainsafia requested a review from davidfowl June 20, 2025 18:58
Copy link
Contributor

@Copilot Copilot AI left a 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)

@captainsafia captainsafia force-pushed the safia/rework-output branch from 3d399da to e3e86c8 Compare June 20, 2025 19:05
@captainsafia captainsafia force-pushed the safia/rework-output branch from f628fbd to 288ade9 Compare June 21, 2025 00:02
@captainsafia captainsafia enabled auto-merge (squash) June 21, 2025 01:05
@captainsafia captainsafia merged commit b3fcf36 into main Jun 21, 2025
252 checks passed
@captainsafia captainsafia deleted the safia/rework-output branch June 21, 2025 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants