-
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds support for a new Python environment provider node with a Pixi port object, integrating it into the Python scripting nodes ecosystem. The changes enable Python script nodes to receive Python environments through an optional input port instead of only relying on configured preferences.
Changes:
- Introduced
PythonProcessProvideras a replacement interface for the deprecatedPythonCommandto support multiple environment sources - Added optional Python environment port support to Python Script and Python View nodes
- Implemented environment port extraction and installation logic with progress reporting
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| org.knime.python3/src/main/java/org/knime/python3/PythonCommand.java | Deprecated PythonCommand interface, now extends PythonProcessProvider for backward compatibility |
| org.knime.python3/src/main/java/org/knime/python3/SimplePythonCommand.java | Updated documentation to reference PythonProcessProvider |
| org.knime.python3/src/main/java/org/knime/python3/QueuedPythonGatewayFactory.java | Refactored to use PythonProcessProvider instead of PythonCommand |
| org.knime.python3/src/main/java/org/knime/python3/PythonGatewayFactory.java | Updated gateway description to work with PythonProcessProvider |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/script/PythonScriptNodeFactory.java | Added optional Python environment port to Python Script node |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/view/PythonViewNodeFactory.java | Added optional Python environment port to Python View node |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptNodeModel.java | Implemented environment port extraction and prioritization over configured Python command |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java | Added environment port handling for interactive sessions |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java | Added PythonCommandAdapter to bridge PythonProcessProvider to legacy PythonCommand |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes/PortsConfigurationUtils.java | Added utility methods for environment port detection and installation |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonIOUtils.java | Updated to filter out environment ports from data processing |
| org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingInputOutputModelUtils.java | Added logic to skip environment ports in input/output model generation |
...pting.nodes/src/main/java/org/knime/python3/scripting/nodes2/view/PythonViewNodeFactory.java
Outdated
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/script/PythonScriptNodeFactory.java
Outdated
Show resolved
Hide resolved
...ipting.nodes/src/main/java/org/knime/python3/scripting/nodes/view/PythonViewNodeFactory.java
Outdated
Show resolved
Hide resolved
...ng.nodes/src/main/java/org/knime/python3/scripting/nodes/script/PythonScriptNodeFactory.java
Outdated
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java
Outdated
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptPortsConfiguration.java
Outdated
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptPortsConfiguration.java
Outdated
Show resolved
Hide resolved
org.knime.python3/src/main/java/org/knime/python3/PythonCommand.java
Outdated
Show resolved
Hide resolved
....nodes/src/main/java/org/knime/python3/scripting/nodes/AbstractPythonScriptingNodeModel.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes/PortsConfigurationUtils.java
Outdated
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes/PortsConfigurationUtils.java
Outdated
Show resolved
Hide resolved
...s/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingInputOutputModelUtils.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java
Outdated
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java
Outdated
Show resolved
Hide resolved
....scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptNodeModel.java
Outdated
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptPortsConfiguration.java
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptPortsConfiguration.java
Outdated
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptPortsConfiguration.java
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/script/PythonScriptNodeFactory.java:1
- The variable
pythonEnvClassis declared but never used on line 130 and 139. This appears to be a leftover from development and should be removed to reduce code clutter.
/*
org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/view/PythonViewNodeFactory.java:1
- The variable
pythonEnvClassis declared on line 91 in the view factory but never used. This appears to be a leftover from development and should be removed to reduce code clutter.
/*
...pting.nodes/src/main/java/org/knime/python3/scripting/nodes2/view/PythonViewNodeFactory.java
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/script/PythonScriptNodeFactory.java
Show resolved
Hide resolved
...nodes/src/main/java/org/knime/python3/scripting/nodes/prefs/Python3ScriptingPreferences.java
Outdated
Show resolved
Hide resolved
5297ab6 to
a396f21
Compare
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.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 13 comments.
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java
Show resolved
Hide resolved
org.knime.python3/src/main/java/org/knime/python3/PythonGatewayFactory.java
Show resolved
Hide resolved
....nodes/src/main/java/org/knime/python3/scripting/nodes/AbstractPythonScriptingNodeModel.java
Outdated
Show resolved
Hide resolved
....nodes/src/main/java/org/knime/python3/scripting/nodes/AbstractPythonScriptingNodeModel.java
Show resolved
Hide resolved
|
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.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 9 comments.
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java
Show resolved
Hide resolved
...nodes/src/main/java/org/knime/python3/scripting/nodes/prefs/Python3ScriptingPreferences.java
Show resolved
Hide resolved
...scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingService.java
Show resolved
Hide resolved
....python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonIOUtils.java
Show resolved
Hide resolved
...g.nodes/src/main/java/org/knime/python3/scripting/nodes2/script/PythonScriptNodeFactory.java
Show resolved
Hide resolved
....nodes/src/main/java/org/knime/python3/scripting/nodes/AbstractPythonScriptingNodeModel.java
Outdated
Show resolved
Hide resolved
732d9ac to
15d85da
Compare
15d85da to
3e4affb
Compare
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.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
| final PortObject[] filtered = new PortObject[inObjects.length - 1]; | ||
| int filteredIndex = 0; | ||
| for (int i = 0; i < inTypes.length && i < inObjects.length; i++) { | ||
| if (!isPythonEnvironmentPort(inTypes[i])) { | ||
| filtered[filteredIndex++] = inObjects[i]; | ||
| } | ||
| } |
Copilot
AI
Feb 11, 2026
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.
filterEnvironmentPort assumes exactly one environment port and blindly allocates inObjects.length - 1. If the configuration ever contains 0 or >1 environment ports (e.g., future changes, misconfiguration, or different port typing), this can either drop a data port or return an array containing trailing nulls (risking downstream NPEs). Allocate the filtered array by counting non-environment ports (based on inTypes) and fill it accordingly, or build a dynamic list and convert to array to guarantee correct sizing.
| final PortObject[] filtered = new PortObject[inObjects.length - 1]; | |
| int filteredIndex = 0; | |
| for (int i = 0; i < inTypes.length && i < inObjects.length; i++) { | |
| if (!isPythonEnvironmentPort(inTypes[i])) { | |
| filtered[filteredIndex++] = inObjects[i]; | |
| } | |
| } | |
| final int max = Math.min(inTypes.length, inObjects.length); | |
| // First pass: count non-environment ports within the overlapping range | |
| int nonEnvCount = 0; | |
| for (int i = 0; i < max; i++) { | |
| if (!isPythonEnvironmentPort(inTypes[i])) { | |
| nonEnvCount++; | |
| } | |
| } | |
| // Any remaining PortObjects beyond the typed range are treated as non-environment ports | |
| if (inObjects.length > max) { | |
| nonEnvCount += inObjects.length - max; | |
| } | |
| final PortObject[] filtered = new PortObject[nonEnvCount]; | |
| int filteredIndex = 0; | |
| // Second pass: copy non-environment ports from the overlapping range | |
| for (int i = 0; i < max; i++) { | |
| if (!isPythonEnvironmentPort(inTypes[i])) { | |
| filtered[filteredIndex++] = inObjects[i]; | |
| } | |
| } | |
| // Third pass: copy any remaining ports beyond the typed range | |
| for (int i = max; i < inObjects.length; i++) { | |
| filtered[filteredIndex++] = inObjects[i]; | |
| } |
| public static PythonEnvironmentPortObject extractPythonEnvironmentPort( | ||
| final PortsConfiguration config, final PortObject[] inObjects) { | ||
| final PortType[] inTypes = config.getInputPorts(); | ||
| for (int i = 0; i < inTypes.length; i++) { | ||
| if (isPythonEnvironmentPort(inTypes[i]) && PythonEnvironmentPortObject.isPythonEnvironmentPortObject(inObjects[i])) { | ||
| return (PythonEnvironmentPortObject) inObjects[i]; | ||
| } | ||
| } | ||
| return null; | ||
| } |
Copilot
AI
Feb 11, 2026
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.
This utility method hard-links to PythonEnvironmentPortObject in its signature and body even though org.knime.pixi.port is declared optional. In OSGi, such direct type references can still trigger NoClassDefFoundError at class load/link time (not just at execution), which would prevent the scripting nodes bundle from loading when the optional bundle isn’t available. To keep the dependency truly optional, avoid exposing Pixi types in public method signatures and isolate direct references behind reflection or a separate fragment/bundle that is only resolved when org.knime.pixi.port is present.
| if (m_ports.hasPythonEnvironmentPort()) { | ||
| PortsConfigurationUtils.installPythonEnvironmentIfPresent(getPortsConfiguration(), inObjects, exec); | ||
| } | ||
|
|
||
| // Extract Python command from environment port if connected, otherwise use configured command | ||
| final PythonProcessProvider pythonCommand; | ||
| final PythonProcessProvider pythonCommandFromEnv = PythonEnvironmentPortObject.extractPythonCommand( | ||
| PortsConfigurationUtils.extractPythonEnvironmentPort(getPortsConfiguration(), inObjects)); | ||
| if (pythonCommandFromEnv != null) { | ||
| LOGGER.debug("Using Python from environment port"); | ||
| pythonCommand = pythonCommandFromEnv; |
Copilot
AI
Feb 11, 2026
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.
New behavior is introduced for (1) early environment installation and (2) precedence of the environment-port-provided Python over the configured executable. Please add tests that cover: env port connected vs. not connected; extraction failure fallback to executable selection; and ensuring the environment port is not forwarded as a data input to the session (i.e., input port filtering). This helps prevent regressions given the multiple new paths and optional-dependency behavior.
3e4affb to
68c0942
Compare
Co-authored-by: Copilot <[email protected]>
68c0942 to
d461935
Compare


Add support for the new python environment provider node and it's pixi port object to the python scripts