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

feat: adding execute method to PodmanConnection class #1813

Merged

Conversation

axel7083
Copy link
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

return this.execute(connection, ['machine', 'ssh', this.getNameLegacyCompatibility(connection), ...args], options);
}

/**
Copy link
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
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
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
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
6 checks passed
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