Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions src/executorlib/executor/flux.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,16 @@
check_wait_on_shutdown,
validate_number_of_cores,
)
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
from executorlib.task_scheduler.interactive.blockallocation import (
BlockAllocationTaskScheduler,
)
from executorlib.task_scheduler.interactive.dependency import DependencyTaskScheduler
from executorlib.task_scheduler.interactive.onetoone import OneProcessTaskScheduler

try:
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
except ImportError:
from executorlib.task_scheduler.base import validate_resource_dict
from executorlib.task_scheduler.base import (
validate_resource_dict as validate_resource_dict_with_optional_keys,
)


class FluxJobExecutor(BaseExecutor):
"""
Expand Down
15 changes: 4 additions & 11 deletions src/executorlib/executor/single.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,16 @@
validate_number_of_cores,
)
from executorlib.standalone.interactive.spawner import MpiExecSpawner
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
from executorlib.task_scheduler.interactive.blockallocation import (
BlockAllocationTaskScheduler,
)
from executorlib.task_scheduler.interactive.dependency import DependencyTaskScheduler
from executorlib.task_scheduler.interactive.onetoone import OneProcessTaskScheduler

try:
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
except ImportError:
from executorlib.task_scheduler.base import validate_resource_dict
from executorlib.task_scheduler.base import (
validate_resource_dict as validate_resource_dict_with_optional_keys,
)


class SingleNodeExecutor(BaseExecutor):
"""
Expand Down
15 changes: 4 additions & 11 deletions src/executorlib/executor/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
check_wait_on_shutdown,
validate_number_of_cores,
)
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
from executorlib.task_scheduler.interactive.blockallocation import (
BlockAllocationTaskScheduler,
)
Expand All @@ -20,17 +24,6 @@
validate_max_workers,
)

try:
from executorlib.standalone.validate import (
validate_resource_dict,
validate_resource_dict_with_optional_keys,
)
except ImportError:
from executorlib.task_scheduler.base import validate_resource_dict
from executorlib.task_scheduler.base import (
validate_resource_dict as validate_resource_dict_with_optional_keys,
)


class SlurmClusterExecutor(BaseExecutor):
"""
Expand Down
31 changes: 27 additions & 4 deletions src/executorlib/standalone/validate.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import warnings
from typing import Optional

from pydantic import BaseModel, Extra
try:
from pydantic import BaseModel, Extra
Comment on lines +4 to +5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🌐 Web query:

pydantic v2 ConfigDict replace Extra Config class

💡 Result:

In Pydantic v2, the old inner class Config: (and Extra) is replaced by model_config = ConfigDict(...) (the Config class is still supported but deprecated). (docs.pydantic.dev)

v1 → v2 mapping for extra

v1

from pydantic import BaseModel, Extra

class M(BaseModel):
    class Config:
        extra = Extra.forbid   # or allow / ignore

v2 (recommended)

from pydantic import BaseModel, ConfigDict

class M(BaseModel):
    model_config = ConfigDict(extra='forbid')  # or 'allow' / 'ignore'

Pydantic v2 supports exactly these values: 'ignore' (default), 'allow', 'forbid'. (docs.pydantic.dev)

Alternative v2 syntax (class kwargs)

from pydantic import BaseModel

class M(BaseModel, extra='forbid'):
    ...

(docs.pydantic.dev)

If you used extra='allow' and need the extras

In v2, allowed extras are stored in __pydantic_extra__ (and may also be accessible as attributes). (docs.pydantic.dev)

Citations:


🏁 Script executed:

head -50 src/executorlib/standalone/validate.py

Repository: pyiron/executorlib

Length of output: 1465


Use Pydantic V2's ConfigDict instead of deprecated Extra and inner Config class

The code imports Pydantic V1 APIs (Extra and inner class Config) which are deprecated in Pydantic V2. With pydantic 2.12.4, these patterns emit PydanticDeprecatedSince20 warnings on every module load. Since _get_accepted_keys() already uses the V2 model_fields attribute, the codebase is V2-aware but the configuration pattern hasn't been modernized.

Replace with Pydantic V2's native ConfigDict:

♻️ Refactor to V2 API
-try:
-    from pydantic import BaseModel, Extra
-
-    HAS_PYDANTIC = True
-except ImportError:
+try:
+    from pydantic import BaseModel, ConfigDict
+
+    HAS_PYDANTIC = True
+except ImportError:
     from dataclasses import dataclass

     BaseModel = object
-    Extra = None
+    ConfigDict = None
     HAS_PYDANTIC = False
     if HAS_PYDANTIC:
-
-        class Config:
-            extra = Extra.forbid
+        model_config = ConfigDict(extra="forbid")

Also applies to: lines 29–32.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/executorlib/standalone/validate.py` around lines 4 - 5, Replace the
Pydantic v1 config pattern: change the import to use ConfigDict (e.g. from
pydantic import BaseModel, ConfigDict) and remove any inner class Config
definitions in the BaseModel subclasses in this file; instead add model_config =
ConfigDict(extra="ignore") (or extra=Extra.ignore if you still import Extra) on
those model classes so the code uses Pydantic v2 configuration; keep
_get_accepted_keys() using model_fields as-is.


HAS_PYDANTIC = True
except ImportError:
from dataclasses import dataclass

BaseModel = object
Extra = None
HAS_PYDANTIC = False


class ResourceDictValidation(BaseModel):
Expand All @@ -17,16 +26,30 @@ class ResourceDictValidation(BaseModel):
priority: Optional[int] = None
slurm_cmd_args: Optional[list[str]] = None

class Config:
extra = Extra.forbid
if HAS_PYDANTIC:

class Config:
extra = Extra.forbid


if not HAS_PYDANTIC:
ResourceDictValidation = dataclass(ResourceDictValidation) # type: ignore


def _get_accepted_keys(class_type) -> list[str]:
if hasattr(class_type, "model_fields"):
return list(class_type.model_fields.keys())
elif hasattr(class_type, "__dataclass_fields__"):
return list(class_type.__dataclass_fields__.keys())
raise TypeError("Unsupported class type for validation")


def validate_resource_dict(resource_dict: dict) -> None:
_ = ResourceDictValidation(**resource_dict)
Comment on lines 47 to 48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the no-op validate_resource_dict still exists in task_scheduler/base.py
echo "=== task_scheduler/base.py validate_resource_dict ==="
rg -n 'def validate_resource_dict' --type=py -C 3

echo ""
echo "=== Imports of validate_resource_dict from task_scheduler ==="
rg -n 'from.*task_scheduler.*import.*validate_resource_dict' --type=py

echo ""
echo "=== All imports from task_scheduler.base ==="
rg -n 'from.*task_scheduler\.base.*import' --type=py

echo ""
echo "=== All imports from standalone.validate ==="
rg -n 'from.*standalone\.validate.*import' --type=py

echo ""
echo "=== Check task_scheduler/base.py content around line 16-17 ==="
sed -n '1,30p' src/executorlib/task_scheduler/base.py 2>/dev/null || echo "File not found at expected location"

Repository: pyiron/executorlib

Length of output: 4644


Remove the no-op validate_resource_dict from task_scheduler/base.py (lines 17-18).

The no-op implementation persists and creates a shadowing problem. Five modules—blockallocation.py, dependency.py, spawner_pysqa.py, onetoone.py, and task_scheduler.py—import the no-op from task_scheduler.base and will silently skip validation. Only the three executor modules (flux.py, single.py, slurm.py) use the proper implementation from standalone.validate. Either remove the no-op entirely or update all affected imports to use standalone.validate.validate_resource_dict.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/executorlib/standalone/validate.py` around lines 47 - 48, The project
defines a no-op validate_resource_dict in task_scheduler.base that shadows the
real validator in standalone.validate (which constructs
ResourceDictValidation(**resource_dict)), causing five modules
(blockallocation.py, dependency.py, spawner_pysqa.py, onetoone.py,
task_scheduler.py) to skip validation; remove the no-op from task_scheduler.base
and update those modules to import validate_resource_dict from
src.executorlib.standalone.validate (or directly call ResourceDictValidation in
their flow) so all callers use the real validation implementation (keep the
existing correct implementation in standalone.validate.validate_resource_dict
which calls ResourceDictValidation).



def validate_resource_dict_with_optional_keys(resource_dict: dict) -> None:
accepted_keys = ResourceDictValidation.model_fields.keys()
accepted_keys = _get_accepted_keys(class_type=ResourceDictValidation)
optional_lst = [key for key in resource_dict if key not in accepted_keys]
validate_dict = {
key: value for key, value in resource_dict.items() if key in accepted_keys
Expand Down
82 changes: 37 additions & 45 deletions tests/unit/standalone/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,63 +12,55 @@
skip_pydantic_test = True


class TestValidateImport(unittest.TestCase):
def test_single_node_executor(self):
class TestValidateFallback(unittest.TestCase):
def test_validate_resource_dict_fallback(self):
with patch.dict('sys.modules', {'pydantic': None}):
if 'executorlib.standalone.validate' in sys.modules:
del sys.modules['executorlib.standalone.validate']
if 'executorlib.executor.single' in sys.modules:
del sys.modules['executorlib.executor.single']

import executorlib.executor.single
importlib.reload(executorlib.executor.single)
from executorlib.standalone.validate import validate_resource_dict, ResourceDictValidation
from dataclasses import is_dataclass

from executorlib.executor.single import validate_resource_dict

source_file = inspect.getfile(validate_resource_dict)
if os.name == 'nt':
self.assertTrue(source_file.endswith('task_scheduler\\base.py'))
else:
self.assertTrue(source_file.endswith('task_scheduler/base.py'))
self.assertIsNone(validate_resource_dict({"any": "thing"}))
self.assertTrue(is_dataclass(ResourceDictValidation))

def test_flux_job_executor(self):
with patch.dict('sys.modules', {'pydantic': None}):
if 'executorlib.standalone.validate' in sys.modules:
del sys.modules['executorlib.standalone.validate']
if 'executorlib.executor.flux' in sys.modules:
del sys.modules['executorlib.executor.flux']
# Valid dict
self.assertIsNone(validate_resource_dict({"cores": 1}))

import executorlib.executor.flux
importlib.reload(executorlib.executor.flux)
# Invalid dict (extra key)
with self.assertRaises(TypeError):
validate_resource_dict({"invalid_key": 1})

from executorlib.executor.flux import validate_resource_dict

source_file = inspect.getfile(validate_resource_dict)
if os.name == 'nt':
self.assertTrue(source_file.endswith('task_scheduler\\base.py'))
else:
self.assertTrue(source_file.endswith('task_scheduler/base.py'))
self.assertIsNone(validate_resource_dict({"any": "thing"}))

def test_slurm_job_executor(self):
def test_validate_resource_dict_with_optional_keys_fallback(self):
with patch.dict('sys.modules', {'pydantic': None}):
if 'executorlib.standalone.validate' in sys.modules:
del sys.modules['executorlib.standalone.validate']
if 'executorlib.executor.slurm' in sys.modules:
del sys.modules['executorlib.executor.slurm']

import executorlib.executor.slurm
importlib.reload(executorlib.executor.slurm)
from executorlib.standalone.validate import validate_resource_dict_with_optional_keys

# Valid dict with optional keys
with self.assertWarns(UserWarning):
validate_resource_dict_with_optional_keys({"cores": 1, "optional_key": 2})

def test_get_accepted_keys(self):
from executorlib.standalone.validate import _get_accepted_keys, ResourceDictValidation

from executorlib.executor.slurm import validate_resource_dict

source_file = inspect.getfile(validate_resource_dict)
if os.name == 'nt':
self.assertTrue(source_file.endswith('task_scheduler\\base.py'))
else:
self.assertTrue(source_file.endswith('task_scheduler/base.py'))
self.assertIsNone(validate_resource_dict({"any": "thing"}))
accepted_keys = _get_accepted_keys(ResourceDictValidation)
expected_keys = [
"cores",
"threads_per_core",
"gpus_per_core",
"cwd",
"cache_key",
"num_nodes",
"exclusive",
"error_log_file",
"run_time_limit",
"priority",
"slurm_cmd_args"
]
self.assertEqual(set(accepted_keys), set(expected_keys))
with self.assertRaises(TypeError):
_get_accepted_keys(int)


@unittest.skipIf(skip_pydantic_test, "pydantic is not installed")
Expand All @@ -81,4 +73,4 @@ def dummy_function(i):

with SingleNodeExecutor() as exe:
with self.assertRaises(ValidationError):
exe.submit(dummy_function, 5, resource_dict={"any": "thing"})
exe.submit(dummy_function, 5, resource_dict={"any": "thing"})
Loading