Add TestClusterExecutor to simplify debugging of SlurmClusterExecutor and FluxClusterExecutor #714
Conversation
… and FluxClusterExecutor
for more information, see https://pre-commit.ci
|
Warning Rate limit exceeded@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA new executor class, Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant TestClusterExecutor
participant CacheDirectory
Tester->>TestClusterExecutor: Create instance with custom cache_directory
Tester->>TestClusterExecutor: Submit task foo(x)
TestClusterExecutor->>CacheDirectory: Store task result in specified cache dir
CacheDirectory-->>TestClusterExecutor: Confirm cache entry
TestClusterExecutor-->>Tester: Return task result
Tester->>CacheDirectory: Check cache data presence/absence
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
+ Coverage 96.89% 96.93% +0.04%
==========================================
Files 29 30 +1
Lines 1320 1338 +18
==========================================
+ Hits 1279 1297 +18
Misses 41 41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_testclusterexecutor.py (1)
7-13: Useimportlib.util.find_specfor cleaner availability check.The static analysis correctly identifies that
h5pyis imported but unused. Consider using the recommended approach for checking module availability.-try: - import h5py - - skip_h5py_test = False -except ImportError: - skip_h5py_test = True +import importlib.util + +skip_h5py_test = importlib.util.find_spec("h5py") is None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
executorlib/api.py(2 hunks)executorlib/executor/single.py(1 hunks)tests/test_testclusterexecutor.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
executorlib/api.py (1)
executorlib/executor/single.py (1)
TestClusterExecutor(187-353)
🪛 Ruff (0.11.9)
tests/test_testclusterexecutor.py
8-8: h5py imported but unused; consider using importlib.util.find_spec to test for availability
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: unittest_win
- GitHub Check: unittest_mpich (ubuntu-latest, 3.12)
- GitHub Check: unittest_openmpi (ubuntu-latest, 3.12)
- GitHub Check: notebooks
- GitHub Check: minimal
🔇 Additional comments (3)
executorlib/api.py (1)
8-8: LGTM!The import and export of
TestClusterExecutorare correctly implemented, making the new class available through the public API.Also applies to: 23-23
tests/test_testclusterexecutor.py (1)
23-53: Well-structured test for cache directory override behavior.The test effectively verifies that the
cache_directoryspecified in the task'sresource_dicttakes precedence over the executor's default cache directory. Running the test twice ensures consistent behavior.executorlib/executor/single.py (1)
296-354: Implementation looks good!The
TestClusterExecutorimplementation correctly provides two execution modes:
- File-based execution using
create_file_executorfor testing cluster-like communication- Dependency graph plotting mode using
DependencyTaskSchedulerfor debuggingThe resource dictionary setup and parameter handling are well-structured.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Based on #708 and the discussion in pyiron/pyiron_workflow#678
Summary by CodeRabbit