Add func workload install/uninstall commands#4926
Conversation
944c298 to
1641c81
Compare
93644cd to
a08e12f
Compare
There was a problem hiding this comment.
Two primary suggestions:
- Bring in NuGet library directly, use its types for loading and parsing a manifest.
- For first iteration of
install, if you don't want to support feeds then add support for.nupkgon disk only. Don't add--versionor--fromyet, justfunc workload install <package-on-disk>.nupkg, we will detect "on disk" via the.nupkgextension.- nuget libraries probably have tooling to work with a
.nupkgon disk directly, like extracting manifest from it.
- nuget libraries probably have tooling to work with a
b43b2db to
e83e899
Compare
jviau
left a comment
There was a problem hiding this comment.
I think some of these comments are already discussed in the sync we had,
| Description = "Package ID of the workload to uninstall.", | ||
| }; | ||
|
|
||
| public Option<string?> VersionOption { get; } = new("--version") |
There was a problem hiding this comment.
Should we add short-names for these options? -v for this one. Maybe -a for --all-versions?
There was a problem hiding this comment.
-v is global verbose (we can discuss if we dont want this) - but -a for all makes sense
There was a problem hiding this comment.
Actually let's support -v and maybe just switch to diagnostics | -d instead of verbose for global?
| var version = nuspec.GetVersion().ToNormalizedString(); | ||
| var title = nuspec.GetTitle(); | ||
| var description = nuspec.GetDescription(); | ||
| var aliases = SplitTags(nuspec.GetTags()); |
There was a problem hiding this comment.
Do we want to treat all tags as aliases? I think this would prevent workloads from using tags for other means. We should probably have a convention. Aliases are alias:<name> or workload_alias:<name>
There was a problem hiding this comment.
Thanks for catching! Should be on alias:, will make sure to update that
| } | ||
| catch (Exception ex) when (ex is InvalidDataException or PackagingException) | ||
| { | ||
| throw new GracefulException( |
There was a problem hiding this comment.
Do we want to include the inner exception?
| private readonly IWorkloadEntryPointScanner _scanner = scanner ?? throw new ArgumentNullException(nameof(scanner)); | ||
|
|
||
| /// <inheritdoc /> | ||
| public async Task<InstalledWorkload> InstallFromPackageAsync( |
There was a problem hiding this comment.
We need to settle on the nupkg format. Nupkg can have many folders. Do we have any requirements on what folders, if any, they must put their entry point into?
We synced offline: will require packages have a workload.json.
There was a problem hiding this comment.
Will operate on workload.json at root for now - Fabio is working on the nupkg format
Installs workloads from a local .nupkg via NuGet.Packaging. The .nupkg is extracted into ~/.azure-functions/workloads/<id>/<version>/, validated against workload.json via IWorkloadMetadataReader, and recorded in the global registry. Uninstall removes both the registry entry and the on-disk install directory. Surface: func workload install <package> # currently a path to a .nupkg func workload uninstall <id> [--version <v>] [-a|--all-versions] Aliases on the registry entry come from nuspec tags prefixed with 'alias:' so workloads can keep using other tags for marketplace search, categories, etc. without leaking into the CLI alias surface.
dc6bfce to
cfa1257
Compare
Resolves #4898
Resolves #4899
Stacked on #4916 (workload entry-point discovery). Builds the install pipeline that consumes the scanner.
What lands
NuspecReader— XDocument-based, namespace-agnostic; reads minimal package metadata (id, version, package types, tags as aliases).WorkloadInstaller— pipeline:.nuspecfrom staging dir.FuncCliWorkloadpackage type is present (per spec PR Add func workload design proposal #4923).IWorkloadPaths.GetInstallDirectory(packageId, version).IWorkloadentry point (via PR Add workload entry-point discovery (attribute + scanner) #4916'sIWorkloadEntryPointScanner) before the move, so failures don't pollute the final install location.Directory.Movestaging → install dir.func workload install --from <dir>— file-based install for now. NuGet feed acquisition is a follow-up; the resolver lands on top of this sameIWorkloadInstaller.func workload uninstall <packageId> [--version <v>] [--all]—--alland--versionare mutually exclusive. With neither flag: succeeds when exactly one version is installed; otherwise lists versions and asks the user to disambiguate.Wired into DI through a new
WorkloadInstallRegistration.AddWorkloadInstaller()extension called fromProgram.cs.Out of scope (follow-ups)
func workload install <packageId>taking a package id rather than a directory)..nupkgfile input (extract-to-temp + install). Trivial wrapper on top of this PR.WorkloadCommandownership refactor (PR-D in the stack).Tests
28 new tests, all 116 in the suite passing: