Fix environment for interactive executors by jan-janssen · Pull Request #908 · pyiron/executorlib

jan-janssen · 2026-02-12T21:30:18Z

Summary by CodeRabbit

Refactor
- Centralized environment setup for spawned processes to improve internal structure.
Bug Fixes / Reliability
- Spawned interactive and scheduled jobs now ensure the current working directory is included in Python module search, improving discovery of local modules.
Tests
- Added/updated tests to validate environment manipulation and restoration around spawned processes.

for more information, see https://pre-commit.ci

coderabbitai · 2026-02-12T21:30:47Z

📝 Walkthrough

Walkthrough

Moved the environment-helper set_current_directory_in_environment() from src/executorlib/standalone/command.py into src/executorlib/standalone/interactive/spawner.py, updated imports across spawner/task-scheduler modules, and added unit tests exercising the new helper in the interactive spawner test suite.

Changes

Cohort / File(s)	Summary
Function relocation `src/executorlib/standalone/command.py`, `src/executorlib/standalone/interactive/spawner.py`	Removed `set_current_directory_in_environment()` from `command.py` and added it to `interactive/spawner.py`; responsibility for ensuring current dir is on `PYTHONPATH` now lives in the interactive spawner module.
Import updates (task scheduler) `src/executorlib/task_scheduler/file/spawner_pysqa.py`, `src/executorlib/task_scheduler/file/spawner_subprocess.py`, `src/executorlib/task_scheduler/interactive/spawner_flux.py`, `src/executorlib/task_scheduler/interactive/spawner_pysqa.py`	Updated imports to reference `set_current_directory_in_environment()` from `executorlib.standalone.interactive.spawner`; calls inserted where bootup/submit paths require the environment tweak.
Tests `tests/unit/standalone/interactive/test_spawner.py`, `tests/unit/standalone/test_command.py`	Added tests for `set_current_directory_in_environment()` in the interactive spawner tests; removed the corresponding export/test from `command.py` tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

[Fix] Set Pythonpath for TestClusterExecutor #907: Modifies the same set_current_directory_in_environment helper and updates callers across spawner modules.
Refactoring: Definition of task spawner #797: Related refactors moving spawner utilities and adjusting imports across task-scheduler and interactive spawner code.

Poem

🐰 I hopped from Command into Spawner’s glade,
Moved PYTHONPATH roots so imports aren’t afraid.
Tests sniff and nudge the path where I play,
Now modules find home when they start each day. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix environment for interactive executors' accurately summarizes the main change: relocating and refactoring the set_current_directory_in_environment function from the standalone.command module to standalone.interactive.spawner to fix environment setup for interactive executors.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into `main`

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch envfix

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

tests/unit/standalone/interactive/test_spawner.py (1)

550-566: Test leaks PYTHONPATH into the environment when it wasn't originally set.

When PYTHONPATH was not present before the test (i.e., python_path is None), the cleanup at lines 565-566 skips the restore, leaving the test-modified PYTHONPATH in os.environ. This can cause flaky behavior in downstream tests that depend on or check PYTHONPATH. Additionally, if an assertion fails mid-test, the cleanup code never runs.

Consider using addCleanup or setUp/tearDown for robust environment restoration:

♻️ Proposed fix for proper environment cleanup

 class TestEnvManipulation(unittest.TestCase):
     def test_set_current_directory_in_environment(self):
         env = os.environ
         if "PYTHONPATH" in env:
             python_path = env["PYTHONPATH"]
-            del env["PYTHONPATH"]
         else:
             python_path = None
+        # Register cleanup first so it runs even if assertions fail
+        def restore():
+            if python_path is not None:
+                env["PYTHONPATH"] = python_path
+            elif "PYTHONPATH" in env:
+                del env["PYTHONPATH"]
+        self.addCleanup(restore)
+        if "PYTHONPATH" in env:
+            del env["PYTHONPATH"]
         self.assertFalse("PYTHONPATH" in env)
         set_current_directory_in_environment()
         self.assertTrue("PYTHONPATH" in env)
         self.assertEqual(env["PYTHONPATH"], os.getcwd())
         env["PYTHONPATH"] = "/my/special/path"
         set_current_directory_in_environment()
         self.assertEqual(env["PYTHONPATH"], os.getcwd() + ":/my/special/path")
-        if python_path is not None:
-            env["PYTHONPATH"] = python_path

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/executorlib/standalone/interactive/spawner.py`:
- Around line 201-210: The function set_current_directory_in_environment
currently checks membership with a substring search against
environment["PYTHONPATH"], which can yield false positives/negatives; change it
to split PYTHONPATH using os.pathsep into individual entries and compare the
absolute/current_path against those entries (prefer os.path.abspath(os.getcwd())
stored in current_path) to determine if it already exists, and when
prepending/appending use os.pathsep to join paths (replace the second
os.getcwd() call with current_path). Reference:
set_current_directory_in_environment, environment, current_path, and
environment["PYTHONPATH"].

coderabbitai · 2026-02-12T21:35:38Z

src/executorlib/standalone/interactive/spawner.py

+def set_current_directory_in_environment():
+    """
+    Add the current directory to the PYTHONPATH to be able to access local Python modules.
+    """
+    environment = os.environ
+    current_path = os.getcwd()
+    if "PYTHONPATH" in environment and current_path not in environment["PYTHONPATH"]:
+        environment["PYTHONPATH"] = os.getcwd() + ":" + environment["PYTHONPATH"]
+    elif "PYTHONPATH" not in environment:
+        environment["PYTHONPATH"] = os.getcwd()


⚠️ Potential issue | 🟠 Major

Substring match on PYTHONPATH can produce false positives/negatives.

Line 207 uses current_path not in environment["PYTHONPATH"], which is a substring search on the raw string, not a check against individual path entries. For example, if cwd is /home/user/pro and PYTHONPATH is /home/user/project, the check passes incorrectly (path not added when it should be — or vice versa).

Split on os.pathsep and compare against actual path components instead.

Proposed fix

def set_current_directory_in_environment(): """ Add the current directory to the PYTHONPATH to be able to access local Python modules. """ environment = os.environ current_path = os.getcwd() - if "PYTHONPATH" in environment and current_path not in environment["PYTHONPATH"]: - environment["PYTHONPATH"] = os.getcwd() + ":" + environment["PYTHONPATH"] + if "PYTHONPATH" in environment and current_path not in environment["PYTHONPATH"].split(os.pathsep): + environment["PYTHONPATH"] = current_path + os.pathsep + environment["PYTHONPATH"] elif "PYTHONPATH" not in environment: - environment["PYTHONPATH"] = os.getcwd() + environment["PYTHONPATH"] = current_path

This also replaces the redundant second os.getcwd() call with the already-captured current_path and uses os.pathsep for cross-platform correctness.

🤖 Prompt for AI Agents

In `@src/executorlib/standalone/interactive/spawner.py` around lines 201 - 210, The function set_current_directory_in_environment currently checks membership with a substring search against environment["PYTHONPATH"], which can yield false positives/negatives; change it to split PYTHONPATH using os.pathsep into individual entries and compare the absolute/current_path against those entries (prefer os.path.abspath(os.getcwd()) stored in current_path) to determine if it already exists, and when prepending/appending use os.pathsep to join paths (replace the second os.getcwd() call with current_path). Reference: set_current_directory_in_environment, environment, current_path, and environment["PYTHONPATH"].

codecov · 2026-02-12T21:40:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.60%. Comparing base (76fab50) to head (35e2a28).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #908      +/-   ##
==========================================
+ Coverage   93.59%   93.60%   +0.01%     
==========================================
  Files          38       38              
  Lines        1888     1891       +3     
==========================================
+ Hits         1767     1770       +3     
  Misses        121      121

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jan-janssen and others added 2 commits February 12, 2026 22:29

set environment in spawner

eb2ba6c

[pre-commit.ci] auto fixes from pre-commit.com hooks

58d947f

for more information, see https://pre-commit.ci

coderabbitai bot reviewed Feb 12, 2026

View reviewed changes

fix tests

35e2a28

jan-janssen merged commit 7e68730 into main Feb 13, 2026
35 checks passed

jan-janssen deleted the envfix branch February 13, 2026 03:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix environment for interactive executors#908

Fix environment for interactive executors#908
jan-janssen merged 3 commits intomainfrom
envfix

jan-janssen commented Feb 12, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Feb 12, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Feb 12, 2026

Uh oh!

codecov bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jan-janssen commented Feb 12, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jan-janssen commented Feb 12, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 12, 2026 •

edited

Loading

codecov bot commented Feb 12, 2026 •

edited

Loading