-
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: Overhaul slurmutils API and add slurmdbd editor #4
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
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]>
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]>
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
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
1 task
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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
List[Tuple[str, Any]]
now the editor usesDict[str, Any]
.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 themodels
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.