-
Notifications
You must be signed in to change notification settings - Fork 109
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
Remove deprecated queue options #9501
Remove deprecated queue options #9501
Conversation
Will squash commits^ |
22a898f
to
1db89d8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9501 +/- ##
==========================================
- Coverage 91.89% 91.83% -0.06%
==========================================
Files 433 433
Lines 26813 26768 -45
==========================================
- Hits 24639 24582 -57
- Misses 2174 2186 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
driver = OpenPBSDriver(memory_per_job="10gb") | ||
await driver.submit(0, "sleep") | ||
assert " -l mem=10gb" in Path("captured_qsub_args").read_text(encoding="utf-8") | ||
async def test_realization_memory(): |
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.
I think there are some realization_memory
tests already
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.
Will delete
@pytest.mark.usefixtures("capturing_qsub") | ||
async def test_full_resource_string(memory_per_job, num_cpu, cluster_label): | ||
async def test_full_resource_string(realization_memory, num_cpu, cluster_label): |
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.
Is the name still valid though? Since this one tests only realization memory in the resource string
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.
We also test for the select
and cspus
portion of the resource_string. The github preview of the changes is just acting up...
You need to rebase here. |
@@ -332,23 +295,6 @@ The following is a list of all queue-specific configuration options: | |||
|
|||
QUEUE_OPTION TORQUE SUBMIT_SLEEP 0.5 |
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.
How does this differ from SUBMIT_SLEEP
https://fmu-docs.equinor.com/docs/ert/reference/configuration/keywords.html#submit-sleep?
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.
After clarification, this will stay.
Need rebasing but otherwise very good job @jonathan-eq ! |
12b73f2
to
371c568
Compare
CodSpeed Performance ReportMerging #9501 will not alter performanceComparing Summary
|
039337f
to
cf31328
Compare
13a3e24
to
05e4043
Compare
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.
Very nice job @jonathan-eq ! Just fix the commit message and it is good to go! 🚀
This commit removes the deprecated torque/openpbs queue options: * QUEUE_QUERY_TIMEOUT * NUM_NODES * NUM_CPUS_PER_NODE * QSTAT_OPTIONS * MEMORY_PER_JOB
05e4043
to
177a3f3
Compare
Issue
Resolves #9500
Resolves #9177
Approach
✂️
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"'
)When applicable