revise option to control number of resubmit fail jobs #545 #554
Conversation
- add option: "strategy": {"customized_script_header_template_file": ""},
- add option: `sge_pe_name`
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Update pbs.py
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This reverts commit c8b0198.
fetch origin master
📝 WalkthroughWalkthroughThis PR moves Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
User->>Machine: __init__(context, retry_count=3)
Machine->>Machine: set self.retry_count = retry_count
rect rgb(200,220,240)
Note over ChildMachine: Old (per-class retry read)
ChildMachine->>remote_profile: remote_profile.get("retry_count", 3)
end
rect rgb(220,240,200)
Note over NewFlow: New (parent-managed)
User->>Bohrium: __init__(context, retry_count=X, **kwargs)
Bohrium->>Machine: super().__init__(context=context, retry_count=X, **kwargs)
Machine->>Machine: set self.retry_count = X
end
User->>Machine: serialize()
Machine-->>User: {..., "retry_count": 3}
User->>Machine: load_from_dict({... "retry_count": N ...})
Machine->>Machine: extract retry_count = N
Machine->>ChildClass: ChildClass(context=ctx, retry_count=N)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dpdispatcher/machine.py (1)
78-100: Add type hint for retry_count parameter.The retry_count parameter is correctly added to the base Machine class with proper default value, serialization, and documentation. However, it's missing a type hint, which violates the coding guideline: "Always add type hints in all Python code".
Apply this diff to add the type hint:
def __init__( self, batch_type=None, context_type=None, local_root=None, remote_root=None, remote_profile={}, - retry_count=3, + retry_count: int = 3, *, context=None, ):Note: While this change addresses the new parameter, consider adding type hints to all parameters in a follow-up refactor for full compliance with coding guidelines.
As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks).vscode/settings.json(1 hunks)dpdispatcher/machine.py(6 hunks)dpdispatcher/machines/dp_cloud_server.py(1 hunks)dpdispatcher/machines/openapi.py(1 hunks)dpdispatcher/machines/pbs.py(2 hunks)tests/test_argcheck.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Always add type hints in all Python code
Ensure code remains compatible with Python 3.7+ (avoid syntax/features newer than 3.7)
Use Ruff (via pre-commit) for linting and formatting
Files:
dpdispatcher/machine.pytests/test_argcheck.pydpdispatcher/machines/dp_cloud_server.pydpdispatcher/machines/pbs.pydpdispatcher/machines/openapi.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the unittest framework for tests with coverage reporting
Files:
tests/test_argcheck.py
.gitignore
📄 CodeRabbit inference engine (AGENTS.md)
Ensure temporary job execution artifacts remain ignored by version control
Files:
.gitignore
🧠 Learnings (1)
📚 Learning: 2025-10-02T19:23:22.456Z
Learnt from: CR
Repo: deepmodeling/dpdispatcher PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-02T19:23:22.456Z
Learning: Applies to .gitignore : Ensure temporary job execution artifacts remain ignored by version control
Applied to files:
.gitignore
🧬 Code graph analysis (1)
dpdispatcher/machine.py (1)
dpdispatcher/utils/dpcloudserver/client.py (1)
get(47-48)
⏰ 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). (4)
- GitHub Check: test (3.7, macos-15-intel)
- GitHub Check: test (3.7, windows-latest)
- GitHub Check: build (pbs)
- GitHub Check: build (slurm)
🔇 Additional comments (5)
.gitignore (1)
43-43: LGTM! Selective VSCode settings tracking is appropriate.The exception to track
.vscode/settings.jsonwhile ignoring other VSCode files is a good practice for sharing project-specific editor configuration across the team.tests/test_argcheck.py (1)
30-30: LGTM! Test correctly validates retry_count serialization.The addition of
"retry_count": 3to the expected serialized output correctly reflects the new retry_count parameter added to the base Machine class with its default value of 3.dpdispatcher/machine.py (3)
153-154: LGTM! Correct implementation of retry_count deserialization.The code correctly reads
retry_countfrom the configuration dictionary with a default of 3 and passes it to the machine constructor. This aligns well with the serialization logic at line 167.
167-167: LGTM! Correct implementation of retry_count serialization.The serialization correctly includes
retry_countin the output dictionary, maintaining symmetry with the deserialization logic at lines 153-154.
403-421: LGTM! Proper documentation and argument metadata for retry_count.The retry_count parameter is well-documented with clear description, appropriate type (int), correct default value (3), and proper integration into the argument schema. This ensures users understand the purpose and usage of this configuration option.
| def __init__(self, context, **kwargs): | ||
| super().__init__(context=context, **kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type hints to comply with coding guidelines.
The refactoring correctly delegates retry_count handling to the base class by accepting and forwarding **kwargs. However, the method signature is missing type hints, which violates the project's coding guideline: "Always add type hints in all Python code".
Apply this diff to add type hints:
- def __init__(self, context, **kwargs):
+ def __init__(self, context: BaseContext, **kwargs: Any) -> None:
super().__init__(context=context, **kwargs)You may need to add the import at the top of the file if not already present:
from typing import AnyAs per coding guidelines.
🤖 Prompt for AI Agents
In dpdispatcher/machines/dp_cloud_server.py around lines 22 to 23, the __init__
method lacks type hints; update the signature to include types for context and
kwargs (e.g., context: Any, **kwargs: Any) and ensure you import Any from typing
at the top of the file if not already present; keep the body unchanged and
continue forwarding kwargs to super().__init__(context=context, **kwargs).
| def __init__(self, context, **kwargs): | ||
| super().__init__(context=context, **kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type hints to comply with coding guidelines.
The refactoring correctly delegates retry_count handling to the base class. However, type hints are missing from the method signature, violating the coding guideline: "Always add type hints in all Python code".
Apply this diff to add type hints:
- def __init__(self, context, **kwargs):
+ def __init__(self, context: BaseContext, **kwargs: Any) -> None:
super().__init__(context=context, **kwargs)Add the import if not already present:
from typing import AnyAs per coding guidelines.
🤖 Prompt for AI Agents
In dpdispatcher/machines/openapi.py around lines 32 to 33, the __init__ method
lacks type hints; add "from typing import Any" to the imports if missing, and
update the signature to include types (e.g., def __init__(self, context: Any,
**kwargs: Any) -> None:) so the method and its kwargs are properly annotated
while preserving the existing super() call.
dpdispatcher/machines/pbs.py
Outdated
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type hints to comply with coding guidelines.
The PBS constructor correctly forwards keyword arguments to the base class. However, type hints are missing, violating the coding guideline: "Always add type hints in all Python code".
Apply this diff:
- def __init__(self, **kwargs):
+ def __init__(self, **kwargs: Any) -> None:
super().__init__(**kwargs)Add the import at the top of the file if not already present:
from typing import AnyAs per coding guidelines.
🤖 Prompt for AI Agents
In dpdispatcher/machines/pbs.py around lines 20 to 21, the __init__ method lacks
type hints; add "from typing import Any" at the top of the file (if missing) and
change the constructor signature to include types (for example: def
__init__(self, **kwargs: Any) -> None:) while leaving the super() call intact so
keyword args are forwarded.
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type hints to comply with coding guidelines.
The SGE constructor correctly forwards keyword arguments to the base class. However, type hints are missing, violating the coding guideline: "Always add type hints in all Python code".
Apply this diff:
- def __init__(self, **kwargs):
+ def __init__(self, **kwargs: Any) -> None:
super().__init__(**kwargs)Ensure the Any type is imported at the top of the file (should be added only once if not present):
from typing import AnyAs per coding guidelines.
🤖 Prompt for AI Agents
In dpdispatcher/machines/pbs.py around lines 194 to 195, the __init__ method
lacks type hints; update its signature to include typed kwargs (e.g., def
__init__(self, **kwargs: Any) -> None:) and ensure you add from typing import
Any at the top of the file if it is not already imported; keep the call to
super().__init__(**kwargs) unchanged.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #554 +/- ##
===========================================
- Coverage 60.38% 47.64% -12.75%
===========================================
Files 39 39
Lines 3908 3912 +4
===========================================
- Hits 2360 1864 -496
- Misses 1548 2048 +500 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
87-94: Ruff configuration properly aligned with project standards.The selection of rule categories (E, F, D, UP, I) and line-length = 88 align well with the existing black/isort configuration. However, the [tool.ruff] section placement at line 119 (after [tool.ruff.lint.pydocstyle]) is valid TOML but unconventional.
Consider consolidating ruff configuration by moving the [tool.ruff] section closer to the start of ruff-related settings (before [tool.ruff.lint]) for better readability, though the current structure is functionally valid:
-[tool.ruff.lint] +[tool.ruff] +line-length = 88 + +[tool.ruff.lint] select = [This keeps all ruff configuration together hierarchically.
Also applies to: 119-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
pyproject.toml: Use setuptools as the build system configured via pyproject.toml
Configure and run Pyright for static type analysis via settings in pyproject.toml
Files:
pyproject.toml
🧠 Learnings (2)
📚 Learning: 2025-10-02T19:23:22.456Z
Learnt from: CR
Repo: deepmodeling/dpdispatcher PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-02T19:23:22.456Z
Learning: Applies to examples/dpdisp_run.py : Keep the example script examples/dpdisp_run.py as a runnable example using PEP 723 script metadata
Applied to files:
pyproject.toml
📚 Learning: 2025-10-02T19:23:22.456Z
Learnt from: CR
Repo: deepmodeling/dpdispatcher PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-02T19:23:22.456Z
Learning: Applies to **/*.py : Use Ruff (via pre-commit) for linting and formatting
Applied to files:
pyproject.toml
⏰ 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). (4)
- GitHub Check: test (3.7, macos-15-intel)
- GitHub Check: build (slurm)
- GitHub Check: build (ssh_rsync)
- GitHub Check: build (pbs)
🔇 Additional comments (2)
pyproject.toml (2)
9-9: TOML inline array formatting looks good.The reformatting from multi-line to inline arrays for authors, keywords, and optional dependencies maintains readability while improving compactness.
Also applies to: 33-41, 68-69
87-120: Ruff configuration addition aligns with project guidelines.The addition of ruff linting rules and line-length configuration supports the learning guideline to "Use Ruff (via pre-commit) for linting and formatting." The configuration is well-structured and complements the existing Pyright type-checking setup (lines 77-82).
2 participants
reopen PR #545 due to branch removed
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Chores