Skip to content
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

CliToolUpdate & CliToolInstaller are not using the result of selectVersion #9339

Open
axel7083 opened this issue Oct 11, 2024 · 0 comments
Open

Comments

@axel7083
Copy link
Contributor

Is your enhancement related to a problem? Please describe

While working on containers/podman-desktop-extension-minikube#149 I found some weird behaviour.

Here is the interface for the CliToolInstaller from the extension api.

export interface CliToolInstaller {
selectVersion: () => Promise<string>;
doInstall: (logger: Logger) => Promise<void>;
doUninstall: (logger: Logger) => Promise<void>;
}

Here is the workflow we currently have

  1. extension register CliToolInstaller
  2. User go to CLI Page and click on install
  3. Core call selectVersion (do nothing with the result ??)
  4. Core call doInstall

The doInstall should take as argument the version got from selectVersion, currently this is not the case, and this is how extension are doing:

  let releaseToInstall: KindGithubReleaseArtifactMetadata | undefined;

 let releaseToInstall: KindGithubReleaseArtifactMetadata | undefined;
  let releaseVersionToInstall: string | undefined;
  kindCli.registerInstaller({
    selectVersion: async () => {
      const selected = await installer.promptUserForVersion();
      releaseToInstall = selected;
      return releaseVersionToInstall; // /!\ useless 
    },
    doInstall: async _logger => {
      if (!releaseToInstall) {
        throw new Error(`Cannot install ${KIND_CLI_NAME}. No release selected.`);
      }

     // install releaseToInstall

      releaseToInstall = undefined;
    },
    doUninstall: async _logger => {
      ...
    },
  });

The extension declare a scoped variable that is shared between the selectVersion and doInstall function. It increase a lot the complexity.

What happen to the return value of selectVersion

It get lost

  1. The frontend request SelectVersion

newVersion = await window.selectCliToolVersionToUpdate(cliTool.id);

  1. The CliToolRegistry ask the extension the value, and pass it down to the frontend

async selectCliToolVersionToUpdate(id: string): Promise<string> {

  1. The frontend send it back to the core

await window.updateCliTool(cliTool.id, loggerHandlerKey, newVersion, eventCollect);

  1. The core use it to create a task Updating kind to X.Y.Z

title: `Update ${tool.name} to v${version}`,

And that's where it stop.

Describe the solution you'd like

Solution proposal would be backward compatible

The CliToolInstaller should be changed to

  export interface CliToolInstaller {
    selectVersion: () => Promise<string>;
-   doInstall: (logger: Logger) => Promise<void>;
+   doInstall: (logger: Logger, version: string) => Promise<void>;
    doUninstall: (logger: Logger) => Promise<void>;
  }

And that's all :)

abvousily this should also to CliToolUpdate

  export interface CliToolSelectUpdate {
    selectVersion: () => Promise<string>;
-   doUpdate: (logger: Logger) => Promise<void>;
+   doUpdate: (logger: Logger,  version: string) => Promise<void>;
  }

Describe alternatives you've considered

No response

Additional context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

1 participant