Add changes for re running "Deploy to Container App" command #999
Add changes for re running "Deploy to Container App" command #999
Conversation
|
|
||
| public confirmationViewProperty(context: T): ConfirmationViewProperty { | ||
| return { | ||
| name: "Container Apps Environment", |
|
|
||
| if (context.subscriptionId && node && imageSource) { | ||
| // If this command gets re run we only want the internal portion of the command to run, so we set the callbackid | ||
| context.callbackId = 'containerApps.deployContainerAppInternal'; |
There was a problem hiding this comment.
Can you explain what this means?
There was a problem hiding this comment.
If you look at the deployContainerApp it is now split into a internal and non internal portion. The internal portion does not include the pickContainerApp and subscriptionContext creation as I am doing that seperately in the copilot re run command. Thus we want to change the callbackId to go straight to the internal portion instead of the regular deployContainerApp route.
| } | ||
|
|
||
| function isCopilotUserInput(context: IActionContext): boolean { | ||
| return context.ui.constructor.name === 'CopilotUserInput'; |
There was a problem hiding this comment.
Can you do context.ui instanceof CopilotUserInput? I think it's a more robust detection method.
There was a problem hiding this comment.
I changes this but actually when debugging using instanceof won't work. Since I am setting the CopilotUserInput in the rg extension I think the instanceof won't work. I could do some more verbose checks than just the name but not sure if that is necessary.
| import { type ManagedEnvironmentContext } from "../../ManagedEnvironmentContext"; | ||
|
|
||
| export interface ManagedIdentityRegistryCredentialsContext extends CreateAcrContext, ManagedEnvironmentRequiredContext, IContainerAppContext { | ||
| export interface ManagedIdentityRegistryCredentialsContext extends CreateAcrContext, ManagedEnvironmentContext, IContainerAppContext { |
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
| 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'); |
| 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'); |
There was a problem hiding this comment.
| 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'); |
|
@copilot fix all |
| function isCopilotUserInput(context: IActionContext): boolean { | ||
| return context.ui instanceof CopilotUserInput; |
There was a problem hiding this comment.
instanceof can break down if there are multiple versions of the utils package at play, which we aren't always careful about. We've had problems before with UserCancelledError. I'd recommend a canary property on CopilotUserInput, adding it if there isn't one already.
Additionally, this helps TypeScript narrow the type better:
| function isCopilotUserInput(context: IActionContext): boolean { | |
| return context.ui instanceof CopilotUserInput; | |
| function isCopilotUserInput(context: IActionContext): context.ui is CopilotUserInput { | |
| return context.ui instanceof CopilotUserInput; |
| promptSteps.push(new ManagedEnvironmentListStep(), new ContainerAppListStep()); | ||
| } | ||
|
|
||
| let wizardContext: ContainerAppDeployContext = {} as ContainerAppDeployContext; |
There was a problem hiding this comment.
This might remove the need for the as cast:
| let wizardContext: ContainerAppDeployContext = {} as ContainerAppDeployContext; | |
| let wizardContext: Partial<ContainerAppDeployContext> = {}; |
This PR adds a couple things:
Things to do: