Enh/ap 25245 pixi env creator

[marc-lehner]

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

[Copilot]

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 PythonProcessProvider as a replacement interface for the deprecated PythonCommand to 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

[Copilot]

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 pythonEnvClass is 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 pythonEnvClass is 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.

/*

[Copilot]

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 13 comments.

[sonarqubecloud]

Quality Gate failed

Failed conditions
18.5% Coverage on New Code (required ≥ 85%)
13.2% Duplication on New Code (required ≤ 1%)

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 9 comments.

[Copilot]

The logic assumes the environment port is always the last input port and filters it by truncating the array. This is brittle with PortsConfigurationBuilder/group ordering and can silently mis-handle inputs if ordering changes. Prefer identifying the environment port by its configured PortType (or reusing the same helper approach as PortsConfigurationUtils.extractPythonEnvironmentPort(...) / filterEnvironmentPort(...)) and log the exception with stack trace (pass e) to aid diagnostics.

[Copilot]

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.

[Copilot]

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.

Suggested change

	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];
	}

[Copilot]

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.

[Copilot]

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.

[Copilot]

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes/PortsConfigurationUtils.java:1

• filterEnvironmentPort(...) assumes there is exactly one environment port and allocates inObjects.length - 1. If the config ever contains 0 (unexpected state), or more than 1 environment port (future extensibility), this produces an incorrectly sized array (either missing entries or leaving trailing nulls). A more robust approach is to compute the exact number of non-environment ports first (similar to createInputPorts) and allocate based on that count, or build a list and convert to array.

/*

[Copilot]

When the environment port exists but no command can be extracted (e.g., optional port not connected, inactive placeholder, or extraction error), dataPortObjects is not filtered and the environment port object (or placeholder) is passed into setupIO(...). This can cause Unsupported port type connected failures or source/input misalignment. Filter out the environment port whenever m_ports.hasPythonEnvironmentPort() is true (independent of extraction success), and separately decide which pythonCommand to use.

[Copilot]

This adds multiple debug log statements inside tight per-port loops (including a log on every isPythonEnvironmentPort check). Even at debug level, this can generate a lot of log traffic and overhead in workflows with many ports or frequent UI refreshes. Consider removing the per-check logs or guarding them behind an explicit LOGGER.isDebugEnabled() (if available) and logging only when an environment port is actually detected.

Suggested change

	// Skip Python environment ports - they are not data ports
	if (isPythonEnvironmentPort(type)) {
	LOGGER.debugWithFormat("Skipping environment port at index %d (type: %s) - not exposed to Python script", i, type.getName());
	continue;
	}
	// Skip Python environment ports - they are not data ports
	if (isPythonEnvironmentPort(type)) {
	if (LOGGER.isDebugEnabled()) {
	LOGGER.debugWithFormat(
	"Skipping environment port at index %d (type: %s) - not exposed to Python script",
	i, type.getName());
	}
	continue;
	}

[Copilot]

This adds multiple debug log statements inside tight per-port loops (including a log on every isPythonEnvironmentPort check). Even at debug level, this can generate a lot of log traffic and overhead in workflows with many ports or frequent UI refreshes. Consider removing the per-check logs or guarding them behind an explicit LOGGER.isDebugEnabled() (if available) and logging only when an environment port is actually detected.

Suggested change

	LOGGER.debugWithFormat("Checking if port type '%s' is environment port: %s",
	portType.getName(), isPythonEnvPort);
	return isPythonEnvPort;
	} catch (NoClassDefFoundError e) {
	// Python environment bundle not available
	LOGGER.debugWithFormat("Python environment bundle not available for port type '%s'", portType.getName());
	if (isPythonEnvPort && LOGGER.isDebugEnabled()) {
	LOGGER.debugWithFormat("Checking if port type '%s' is environment port: %s",
	portType.getName(), isPythonEnvPort);
	}
	return isPythonEnvPort;
	} catch (NoClassDefFoundError e) {
	// Python environment bundle not available
	if (LOGGER.isDebugEnabled()) {
	LOGGER.debugWithFormat("Python environment bundle not available for port type '%s'",
	portType.getName());
	}

[Copilot]

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 7 comments.

[Copilot]

The adapter is now unnecessary (and risky): PythonGatewayDescription.builder(...) has been updated to accept ExternalProcessProvider, so wrapping pythonCommand adds no value. Worse, PythonCommandAdapter.equals(...) can violate symmetry/transitivity (e.g., adapter.equals(delegate) may be true while delegate.equals(adapter) is false), which can break Map/queue lookups and eviction logic. Prefer passing pythonCommand directly to PythonGatewayDescription.builder(...) and removing PythonCommandAdapter entirely.

[Copilot]

The adapter is now unnecessary (and risky): PythonGatewayDescription.builder(...) has been updated to accept ExternalProcessProvider, so wrapping pythonCommand adds no value. Worse, PythonCommandAdapter.equals(...) can violate symmetry/transitivity (e.g., adapter.equals(delegate) may be true while delegate.equals(adapter) is false), which can break Map/queue lookups and eviction logic. Prefer passing pythonCommand directly to PythonGatewayDescription.builder(...) and removing PythonCommandAdapter entirely.

[Copilot]

filterEnvironmentPort(...) assumes there is exactly one environment port whenever the configuration contains an environment port type, and blindly allocates inObjects.length - 1. This can drop a real data port or produce an array with trailing nulls if the environment port is absent/not matched, and it breaks if there are 0 or >1 environment ports. Compute the filtered array size by counting environment-port indices in inTypes (and only excluding those indices), rather than hard-coding - 1.

[Copilot]

This adds multiple debugWithFormat(...) calls in code that is likely invoked frequently (e.g., dialog/model refresh). Even at debug level, formatted logging can become noisy/expensive and obscure other debug output. Consider removing the per-port debug logs (or gating them behind a more granular trace flag) and keep only the functional check.

Suggested change

LOGGER.debugWithFormat("Skipping environment port at index %d (type: %s) - not exposed to Python script", i, type.getName());

[Copilot]

The code still uses a raw VariableType cast ((VariableType) variable.getVariableType()), which typically triggers both unchecked and rawtypes warnings. If the intent is to suppress both, consider restoring the previous @SuppressWarnings({\"unchecked\", \"rawtypes\"}), or refactor the cast to avoid raw types if possible.

Suggested change

	private void pushNewFlowVariable(final FlowVariable variable) {
	pushFlowVariable(variable.getName(), (VariableType)variable.getVariableType(),
	variable.getValue(variable.getVariableType()));
	private <T> void pushNewFlowVariable(final FlowVariable variable) {
	final VariableType<T> type = (VariableType<T>)variable.getVariableType();
	final T value = variable.getValue(type);
	pushFlowVariable(variable.getName(), type, value);

[Copilot]

Creating a logger instance inline on every call is avoidable overhead. Prefer a private static final NodeLogger LOGGER = NodeLogger.getLogger(PythonIOUtils.class); and reuse it for logging.

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonScriptingSession.java:1

• The extractPythonEnvironmentPort method is called twice in this code block (lines 262 and 266). Store the result in a variable to avoid redundant traversal of the input array.

/*

[Copilot]

This TODO comment should be resolved. Consider checking if the environment is already installed before logging the installation message, or remove the TODO if the current behavior is acceptable.

Suggested change

// TODO only log about environment installation if it actually needs to be installed

[Copilot]

This TODO comment is vague. Specify what cleanup is needed or implement the cleanup logic. This appears to be in error handling code where proper cleanup is critical.

Suggested change

	// TODO cleanup
	// Session startup failed. At this point no new interactive session has been
	// registered in m_interactiveSession, so there are no additional resources
	// to clean up here beyond preserving the interrupted status and reporting
	// the error to the client.

[Copilot]

Remove this commented-out code block. Dead code should be eliminated rather than left in comments, especially in a version-controlled codebase.

Suggested change

	/*public static void installPythonEnvironmentIfPresent(final PortsConfiguration config, final PortObject[] inObjects,
	final ExecutionMonitor exec) throws IOException, CanceledExecutionException {
	final PythonEnvironmentPortObject envPort = extractPythonEnvironmentPort(config, inObjects);
	if (envPort != null) {
	PythonEnvironmentPortObject.installPythonEnvironmentWithProgress(config, inObjects, exec);
	}
	}*/

[Copilot]

This code assumes exactly one environment port exists, but doesn't validate this assumption. If there are zero or multiple environment ports, this will create an incorrectly-sized array and may lead to ArrayIndexOutOfBoundsException.

Suggested change

	// Find and exclude the environment port
	final PortObject[] filtered = new PortObject[inObjects.length - 1];
	int filteredIndex = 0;
	for (int i = 0; i < inObjects.length; i++) {
	if (!(inObjects[i] instanceof PythonEnvironmentPortObject)) {
	filtered[filteredIndex++] = inObjects[i];
	// First count how many environment ports are present
	int envPortCount = 0;
	for (final PortObject inObject : inObjects) {
	if (inObject instanceof PythonEnvironmentPortObject) {
	envPortCount++;
	}
	}
	// If there is no environment port, nothing to filter out
	if (envPortCount == 0) {
	return inObjects;
	}
	// Find and exclude all environment ports
	final PortObject[] filtered = new PortObject[inObjects.length - envPortCount];
	int filteredIndex = 0;
	for (final PortObject inObject : inObjects) {
	if (!(inObject instanceof PythonEnvironmentPortObject)) {
	filtered[filteredIndex++] = inObject;

[HedgehogCode]

Please move this port to the top for both nodes.

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

org.knime.python3.scripting.nodes/src/main/java/org/knime/python3/scripting/nodes2/PythonIOUtils.java:117

• The JavaDoc states that PythonEnvironmentPortObjects are filtered out, but this filtering doesn't actually happen in this method. The filtering must be done by the caller before passing the data array to this method. Update the JavaDoc to clarify that the caller is responsible for filtering out environment ports, or that this method expects only data ports (BufferedDataTable and PickledObjectFileStorePortObject).

    /**
     * Create an array of Python data sources for the given input ports. The input ports can be either a
     * {@link BufferedDataTable}, a {@link PickledObjectFileStorePortObject}, or {@link PythonEnvironmentPortObject}.
     * Note that {@link PythonEnvironmentPortObject}s are filtered out as they are only used for environment
     * configuration and not passed to Python as data.
     *
     * @param data a list of port objects. Only {@link BufferedDataTable}, {@link PickledObjectFileStorePortObject}, and
     *            {@link PythonEnvironmentPortObject} are supported.
     * @param tableConverter a table converter that is used to convert the {@link BufferedDataTable}s to Python sources
     * @param exec for progress reporting and cancellation
     * @return an array of Python data sources
     * @throws IOException if the path to a pickled file could not be created or a {@link BufferedDataTable} couldn't be
     *             written
     * @throws CanceledExecutionException if the execution was canceled
     * @throws IllegalArgumentException if the data contains unsupported port types
     */

[Copilot]

The filterEnvironmentPort method assumes exactly one environment port exists and creates an array with length - 1. If there's no environment port in the array, this will create an array that's too small and cause an ArrayIndexOutOfBoundsException. The method should either count the number of environment ports first or check if any exist before creating the filtered array.

[Copilot]

The extractPythonEnvironmentPort method is called unconditionally but always throws an exception if no environment port is found. This will cause the execute method to fail even when hasPythonEnvironmentPort() returns false. The call should only be made inside the if block at line 186, or the method should be changed to return null when no port is found.

Suggested change

	var pythonEnvironmentPort = PortsConfigurationUtils.extractPythonEnvironmentPort(inObjects);
	// Install Python environment early to avoid timeout issues during gateway connection
	final ExecutionMonitor remainingProgress;
	final ExternalProcessProvider pythonCommand;
	if (m_ports.hasPythonEnvironmentPort()) {
	// Install Python environment early to avoid timeout issues during gateway connection
	final ExecutionMonitor remainingProgress;
	final ExternalProcessProvider pythonCommand;
	if (m_ports.hasPythonEnvironmentPort()) {
	var pythonEnvironmentPort = PortsConfigurationUtils.extractPythonEnvironmentPort(inObjects);

[Copilot]

The dataPortObjects variable is never filtered to remove the environment port when hasPythonEnvironmentPort() is true. This means the environment port will be passed to setupIO, which will likely cause issues since it's not a data port. Add filtering logic similar to PythonScriptNodeModel lines 204-208 to filter out the environment port before passing to setupIO.

[Copilot]

AbstractPythonCommand now implements ExternalProcessProvider directly instead of PythonCommand. This breaks backward compatibility because concrete subclasses like SimplePythonCommand, CondaPythonCommand, and BundledPythonCommand will no longer implement the PythonCommand interface. Since PythonCommand is deprecated but marked for removal in 5.11, existing code that expects these classes to implement PythonCommand will break. AbstractPythonCommand should implement PythonCommand instead (which extends ExternalProcessProvider), and should also implement getPythonExecutablePath() which delegates to getExecutablePath().

[Copilot]

The minimum version requirement for org.knime.core.ui has been lowered from 5.11.0 to 5.10.0, and for org.knime.workbench.editor from 5.10.0 to 5.9.0. These changes appear unrelated to the PR's purpose of adding pixi environment support. Verify that these version downgrades are intentional and necessary, as they may allow the bundle to run with older, potentially incompatible versions of these dependencies.

Suggested change

	org.knime.core.ui;bundle-version="[5.10.0,6.0.0)",
	org.knime.workbench.editor;bundle-version="[5.9.0,6.0.0)",
	org.knime.core.ui;bundle-version="[5.11.0,6.0.0)",
	org.knime.workbench.editor;bundle-version="[5.10.0,6.0.0)",