-
Notifications
You must be signed in to change notification settings - Fork 38
Enh/ap 25245 pixi env creator #47
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
Open
marc-lehner
wants to merge
10
commits into
master
Choose a base branch
from
enh/AP-25245-pixi-env-creator
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
0b2e88f
AP-25245: add pixi port to python scripting node
marc-lehner cf05f86
AP-25245: move ports to dedicated plugin
marc-lehner 366d73f
AP-25245: use pixi run in pixiPythonCommand for all pixi environment …
marc-lehner bbe5ab0
AP-25245: add pixi port to python view
marc-lehner 2cc6dfc
AP-25245: add pixi port to interactive execution
marc-lehner 8b0dbff
AP-25245: replace pixi port with python port
marc-lehner 23b735b
AP-25245: add progress for the python port installation and make it c…
marc-lehner 18c1007
AP-25245: deprecate PythonCommand and use PythonProcessProvider from …
marc-lehner 9d64458
AP-25245: only use import PixiPythonCommand from knime-conda (import …
marc-lehner 3ae27b1
AP-25245: WIP: use debug instead of info log for trivial messages. Co…
chaubold File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,8 @@ | |
| import org.knime.core.util.PathUtils; | ||
| import org.knime.core.util.asynclose.AsynchronousCloseableTracker; | ||
| import org.knime.core.webui.node.view.NodeView; | ||
| import org.knime.pixi.port.PixiPythonCommand; | ||
| import org.knime.pixi.port.PythonEnvironmentPortObject; | ||
| import org.knime.python2.PythonCommand; | ||
| import org.knime.python2.PythonModuleSpec; | ||
| import org.knime.python2.PythonVersion; | ||
|
|
@@ -101,6 +103,7 @@ | |
| import org.knime.python2.ports.OutputPort; | ||
| import org.knime.python2.ports.PickledObjectOutputPort; | ||
| import org.knime.python2.ports.Port; | ||
| import org.knime.python3.processprovider.PythonProcessProvider; | ||
| import org.knime.python3.scripting.Python3KernelBackend; | ||
| import org.knime.python3.scripting.nodes.prefs.Python3ScriptingPreferences; | ||
|
|
||
|
|
@@ -155,6 +158,8 @@ static void setExpectedOutputView(final PythonKernel kernel, final boolean expec | |
|
|
||
| private final boolean m_hasView; | ||
|
|
||
| private final boolean m_hasPixiPort; | ||
|
|
||
| private String m_script; | ||
|
|
||
| private final PythonCommandConfig m_command = createCommandConfig(); | ||
|
|
@@ -165,11 +170,12 @@ static void setExpectedOutputView(final PythonKernel kernel, final boolean expec | |
| new AsynchronousCloseableTracker<>(t -> LOGGER.debug("Kernel shutdown failed.", t)); | ||
|
|
||
| protected AbstractPythonScriptingNodeModel(final InputPort[] inPorts, final OutputPort[] outPorts, | ||
| final boolean hasView, final String defaultScript) { | ||
| super(toPortTypes(inPorts), toPortTypes(outPorts)); | ||
| final boolean hasView, final boolean hasPixiPort, final String defaultScript) { | ||
| super(toPortTypes(inPorts, hasPixiPort), toPortTypes(outPorts)); | ||
| m_inPorts = inPorts; | ||
| m_outPorts = outPorts; | ||
| m_hasView = hasView; | ||
| m_hasPixiPort = hasPixiPort; | ||
| m_view = Optional.empty(); | ||
| m_script = defaultScript; | ||
| } | ||
|
|
@@ -178,6 +184,23 @@ private static final PortType[] toPortTypes(final Port[] ports) { | |
| return Arrays.stream(ports).map(Port::getPortType).toArray(PortType[]::new); | ||
| } | ||
|
|
||
| private static final PortType[] toPortTypes(final Port[] ports, final boolean hasPixiPort) { | ||
| if (!hasPixiPort) { | ||
| return toPortTypes(ports); | ||
| } | ||
| // Add the optional Python environment port at the end of the input ports | ||
| final PortType[] portTypes = new PortType[ports.length + 1]; | ||
| for (int i = 0; i < ports.length; i++) { | ||
| portTypes[i] = ports[i].getPortType(); | ||
| } | ||
| try { | ||
| portTypes[ports.length] = PythonEnvironmentPortObject.TYPE_OPTIONAL; | ||
| } catch (NoClassDefFoundError e) { | ||
| throw new IllegalStateException("Could not load PythonEnvironmentPortObject class", e); | ||
| } | ||
| return portTypes; | ||
| } | ||
|
|
||
| @Override | ||
| protected void saveSettingsTo(final NodeSettingsWO settings) { | ||
| saveScriptTo(m_script, settings); | ||
|
|
@@ -198,14 +221,25 @@ protected void loadValidatedSettingsFrom(final NodeSettingsRO settings) throws I | |
|
|
||
| @Override | ||
| protected PortObjectSpec[] configure(final PortObjectSpec[] inSpecs) throws InvalidSettingsException { | ||
| for (int i = 0; i < m_inPorts.length; i++) { | ||
| // The Pixi port (if present) is at the end of the input specs | ||
| final int numRegularPorts = m_inPorts.length; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should drop the pixi port here, right? |
||
| for (int i = 0; i < numRegularPorts; i++) { | ||
| m_inPorts[i].configure(inSpecs[i]); | ||
| } | ||
| return null; // NOSONAR Conforms to KNIME API. | ||
| } | ||
|
|
||
| @Override | ||
| protected PortObject[] execute(final PortObject[] inObjects, final ExecutionContext exec) throws Exception { | ||
| // Extract Pixi environment if present | ||
| // The Pixi port (if present) is at the end of the input objects | ||
| final PythonCommand pythonCommandFromPixi; | ||
| if (m_hasPixiPort && inObjects.length > m_inPorts.length) { | ||
| pythonCommandFromPixi = extractPythonCommandFromPixiPort(inObjects[inObjects.length - 1]); | ||
| } else { | ||
| pythonCommandFromPixi = null; | ||
| } | ||
|
|
||
| double inWeight = 0d; | ||
| final Set<PythonModuleSpec> requiredAdditionalModules = new HashSet<>(); | ||
| for (int i = 0; i < m_inPorts.length; i++) { | ||
|
|
@@ -217,7 +251,8 @@ protected PortObject[] execute(final PortObject[] inObjects, final ExecutionCont | |
|
|
||
| final var cancelable = new PythonExecutionMonitorCancelable(exec); | ||
| try (final PythonKernel kernel = | ||
| getNextKernelFromQueue(requiredAdditionalModules, Collections.emptySet(), cancelable)) { | ||
| getNextKernelFromQueue(requiredAdditionalModules, Collections.emptySet(), cancelable, | ||
| pythonCommandFromPixi)) { | ||
| final Collection<FlowVariable> inFlowVariables = | ||
| getAvailableFlowVariables(Python3KernelBackend.getCompatibleFlowVariableTypes()).values(); | ||
| kernel.putFlowVariables(null, inFlowVariables); | ||
|
|
@@ -349,10 +384,70 @@ private static Path persistedViewPath(final File nodeInternDir) { | |
| return nodeInternDir.toPath().resolve("view.html"); | ||
| } | ||
|
|
||
| /** | ||
| * Extract the Python command from a PythonEnvironmentPortObject. | ||
| * | ||
| * @param portObject the port object (may be null if optional port is not connected) | ||
| * @return the Python command, or null if the port is not connected or doesn't contain a valid Python executable | ||
| * @throws InvalidSettingsException if the Python executable path from the Pixi environment doesn't exist | ||
| */ | ||
| private static PythonCommand extractPythonCommandFromPixiPort(final PortObject portObject) | ||
| throws InvalidSettingsException { | ||
| if (portObject == null) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| if (!(portObject instanceof PythonEnvironmentPortObject)) { | ||
| return null; | ||
| } | ||
|
|
||
| // Handle PythonEnvironmentPortObject | ||
| final PythonEnvironmentPortObject pythonEnvPort = (PythonEnvironmentPortObject)portObject; | ||
| final Path pixiTomlPath; | ||
| try { | ||
| pixiTomlPath = pythonEnvPort.getPixiEnvironmentPath().resolve("pixi.toml"); | ||
| } catch (IOException e) { | ||
| throw new InvalidSettingsException("Failed to get pixi.toml path from PythonEnvironmentPortObject: " + e.getMessage(), e); | ||
| } | ||
|
|
||
| // Create PixiPythonCommand from the pixi.toml path | ||
| final PythonProcessProvider pythonCommand = new PixiPythonCommand(pixiTomlPath); | ||
|
|
||
| // Verify that the Python executable exists | ||
| final Path pythonExecPath = pythonCommand.getPythonExecutablePath(); | ||
| if (!Files.exists(pythonExecPath)) { | ||
| throw new InvalidSettingsException( | ||
| "The Python executable from the Pixi environment does not exist at path: " + pythonExecPath | ||
| + ". Please check that the Pixi environment was created successfully."); | ||
| } | ||
|
|
||
| LOGGER.debug("Using Python from Pixi environment via pixi run: " + pythonCommand); | ||
| return new LegacyPythonCommand(pythonCommand); | ||
|
|
||
| } catch (NoClassDefFoundError e) { | ||
| // Python environment bundle is not available - this is fine since it's optional | ||
| LOGGER.debug("PythonEnvironmentPortObject class not available - bundle may not be installed", e); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| protected PythonKernel getNextKernelFromQueue(final Set<PythonModuleSpec> requiredAdditionalModules, | ||
| final Set<PythonModuleSpec> optionalAdditionalModules, final PythonCancelable cancelable) | ||
| throws PythonCanceledExecutionException, PythonIOException { | ||
| return PythonKernelQueue.getNextKernel(m_command.getCommand(), PythonKernelBackendType.PYTHON3, | ||
| return getNextKernelFromQueue(requiredAdditionalModules, optionalAdditionalModules, cancelable, null); | ||
| } | ||
|
|
||
| protected PythonKernel getNextKernelFromQueue(final Set<PythonModuleSpec> requiredAdditionalModules, | ||
| final Set<PythonModuleSpec> optionalAdditionalModules, final PythonCancelable cancelable, | ||
| final PythonCommand pythonCommandFromPixi) | ||
| throws PythonCanceledExecutionException, PythonIOException { | ||
| // Use Python command from Pixi port if available | ||
| // TODO: We might want to consider flow variables in addition to the Pixi port in the future | ||
| final PythonCommand commandToUse = | ||
| pythonCommandFromPixi != null ? pythonCommandFromPixi : m_command.getCommand(); | ||
|
|
||
| return PythonKernelQueue.getNextKernel(commandToUse, PythonKernelBackendType.PYTHON3, | ||
| requiredAdditionalModules, optionalAdditionalModules, new PythonKernelOptions(), cancelable); | ||
| } | ||
|
|
||
|
|
@@ -381,14 +476,14 @@ private void pushNewFlowVariable(final FlowVariable variable) { | |
| } | ||
|
|
||
| /** | ||
| * Wraps a {@link org.knime.python3.PythonCommand} into the legacy implementation for using it in a | ||
| * Wraps a {@link org.knime.pixi.port.PythonProcessProvider} into the legacy implementation for using it in a | ||
| * {@link PythonKernelBackend}. | ||
| */ | ||
| private static final class LegacyPythonCommand implements PythonCommand { | ||
|
|
||
| private final org.knime.python3.PythonCommand m_pythonCommand; | ||
| private final PythonProcessProvider m_pythonCommand; | ||
|
|
||
| private LegacyPythonCommand(final org.knime.python3.PythonCommand pythonCommand) { | ||
| private LegacyPythonCommand(final PythonProcessProvider pythonCommand) { | ||
| m_pythonCommand = pythonCommand; | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why do we do it like this, and don't add a dependency to the plugin
org.knime.python3.processprovider? Then we can also get rid of all theNoClassDefFoundchecks