Skip to content
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: Overhaul slurmutils API and add slurmdbd editor #4

Merged
merged 19 commits into from
Jan 31, 2024

Conversation

NucciTheBoss
Copy link
Member

This pull request completely overhauls the slurmutils API to be much better than both the original slurmdbd.conf and slurm.conf editors that I wrote. These changes also enable the editors to share common resources such as parsing/marshalling logic and descriptor generation.

  • Reasons why the new slurmdbd.conf editor is better than the old slurmdbd.conf editor:
    • Configuration knobs have much better representation now. Rather than List[Tuple[str, Any]] now the editor uses Dict[str, Any].
    • Less cut and pasting of common operations. Descriptors for configuration knob attributes are now dynamically generated.
    • Better parsing mechanism. Instead of lexically analyizing tokens using deque's, now use built-in string splitting.
  • Reasons why the new slurm.conf editor is better than the old slurm.conf editor:
    • Better data models for types such as Node, NodeSet, DownNodes, etc. No longer expecting authors to know the magical values to put into dictionaries.
    • Better mechanism for mutating Nodes, NodeSets, FrontendNodes, etc. Special containers are now used for returning and accessing these objects.
    • No longer using a clunky context manager class for accessing the slurm.conf file. The edit method is now provided instead. This class made it too difficult to dump and load slurm.conf files from strings.

BREAKING CHANGES: Changes the API of the slurmutils package. An editors module is now used to provide the configuration file editros, and the models module is used for common data objects in Slurm. A context manager class is no longer provided for accessing the slurm.conf file since it's API was clunky.

BREAKING CHANGE: Drops the old API for parsing and marshalling the slurm.conf file. This will allow common parsing logic to be shared among further editors, and will enable better code suggestions to programmers working with Slurm configuration files.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Ignore lint error where magic methods do not have a docstring. Magic method behavior is already explained by Python documentation, and the defined methods behave the same as the  would if they were not overridden.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Using poetry for packaging and dependency management since the tool integrates nicely with pyproject.toml and does not require carrying around a requirements.txt file.

Signed-off-by: Jason C. Nucciarone <[email protected]>
BREAKING CHANGE: Switches license away from Apache-2.0. This change is in line with Canonical's policy on how libraries should be licensed.

Signed-off-by: Jason C. Nucciarone <[email protected]>
BREAKING CHANGE: No longer use the token lib that was used to classify configuration knobs in slurm.conf. Data models are no longer dynamically generated at runtime.

Signed-off-by: Jason C. Nucciarone <[email protected]>
BREAKING CHANGE: Marshaller is not currently ready for full-scale deployment. Also, a context management class is no longer used for SlurmConf because it was annoying to manage all the internal class attributes but also create secondary constructors.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
BREAKING CHANGE: Overrides default __iter__ behavior of UserList.

Signed-off-by: Jason C. Nucciarone <[email protected]>
The conditional for determining whether to create an empty Config object or to load in a pre-existing configuration was busted. If the pre-existing config existed, the context manager would return an empty Config object which is not the desired behavior. By adding `not` in front of the path check, the conditional now evaluates properly. Loads in the config if the config file exists; returns an empty Config object if it doesn't.

Also fixed a grammar error in the slurmconfig `_marshaller` function docstring.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Changes:
- Changed callback for communication_parameters to SlurmDict. Originally used CommaSeparator, but remembered that there are some possible key=value options as well.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
BREAKING CHANGE: Since poetry is now the primary packaging and dependency manager, no longer using setuptools when installing from source. Folks installing from source will now need to install poetry if going to install slurmutils from source. Pulling packages from PyPI will remain the same however.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Removes the dependency on the plain-text .pypirc file that can introduce potential security vulnerabilities.

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss NucciTheBoss added Priority: Critical This should be fixed RIGHT NOW. Should be used only for issues that affect everyone. Type: Enhancement Proposes a new feature to be added to the project. Type: Refactor Proposes changes to the code base but does not fundamentally change the API. labels Jan 31, 2024
@NucciTheBoss NucciTheBoss self-assigned this Jan 31, 2024
@NucciTheBoss NucciTheBoss merged commit 8e1ac8b into main Jan 31, 2024
4 checks passed
@NucciTheBoss NucciTheBoss deleted the add-slurmdbd branch February 5, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical This should be fixed RIGHT NOW. Should be used only for issues that affect everyone. Type: Enhancement Proposes a new feature to be added to the project. Type: Refactor Proposes changes to the code base but does not fundamentally change the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant