Skip to content

Add changes for re running "Deploy to Container App" command #999

Open
motm32 wants to merge 12 commits intomainfrom
meganmott/redeploy
Open

Add changes for re running "Deploy to Container App" command #999
motm32 wants to merge 12 commits intomainfrom
meganmott/redeploy

Conversation

@motm32
Copy link
Contributor

@motm32 motm32 commented Dec 10, 2025

This PR adds a couple things:

  • changes to the deploy to container app command to use an internal function. The command id when deploy to container app is run the first time around gets set directly to the internal command so when copilot re runs this command it goes straight into the internal command (see this rg PR for how that is done)
  • adds ACA list step and managed environment list steps to the confirmation view

Some things to note

  • If no subscription is passed in from the activity log items (which there almost always should be) this will revert to the regular deploy flow. I have found that copilot is not very good at picking a subscription.
  • If there is an error I added logic so the loading view closes.

Things to do:

  • update and release utils changes

@motm32 motm32 requested a review from a team as a code owner December 10, 2025 22:57
import { type ManagedEnvironmentContext } from "../../ManagedEnvironmentContext";

export interface ManagedIdentityRegistryCredentialsContext extends CreateAcrContext, ManagedEnvironmentRequiredContext, IContainerAppContext {
export interface ManagedIdentityRegistryCredentialsContext extends CreateAcrContext, ManagedEnvironmentContext, IContainerAppContext {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context loosened from ManagedEnvironmentRequiredContext → ManagedEnvironmentContext.
This is fine only if all consumers tolerate managedEnvironment being undefined. Worth a quick scan of downstream usage.

wizardContext.telemetry.properties.revisionMode = item.containerApp.revisionsMode;

const confirmationViewTitle: string = localize('summary', 'Summary');
let confirmationViewDescription: string = localize('viewDescription', 'Please select an input you would like to change. Note: Any input proceeding the changed input will need to change as well');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let confirmationViewDescription: string = localize('viewDescription', 'Please select an input you would like to change. Note: Any input proceeding the changed input will need to change as well');
let confirmationViewDescription: string = localize('viewDescription', 'Please select an input you would like to change. Note: Any input preceding the changed input will need to change as well');

Copy link
Contributor

@MicroFish91 MicroFish91 Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait proceeding is correct here right? It's what comes after that might need to change?

if (wizardContext.ui instanceof CopilotUserInput) {
promptSteps.push(new OpenLoadingViewStep());
if (isCopilotUserInput(wizardContext)) {
confirmationViewDescription = localize('viewDescription', 'Please review AI generated inputs and select any you would like to modify. Note: Any input proceeding the modified input will need to change as well');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
confirmationViewDescription = localize('viewDescription', 'Please review AI generated inputs and select any you would like to modify. Note: Any input proceeding the modified input will need to change as well');
confirmationViewDescription = localize('viewDescription', 'Please review AI generated inputs and select any you would like to modify. Note: Any input preceding the modified input will need to change as well');


let wizardContext: ContainerAppDeployContext = {} as ContainerAppDeployContext;

if (node && imageSource) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I get some context about this if else if here?

If you have a node and an image source, what does that imply? And if you have a subscription?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node and image source implies you are coming from the regular flow. Just having a subscription and not those other two implies you are coming from a copilot version since we don't pass in a node in that case. I can add comments or something if that is super confusing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think it'd be more readable if you had a flag that says something like: runFromCopilot that's set to true if the subscription is defined or something like that. It just makes it clearer as to why we're checking for it. Maybe add it as a flag up at L68.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 I'd love to see Copilot specific logic consolidated as much as possible to improve maintainability of the normal command flow vs. copilot command flow.


let wizardContext: ContainerAppDeployContext = {} as ContainerAppDeployContext;

if (node && imageSource) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 I'd love to see Copilot specific logic consolidated as much as possible to improve maintainability of the normal command flow vs. copilot command flow.


const subscription = (context as { azureSubscription?: AzureSubscription }).azureSubscription;

if (!subscription && !node) {
Copy link
Contributor

@MicroFish91 MicroFish91 Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding this correctly, could this check be consolidated to a section dedicated to Copilot flow specific changes? A brief comment noting that Copilot may sometimes fail to provide a subscription would help make the intent clearer.

await openLoadingViewPanel(context);
} else {
// Prompt for image source before initializing the wizard in case we need to redirect the call to 'deployWorkspaceProject' instead
imageSource = await promptImageSource(context);
Copy link
Contributor

@MicroFish91 MicroFish91 Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for moving the image source prompt into internal instead of leaving it in the parent function deployContainerApp?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants