-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add gres.conf
editor to slurmutils
#37
feat: add gres.conf
editor to slurmutils
#37
Conversation
Changes: * Adds `GRESConfig` for modelling the gres.conf configuration file. * Adds `GRESName` for modelling lines in the gres.conf configuration file. * Adds `GRESNode` for modelling nodes with configured generic resources in gres.conf. Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Release adds new `gres.conf` editor. Also tests out explicitly defining descriptors for static typing tools such as `pyright` or `mypy` Signed-off-by: Jason C. Nucciarone <[email protected]>
3cb0a89
to
0f20a0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I checked out the branch and created a few test gres.confs without issue. I see GresTypes and Gres have been in slurm.conf for a while so no changes needed there.
For the getters/setters/deleters, my preference would be for dynamic over IDE-generated boilerplate, just to make the code more concise, but I can see your point re: pyright
.
That's a fair point from the library development perspective; you can only do Dynamic attributes save you time when developing Explicit attributes take more time to create (and possibly maintain), but it allows us to be serious about static type checking. It's also arguably better to be explicit rather than implicit as there's less ambiguity about what data models can and can't do. |
This PR adds the
gresconfig
module for editinggres.conf
configuration files on machines. This module is necessary for configuring GPUs in Charmed HPC.Key things to note
This editor uses a new approach for defining descriptors for mutating configuration options through data models. Rather than dynamically create descriptors using the
generate_descriptors(...)
function, they're instead manually defined. Static type checkers such aspyright
do not effectively support dynamic attributes without ugly hacks, so here I just created all the descriptors using live templates in PyCharm.Ctrl+J
propsd
will quickly enter the boilerplate for the descriptors.This should make it easier for using a static type checker on code that uses
slurmutils
without needing to deactivate checks. If we're happy with how it looks here, I will look into applying it to all the other data models withinslurmutils
.Why are all the deleters encapsulated with
try: ... except AttributeError: pass
? I realized that if you calldel
on a data model attribute before setting a key within the internal dictionary associated with the created data model, Python will raise anAttributeError
since the key does exist inside the internal dictionary. Since the desired behavior is that the configuration option is unset and not present in the generated configuration file, theAttributeError
is silently handled rather than emit a warning message.