-
Notifications
You must be signed in to change notification settings - Fork 49
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
flux-resource: add -q, --queue=QUEUE
option
#5935
Conversation
@@ -0,0 +1 @@ | |||
101 Nodes, 404 Cores, 0 GPUs |
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.
commit message "input input"
t/t2350-resource-list.t
Outdated
test_expect_success 'flux-resource list supports -q, --queue' ' | ||
flux resource list --queue=debug \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-debug.output && | ||
test_debug "cat fluke-debug.output" && | ||
test_must_fail grep batch fluke-debug.output && | ||
flux resource list --queue=batch \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-batch.output && | ||
test_debug "cat fluke-batch.output" && | ||
test_must_fail grep debug fluke-batch.output && | ||
flux resource list --queue=debug,batch \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-both.output && | ||
grep debug fluke-both.output && | ||
grep batch fluke-both.output | ||
' | ||
test_expect_success 'flux-resource list supports -q, --queue' ' | ||
flux resource list --queue=debug \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-debug.output && | ||
test_debug "cat fluke-debug.output" && | ||
test_must_fail grep batch fluke-debug.output && | ||
flux resource list --queue=batch \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-batch.output && | ||
test_debug "cat fluke-batch.output" && | ||
test_must_fail grep debug fluke-batch.output && | ||
flux resource list --queue=debug,batch \ | ||
--from-stdin --config-file=$FLUKE_CONFIG \ | ||
< $FLUKE_INPUT > fluke-both.output && | ||
grep debug fluke-both.output && | ||
grep batch fluke-both.output | ||
' |
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.
duplicated test? or am i not seeing the difference?
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.
oh, cut and paste and intended to test flux resource R
? i think a flux resource R
test is missing
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.
Oh yeah, good catch. I think it is just a duplicate because flux resource R
uses the same code as flux resource list
, but might as well add similar tests for flux resource R
here just using grep
.
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.
Fixed that and force-pushed
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.
LGTM!
Problem: flux-resource(1) subcommands don't support filtering output by queue. Add a -q, --queue=QUEUE option to most subcommands to allow filtering resources to one or more queues.
Problem: When using --from-stdin in tests there is no way to provide a corresponding Flux config for determining queues. Add a hidden --config-file=PATH option which can be used to provide a config object for testing.
Problem: There are no tests of flux-resource(1) -q, --queue option handling. Add some sample resource.sched-status and resource.status payloads derived from the fluke test system to t/flux-resource/{status,list}. Also add a flux.config input file capturing the queues configuration. Use these new input files to test `-q, --queues` handling for the flux-resource(1) subcommands: list, info, R, and status.
Problem: The -q, --queue=QUEUE option is not documented in flux-resource(1). Document the option.
Problem: The bash completions file distributed with flux does not complete the -q, --queue option in flux-resource(1) commands. Add this option to the completions.
Thanks! Setting MWP. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5935 +/- ##
==========================================
+ Coverage 83.34% 83.36% +0.01%
==========================================
Files 514 514
Lines 83053 83104 +51
==========================================
+ Hits 69218 69276 +58
+ Misses 13835 13828 -7
|
This PR adds a
-q, --queue=QUEUE
option toflux resource
subcommandslist
,status
,info
, andR
.