Support for new Python language model #3235

philliphoff · 2022-07-07T16:55:43Z

Adds support for the new Python language model, where bindings are attributes within the application code rather than in separate JSON files.

package-lock.json

package.json

bwateratmsft · 2022-07-08T13:14:37Z

src/commands/initProjectForVSCode/InitVSCodeLanguageStep.ts

-            { label: ProjectLanguage.Python },
-            { label: ProjectLanguage.TypeScript },
-            { label: ProjectLanguage.Custom }
+        const languagePicks: IAzureQuickPickItem<{ language: ProjectLanguage, model?: number }>[] = [


This { language: ProjectLanguage, model?: number } "type" is used in a couple places; it might benefit from a proper type definition.

How about, if I see it in one more place, I promise to do so. :-)

src/commands/initProjectForVSCode/InitVSCodeLanguageStep.ts

src/commands/initProjectForVSCode/initProjectForVSCode.ts

src/commands/createNewProject/ProjectCreateStep/PythonProjectCreateStep.ts

src/commands/createNewProject/NewProjectLanguageStep.ts

src/commands/createFunction/scriptSteps/PythonScriptStep.ts

src/commands/createFunction/FunctionListStep.ts

src/commands/createFunction/scriptSteps/PythonFunctionCreateStep.ts

src/commands/createNewProject/ProjectCreateStep/PythonProjectCreateStep.ts

src/constants.ts

src/debug/validatePreDebug.ts

alexweininger · 2022-08-18T19:46:09Z

I like the auto open markdown feature, really cool (and tricky to implement IIRC) that it grabs the markdown file from the template and displays it rendered.

I wish we could do something similar with walkthroughs, ex: spawn a walkthrough from a markdown file, or maybe a special walkthrough.json file. Could be really useful for templates, samples, etc.

alexweininger · 2022-08-18T19:53:10Z

When I append a new function to a file, it's not smart about making sure the new function name is unique. Sounds like it's out of scope for the time being. But something we might revisit later.

philliphoff · 2022-08-18T20:00:01Z

When I append a new function to a file, it's not smart about making sure the new function name is unique. Sounds like it's out of scope for the time being. But something we might revisit later.

Yes, it is exactly things of that sort that require a robust language service in order to solve, but which take time we didn't think we had for this initial preview. Even then it's likely to result in a long tail of possible code structure to try to cover.

philliphoff · 2022-08-18T20:01:44Z

I like the auto open markdown feature, really cool (and tricky to implement IIRC) that it grabs the markdown file from the template and displays it rendered.

It's admittedly a hack (in particular, to support VS Code's need to reopen the document on a new file) to avoid having to manage a local disk cache), but I think it will work well enough for most cases.

nturinski

There is no way to pass an argument to change the default trigger names right now since everything is static content in the backupTemplates, right?

I was surprised by the lack of prompt steps, but I'm assuming when the template feed is actually serving the templates, that there will be a way to pass in parameters for starting names and other properties similar to the other templates?

NOTE: I left a comment in the templates.json but it might be annoying to view due to the size of that file so I'll leave it here as well:

Alex mentioned that the host.json can't contain the extensionBundle property in order to debug (and you have denoted as such in the code.) I noticed that all the templates have host.jsons that include it by default, so whenever you create a new trigger, it automatically updates the host.json to include the property again.

src/commands/createNewProject/NewProjectLanguageStep.ts

src/commands/createNewProject/ProjectCreateStep/PysteinProjectCreateStep.ts

src/debug/validatePreDebug.ts

src/templates/script/PysteinTemplateProvider.ts

src/templates/script/parseScriptTemplates.ts

src/tree/localProject/LocalFunctionsTreeItem.ts

nturinski · 2022-08-19T03:02:04Z

src/commands/createFunction/scriptSteps/PythonScriptStep.ts

+
+export class PythonScriptStep extends AzureWizardPromptStep<IPythonFunctionWizardContext> {
+    public async prompt(wizardContext: IPythonFunctionWizardContext): Promise<void> {
+        const entries = await vscode.workspace.fs.readDirectory(vscode.Uri.file(wizardContext.projectPath));


AzExtFsExtra does have a readDirectory() command though the return types are a little bit different.

Noted, though I'd prefer to use the VS Code APIs when available (and reasonably convenient). If anything, it makes it easier for the extension to support their new remote host/filesystem scenarios. (Yes, there are lots of other things that wouldn't work for Functions in those scenarios, but still...)

Well, under the hood, AzExtFsExtra is using all VS Code APIs which was the whole point of this PR.

https://github.com/microsoft/vscode-azuretools/blob/main/utils/src/utils/AzExtFsExtra.ts#L94-L100

It just offers some QOL, like not having to wrap projectPath with vscode.Uri.file(), but definitely not a ship blocker or anything.

Ah, ok, I see, then forget everything I said! For some reason I thought AzExtFsExtra was still using fs under the hood. :-) Eh, in this particular case I find direct use of the VS Code API easier to read than using the wrapper. I could go either way.

resources/backupTemplates/pystein/templates/templates.json

nturinski

I'm good to approve this. I kind of want to merge this sooner so we can start to get CTI doing test passes on it, but I'd understand wanting to wait for Brandon to rise from the dead to give it a thorough review. 🧟

philliphoff · 2022-08-19T18:20:47Z

There is no way to pass an argument to change the default trigger names right now since everything is static content in the backupTemplates, right?

I was surprised by the lack of prompt steps, but I'm assuming when the template feed is actually serving the templates, that there will be a way to pass in parameters for starting names and other properties similar to the other templates?

@nturinski Agreed that this initial preview is limited in scope with respect to customization of the template (i.e. there effectively is none). To do it properly requires more than just token replacement in the added function source, as there may be other changes needed elsewhere (e.g. new imports, validation against existing function names, etc., and, in particular, when they introduce PyStein's "blueprints"). We really need a robust language service that allows source manipulation a la Roslyn, and I'm not sure we have that now (and our time frame didn't really allow it in any case). This is one reason we're trying to lean on the "inline" documentation in the function templates, to help users understand what needs to change in order to add new functions.

nturinski · 2022-08-19T18:24:41Z

src/commands/createNewProject/ProjectCreateStep/PysteinProjectCreateStep.ts

@@ -19,6 +20,16 @@ import { showMarkdownPreviewFile } from '../../../utils/textUtils';

 const gettingStartedFileName = 'getting_started.md';

+async function fileExists(uri: vscode.Uri): Promise<boolean> {


You could use this to save some lines, though I will go back and add a fileExists utility function later.

You would also not have to wrap the file paths with Uri.file() as well.

Doesn't stat() throw if the file doesn't exist?

Oh, wait, never mind, I see what you mean now.

philliphoff · 2022-08-19T18:33:51Z

I'm good to approve this. I kind of want to merge this sooner so we can start to get CTI doing test passes on it, but I'd understand wanting to wait for Brandon to rise from the dead to give it a thorough review. 🧟

Agree that I'd like CTI taking a look and making sure I didn't horribly break the existing scenarios. @bwateratmsft did an early review of the draft PR, but I'm happy for him to tear it apart further when he's back next week (and I'm out). I can then clean up when I get back.

bwateratmsft · 2022-08-22T13:57:18Z

I took a brief look at the changes since my review and didn't see anything that concerned me. Agree, getting it in sooner than later is a good idea so CTI can take a look. Thanks!

philliphoff added 23 commits May 24, 2022 15:50

Sketch preview project type.

f292a6e

Switch to language model rather than separate (sub) languages.

bd581ea

Exclude Python v2+ from creating functions on project create.

170361f

Store project language model in workspace settings.

ee2b112

Add project-level function source.

50f8425

Use "isolated mode" detection of functions for Python v2+.

18fa4b2

Merge branch 'main' into philliphoff-new-python-model

a5e5b40

Open function .py file on project creation.

a1e2427

Detect Python language model.

c23d8d6

Add Python preview to VS Code init list.

0e965f4

Extract python preview model into constant.

e3b5efe

Merge branch 'main' into philliphoff-new-python-model

2fde907

Sketch prompt for Python function location.

d11297c

Sketch selecting a script.

5c14fb6

Sketch opening template.

2d6e945

Append new function to file.

ddbdec4

Enforce storage configuration for Python V2+.

33bccc0

Exclude extension bundles from Python V2+ projects.

5493c6a

Check for valid Functions version.

6dc3230

Merge branch 'main' into philliphoff-new-python-model

8daf287

Complete merge from main.

4f15b00

Resolve linter warnings.

e9e4cab

Updates per PM feedback.

351ea86

bwateratmsft reviewed Jul 8, 2022

View reviewed changes

Updates per PR feedback.

bb5b5cb

nturinski reviewed Jul 14, 2022

View reviewed changes

src/commands/createNewProject/ProjectCreateStep/PythonProjectCreateStep.ts Outdated Show resolved Hide resolved

nturinski reviewed Jul 14, 2022

View reviewed changes

src/commands/createNewProject/ProjectCreateStep/PythonProjectCreateStep.ts Outdated Show resolved Hide resolved

nturinski reviewed Jul 14, 2022

View reviewed changes

src/constants.ts Show resolved Hide resolved

nturinski reviewed Jul 14, 2022

View reviewed changes

src/debug/validatePreDebug.ts Show resolved Hide resolved

Merge branch 'main' into philliphoff-new-python-model

ec0bc15

philliphoff added 6 commits August 17, 2022 15:33

Disable name prompt for V2.

0b50d12

Update required Function host version, add whitespace during append.

e9ff169

Update version number.

40b2bdd

Replace direct use of fs-extra.

d5e70ab

Add notes for preview Python model numbering.

65b9180

More notes.

4512d63

philliphoff marked this pull request as ready for review August 18, 2022 16:39

philliphoff requested a review from a team as a code owner August 18, 2022 16:39

philliphoff added 2 commits August 18, 2022 10:16

Show getting started MD on creation.

532af72

Do not expose properties commands from PyStein functions in tree.

9f89955

nturinski reviewed Aug 19, 2022

View reviewed changes

philliphoff added 4 commits August 19, 2022 09:51

Exclude PyStein from re-adding bundle property on function create.

11068e5

Updates to documentation per feedback in PR.

4137ea2

Update supported PyStein version number and comments.

1056c34

Allow opening MD to the side.

5718cf9

nturinski previously approved these changes Aug 19, 2022

View reviewed changes

Use opt-out list instead of opt-in.

70e4d64

philliphoff dismissed nturinski’s stale review via 70e4d64 August 19, 2022 18:09

nturinski reviewed Aug 19, 2022

View reviewed changes

nturinski approved these changes Aug 19, 2022

View reviewed changes

philliphoff merged commit 925796c into main Aug 19, 2022

philliphoff deleted the philliphoff-new-python-model branch August 19, 2022 20:54

microsoft locked and limited conversation to collaborators Oct 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for new Python language model #3235

Support for new Python language model #3235

philliphoff commented Jul 7, 2022

bwateratmsft Jul 8, 2022

philliphoff Aug 17, 2022

alexweininger commented Aug 18, 2022

alexweininger commented Aug 18, 2022

philliphoff commented Aug 18, 2022

philliphoff commented Aug 18, 2022

nturinski left a comment

nturinski Aug 19, 2022

philliphoff Aug 19, 2022

nturinski Aug 19, 2022

philliphoff Aug 19, 2022

nturinski left a comment

philliphoff commented Aug 19, 2022

nturinski Aug 19, 2022

nturinski Aug 19, 2022

philliphoff Aug 19, 2022

philliphoff Aug 19, 2022

philliphoff commented Aug 19, 2022

bwateratmsft commented Aug 22, 2022

		@@ -19,6 +20,16 @@ import { showMarkdownPreviewFile } from '../../../utils/textUtils';

		const gettingStartedFileName = 'getting_started.md';

		async function fileExists(uri: vscode.Uri): Promise<boolean> {

Support for new Python language model #3235

Support for new Python language model #3235

Conversation

philliphoff commented Jul 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexweininger commented Aug 18, 2022

alexweininger commented Aug 18, 2022

philliphoff commented Aug 18, 2022

philliphoff commented Aug 18, 2022

nturinski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nturinski left a comment

Choose a reason for hiding this comment

philliphoff commented Aug 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philliphoff commented Aug 19, 2022

bwateratmsft commented Aug 22, 2022