TemplateProvider for python v2 templates #3799
Conversation
| import { ParsedAction } from "../../../templates/script/parseScriptTemplatesV2"; | ||
| import { FunctionV2WizardContext } from "../FunctionV2WizardContext"; | ||
|
|
||
| export abstract class ActionSchemaBaseStep<T extends FunctionV2WizardContext> extends AzureWizardExecuteStep<T> { |
There was a problem hiding this comment.
I think Base should always be a postfix. So ActionSchemaStepBase.
Also I think you could remove schema from the name. Maybe add in a V2 somewhere ActionV2StepBase?
There was a problem hiding this comment.
Yeah, you're right. I was thinking of it as the base for the step, but it's actually the step's base.
English 💯
| const filePath = nonNullProp(this.action, 'filePath'); | ||
| let source: string | undefined = context.functionTemplateV2?.files[filePath]; | ||
| if (!source) { | ||
| throw new Error(`Could not find file "${filePath}" in template`); |
| const assignToTokens = Object.keys(context).filter(k => k.startsWith('$(')); | ||
| for (const token of assignToTokens) { | ||
| source = source.replace(new RegExp(this.escapeRegExp(token), 'g'), context[token] as string); | ||
| } |
There was a problem hiding this comment.
This code is duplicated in ReplaceTokensInTextExecuteStep.
There was a problem hiding this comment.
Yeah, I wasn't sure what to do. Technically, this step is supposed to exist, but I don't actually see it in any of the jobs. Ideally, I could remove this code and it would be done in ReplaceTokenInTextExecuteStep, but since it never seems to be a step, I'll just delete this with a comment in GetTemplateFileContentExecuteStep`.
| export class ShowMarkdownPreviewExecuteStep<T extends FunctionV2WizardContext> extends ActionSchemaBaseStep<T> { | ||
| public async executeAction(context: T): Promise<void> { | ||
| const filename = nonNullProp(this.action, 'filePath'); | ||
| const content = context.functionTemplateV2?.files[filename] || ''; |
There was a problem hiding this comment.
Any specific reason using || over ?? here? Just curious.
| } | ||
|
|
||
| public shouldPrompt(context: T): boolean { | ||
| return !context[this.input.assignTo]; |
There was a problem hiding this comment.
If the input type is boolean, and the user inputs false and context[this.input.assignTo] === false then shouldPrompt() will return true when it should return false since the user has already completed this prompt.
Certainly an edge case, but I could see it causing issues.
We probably should just check if the value is not null or undefined.
…mplate.v# rather than bundleVersions prop, default main app name
| switch (templateFilter) { | ||
| case TemplateFilter.All: | ||
| return templates.functionTemplatesV2; | ||
| // TODO: Verify if schema V2 templates have categories |
There was a problem hiding this comment.
They do not. We could still have All vs. Verified (once we add tests) but I'm not sure if there's much value in that. I may just remove the filter for V2
| protected _language: string; | ||
|
|
||
| public async getLatestTemplates(context: IActionContext, latestTemplateVersion: string): Promise<ITemplates> { | ||
| // just use backup templates until template feed deploys v2 templates |
| for (const job of templateV2.jobs) { | ||
| const parsedInputs: ParsedInput[] = []; | ||
| job.inputs.forEach(input => { | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion |
| const userPrompts: RawUserPrompt[] = parseUserPrompts(rawBindings, resources); | ||
| const templates: FunctionV2Template[] = []; | ||
| for (const templateV2 of rawTemplates) { | ||
| // look into jobs-- each job can be an Azure Wizard. Name is the title |
|
|
||
| export interface ITemplatesReleaseV2 extends ITemplatesReleaseBase { | ||
| userPrompts: string; | ||
| // it is not supposed to exist in the v2 schema, but sometimes userPrompts accidentally gets replaced with bindings |
There was a problem hiding this comment.
link to a GitHub issue for this?
src/utils/bundleFeedUtils.ts
Outdated
| const bundleTemplateTypes: string[] = ['durable', 'signalr']; | ||
| // I don't think v2 templates are ever a bundle template? Will have to verify this |
There was a problem hiding this comment.
gif caught my attention 😅
Yeah they should be bundle templates. For example, we should have a different template for a Cosmos DB trigger depending if it's a newer bundle or old bundle. Here are examples of how the code is different:
- New bundle: /~https://github.com/Azure/azure-functions-nodejs-e2e-tests/blob/main/app/v4/src/functions/cosmosDBTrigger1.ts
- Old bundle: /~https://github.com/Azure/azure-functions-nodejs-e2e-tests/blob/main/app/v4-oldBundle/src/functions/cosmosDBTrigger1.ts
If this isn't the case, we can have a chat with Naren and I'm sure he'll sort us out.
There was a problem hiding this comment.
Sorry, what I meant is, I assume that they are all "true" and since it wouldn't be included in the v1 runtime, we don't ever call it in the first place.
So nah, we don't need this.
nturinski
force-pushed
the
nat/v2TemplateSchema
branch
from
3e46c10 to
1878157
Compare
Combines the feedback from both #3729 and #3742
This is a massive PR, but hopefully shouldn't touch any existing logic with our old code paths. I'll delete the readme file before I merge, but I left it in here for some context as to what/why I'm doing for parsing the new templates.