-
Notifications
You must be signed in to change notification settings - Fork 140
Basic support for extension bundles #1204
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
Conversation
ping @StephenWeatherford @PrashanthCorp got some work that builds off this PR and would like it reviewed ASAP |
// https://github.com/Microsoft/vscode-azurefunctions/issues/1202 | ||
hostJson.extensionBundle = { | ||
id: 'Microsoft.Azure.Functions.ExtensionBundle', | ||
version: '[1.*, 2.0.0)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a line that needs to be changed often. Is there an API call you can make to reduce how much this needs to be maintained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See linked issue a few lines up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was writing in the context of #1202. If they bump up the version often, does it make sense to ask an API endpoint to poll for the supported versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That issue is about getting it from an api. Specifically this one:
https://functionscdn.azureedge.net/public/cli-feed-v3.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well ideally the cli-feed will provide this property anyway, right? We only write it manually if it didn't exist?
Aka logic related to "function.json", "local.settings.json", "host.json", etc.
const tasks: TaskDefinition[] = [ | ||
{ | ||
type: func, | ||
command: hostStartCommand, | ||
problemMatcher: funcWatchProblemMatcher, | ||
isBackground: true, | ||
dependsOn: extInstallTaskName | ||
dependsOn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is an extInstallCommand
the dependsOn property would just have the venv name, wouldn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -196,8 +194,7 @@ export function getPowerShellValidateOptions(): IValidateProjectOptions { | |||
expectedSettings: { | |||
projectLanguage: ProjectLanguage.PowerShell, | |||
projectRuntime: ProjectRuntime.v2, | |||
deploySubpath: '.', | |||
preDeployTask: 'func: extensions install' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Does this PR really remove the dependency of dotnet SDK? What the Azure Function VSCode extension do for the |
This only affects new projects. When you say There will also be more documentation on this feature coming in the near future. |
No more dependency on .NET Core!!! Instead of running
func extensions install
in every project, users just have to specify a default bundle version in their "host.json" file and it will pull down the right binaries for them.This PR will only affect new projects going forward. In the future, we might consider prompting users to "convert" their project from
func extensions install
to bundles, but I'd rather let this bake a bit before we do that.There's still some pending open questions being discussed offline. Tracking that here: #1203 #1202