[feat] Remove sched_* internal variables
#1372
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1372 +/- ##
==========================================
- Coverage 91.66% 91.60% -0.07%
==========================================
Files 83 83
Lines 12673 12610 -63
==========================================
- Hits 11617 11551 -66
- Misses 1056 1059 +3
Continue to review full report at Codecov.
|
| f"use `-J account={options.account}'") | ||
|
|
||
| if options.partition: | ||
| options.job_options.append(f'partition={options.partition}') |
There was a problem hiding this comment.
@vkarak do we want to keep the option working only for slurm? Is there a way to pass this argument for both pbs and slurm through job_options?
There was a problem hiding this comment.
@ekout, you are right. We cannot drop the support for other schedulers now, so we should keep the sched_partition until we completely remove the scheduler-specific options.
vkarak
left a comment
vkarak
left a comment
There was a problem hiding this comment.
sched_partition should remain along with all the unit tests that are using it. Check also my comments about the flexible allocation tests.
| f"use `-J account={options.account}'") | ||
|
|
||
| if options.partition: | ||
| options.job_options.append(f'partition={options.partition}') |
There was a problem hiding this comment.
@ekout, you are right. We cannot drop the support for other schedulers now, so we should keep the sched_partition until we completely remove the scheduler-specific options.
| def test_flex_alloc_valid_reservation_cmd(make_flexible_job): | ||
| job = make_flexible_job('all', | ||
| sched_access=['--constraint=f2'], | ||
| sched_reservation='dummy') |
There was a problem hiding this comment.
This test should remain except that the reservation should be passed through sched_options.
| def test_flex_alloc_exclude_nodes_cmd(make_flexible_job): | ||
| job = make_flexible_job('all', | ||
| sched_access=['--constraint=f1'], | ||
| sched_exclude_nodelist='nid00001') |
|
After discussing with @vkarak we are postponing this Reframe 3.2 when we will remove |
|
Closing in favor of #1483 |
4 participants
Closes #1368