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

Introduce jobscript exit code check #3

Merged

Conversation

leahaeusel
Copy link
Contributor

@leahaeusel leahaeusel commented Dec 19, 2024

Description and Context

This is an alternative implementation of merge request !786 on GitLab that improves the error handling in our jobscript driver. This new implementation introduces a flag to the JobscriptDriver that determines whether to raise an error if the jobscript returns a non-zero exit code.

This new implementation does not introduce a second constructor and, therefore, does not break the input-file-based workflow.

In addition, the new error handling allows for catching OSErrors which used to occur when the jobscript_template string was too long.

Related Issues and Merge Requests

How Has This Been Tested?

Checklist

  • My commit messages mention the appropriate issue numbers (advised but optional).
  • I mention the appropriate issue numbers in this MR.
  • I updated documentation where necessary.
  • I updated the README where necessary
  • I have added as (few,) small and fast tests as possible to cover my changes.

Additional Information

Interested Parties

Possible reviewers:

Other interested parties:

Copy link
Contributor

@gilrrei gilrrei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @leahaeusel, for the addition; I think this is an important one! Also nice that you added loads of tests.

queens/drivers/jobscript_driver.py Outdated Show resolved Hide resolved
tests/unit_tests/drivers/test_jobscript_driver.py Outdated Show resolved Hide resolved
tests/unit_tests/drivers/test_jobscript_driver.py Outdated Show resolved Hide resolved
tests/unit_tests/drivers/test_jobscript_driver.py Outdated Show resolved Hide resolved
@gilrrei
Copy link
Contributor

gilrrei commented Dec 21, 2024

Closes #69

@leahaeusel leahaeusel force-pushed the introduce-jobscript-exit-code-check branch from f4b10c9 to 3a0a7a0 Compare December 23, 2024 10:58
@leahaeusel leahaeusel merged commit 9d1625f into queens-py:main Dec 30, 2024
1 check passed
@leahaeusel leahaeusel deleted the introduce-jobscript-exit-code-check branch January 2, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long Jobscript Template String Not Possible
2 participants