LIU-420: Protoype config.INI and Slurm script templates #297
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
(Incomplete) - Basic use of SLURM 'template' script - Skeleton use of .ini file for environment variables - Added unittests to help prototyping
- Tests pass for config and slurm template - Tested on Setonix
Reviewer's Guide by SourceryThis PR introduces a flexible configuration system for SLURM job submissions by implementing support for INI configuration files and SLURM script templates. The changes maintain backward compatibility while allowing users to customize deployment settings and SLURM directives through external files rather than hardcoded configurations or CLI arguments. Sequence diagram for job submission with new configurationsequenceDiagram
actor User
participant SlurmClient
participant ConfigParser
participant RemoteServer
User->>SlurmClient: Initialize with config_file and slurm_template
SlurmClient->>ConfigParser: process_config(config_file)
ConfigParser-->>SlurmClient: Return configuration
SlurmClient->>RemoteServer: Create session directory
SlurmClient->>SlurmClient: apply_slurm_template()
SlurmClient->>RemoteServer: Submit job
RemoteServer-->>SlurmClient: Job ID
SlurmClient-->>User: Return Job ID
Class diagram for SlurmClient modificationsclassDiagram
class SlurmClient {
-String host
-String _acc
-String dlg_root
-String modules
-String venv
-String exec_prefix
-String username
-String _slurm_template
-int _num_nodes
-int _job_dur
-String _suffix
-boolean _remote
+create_session_suffix(suffix)
+get_session_dirname()
+apply_slurm_template(template_str, session_id, dlg_root)
+create_job_desc(physical_graph_file)
+mk_session_dir(dlg_root)
+submit_job()
}
note for SlurmClient "New attributes: config, slurm_template, suffix. New methods: create_session_suffix, apply_slurm_template."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @myxie - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add documentation with examples of the config.INI format and SLURM template structure to help users adopt these new features.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1 task
- Previous approach ended up using local graph as input to the script; this isn't how client.submit_job() works; it uses the remote path the phyiscal graph is expected
|
@awicenec and @alxndrwllmsn, it would be great to get your eyes on this if you have the time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
JIRA Ticket
LIU-420
Type
Problem/Issue
The current configuration options for the
create_dlg_job.pyscript are not flexible, as it is necessary to create a brand new class for any new facility or non-standard approach for using the script (e.g. a different/venv/directory or DLG installation`. This is true for the SLURM script too; currently any additional SBATCH directives must be added as a CLI option to the script, which will get unwieldy quickly.It would be good to remove the existing approach, and instead rely on configuration files that the user can edit.
Solution
This PR delivers an MVP of behaviour supporting a configuration file for the facility environment variables, and a SLURM script template that allows for user-defined options and directives.
The following use of the new additions successfully deploys to setonix using a setonix.ini file and a setonix.slurm template:
Things to note are:
Ideally, this PR delivers the minimal-viable functionality that will allow us to start prioritising the features we want to support and trigger conversations moving forward about the final implementation we are working towards.
Checklist
Summary by Sourcery
Prototype the use of configuration INI files and SLURM script templates in the SlurmClient to enhance job submission flexibility and configuration management.
New Features:
Enhancements:
Tests:
Summary by Sourcery
Prototype the use of configuration INI files and SLURM script templates in the SlurmClient to enhance job submission flexibility and configuration management.
New Features:
Enhancements:
Documentation:
Tests: