You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The SlurmConfig.from_dict method doesn't verify that the input fields for dict-like and array-like configs are of the correct shape; Nodes, DownNodes, Partitions, etc.
To Reproduce
config = SlurmConfig.from_dict({
"Nodes": [
{
"NodeAddr": "10.152.28.98",
"CPUs": "9",
"RealMemory": "9000",
"TmpDisk": "90000",
},
],
"DownNodes": {
"DownNodes": ["juju-c9fc6f-6", "juju-c9fc6f-7"],
"State": "DOWN",
"Reason": "New nodes",
},
"Partitions": [
{
"MaxTime": "10",
"MaxNodes": "5",
"State": "UP",
},
],
})
print(config.nodes) // Prints the malformed "Nodes" array
print(config.down_nodes) // Prints the malformed "DownNodes" object
print(config.partitions) // Prints the malformed "Partitions" array
Yeah, this is kinda expected. The from_dict(...) constructor is really no thrills for SlurmConfig. It just checks against the SlurmConfigOptionSet to ensure that there are no unexpected configuration keys passed to __init__.
This should be a milestone for a future slurmutils. Ideally we can add this form checking back in when we refine the story around the NodeMap, PartitionMap, etc data structures that can help verify is the data is good or not.
NucciTheBoss
changed the title
SlurmConfig.from_dict not verifying correct shape of dict-like and array-like configs
[Bug]: SlurmConfig.from_dict not verifying correct shape of dict-like and array-like configs
Sep 20, 2024
I believe adding a temporary fix to validate the shape of dict-like and array-like fields (e.g., Nodes, DownNodes, Partitions) could significantly improve the from_dict method's robustness. While I understand that the current implementation primarily checks for unexpected keys, adding basic structural validation could prevent malformed data from being passed through and causing subtle bugs.
A simple check for fields like Nodes (ensuring it's a list of dicts, for example) would help catch configuration errors early, without requiring a major overhaul. This would serve as an interim solution until the more refined NodeMap and PartitionMap data structures are implemented.
I’d love to contribute to implementing this temporary fix if the team agrees it’s a helpful step forward.
Sure, if you'd like to take a crack at a temporary fix, we can help by reviewing your PR 😄
Ideally in the future I'd like to get it where there's proper linting of the entire Slurm configuration data via callbacks, but for now we can include checks to ensure that the data structs are (a) the proper type and (b) "well-formed" to reduce the chances of semantic errors occurring.
NucciTheBoss
changed the title
[Bug]: SlurmConfig.from_dict not verifying correct shape of dict-like and array-like configsSlurmConfig.from_dict not verifying correct shape of dict-like and array-like configs
Jan 16, 2025
Bug Description
The
SlurmConfig.from_dict
method doesn't verify that the input fields for dict-like and array-like configs are of the correct shape;Nodes
,DownNodes
,Partitions
, etc.To Reproduce
Environment
Ran locally on Ubuntu Noble (24.04)
Additional context
This operation should probably throw when the input configs are not of the correct shape.
The text was updated successfully, but these errors were encountered: