Skip to content

feat: adding execute method to PodmanConnection class#1813

Merged
axel7083 merged 1 commit intocontainers:mainfrom
axel7083:feature/adding-execute-method-podman
Oct 1, 2024
Merged

feat: adding execute method to PodmanConnection class#1813
axel7083 merged 1 commit intocontainers:mainfrom
axel7083:feature/adding-execute-method-podman

Conversation

@axel7083
Copy link
Copy Markdown
Contributor

What does this PR do?

This PR adds execute and executeSSH method to the PodmanConnection class in AI Lab. The purpose is to centralize the code using the podman cli (E.g. model management, WSL Upload)

The new added code takes advantages of podman-desktop/podman-desktop#8891, but still support through Legacy function previous version of podman-desktop (<1.13).

I split the feature in two

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

Required for #1810

How to test this PR?

  • unit tests has been added

Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
return this.execute(connection, ['machine', 'ssh', this.getNameLegacyCompatibility(connection), ...args], options);
}

/**
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need the code for < 1.13 as the extension is already requiring API > 1.13.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not ?

We uses the latest api, but we don't have hard requirements on latest version, so we are in theory compatible with older version up to 1.8.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the catalog it says minimum 1.12 for 1.2 of ai lab

https://github.com/containers/podman-desktop-catalog/blob/gh-pages/api/extensions.json#L689

and in package.json, we consume 1.13.0 snapshot API

I'm assuming some API changes (new additions) are also missing since 1.8

so consuming 1.13 API should result in 1.13 minimal requirement

Copy link
Copy Markdown
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving as the need for the version compatibility is a separate issue

@axel7083 axel7083 merged commit 9a1421f into containers:main Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants