-
Notifications
You must be signed in to change notification settings - Fork 8
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
ASP-1915 Move SBATCH params to Job Script submission #194
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
to job submissions
to the `job_submissions` POST endpoint
assert if mocked method is called
matheushent
reviewed
Dec 5, 2022
they now also check is the data at the database is as expected
ASP-2256 Migrate the SBATCH param mapper to jobbergate-api
ASP-2300 Add "execution_parameters" at the CLI
This is the result of integration tests with slurm restd
fschuch
commented
Dec 9, 2022
matheushent
reviewed
Dec 9, 2022
matheushent
reviewed
Dec 9, 2022
dusktreader
suggested changes
Dec 12, 2022
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.
Sorry for all the feedback here, but I think there are a few fundamental things to address.
jobbergate-api/jobbergate_api/apps/job_submissions/properties_parser.py
Outdated
Show resolved
Hide resolved
jobbergate-api/jobbergate_api/apps/job_submissions/properties_parser.py
Outdated
Show resolved
Hide resolved
jobbergate-api/jobbergate_api/apps/job_submissions/properties_parser.py
Outdated
Show resolved
Hide resolved
jobbergate-api/jobbergate_api/apps/job_submissions/properties_parser.py
Outdated
Show resolved
Hide resolved
from sbatch_to_slurm to sbatch_to_slurm_mapping
dusktreader
approved these changes
Dec 19, 2022
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.
What
Currently in Jobbergate, the extra SBATCH params used for Job Script execution must be supplied when an Application is being rendered into a Job Script. The SBATCH params are injected into the Job Script code itself. Then, when the jobs are submitted to the cluster, those params are extracted from the script and re-written as API request parameters to the Slurm REST API.
Since the SBATCH params actually control job execution, they should instead be passed as part of the Job Submission process where a Job Script is submitted for execution. To this end, they should be changed to be a property of a Job Submission.
Why
Task
: https://jira.scania.com/browse/ASP-1915Peer Review
Please follow the upstream omnivector documentation concerning
peer-review guidelines.