search

flux-framework / flux-core

core services for the Flux resource management framework
GNU Lesser General Public License v3.0
168 stars 50 forks source link

add `-S, --setattr` and `-c, --config-path` options directly to `flux start` #6452

Closed grondo closed 3 days ago

grondo commented 3 days ago

I kept getting annoyed that the common broker options for configuration path and setting attributes have to be passed through flux start's -o, --broker-opts option. flux start is used solely to start brokers, so why can't it just support these common options directly? So, submitted for your consideration is a PR that adds -S, --setattr and -c, --config-path options directly to flux start. If used, these options are just passed along to the broker(s) being started.

WIP because there's no tests or documentation updates in case there's some reason I'm not thinking of that this was a bad idea.

garlick commented 3 days ago

Sounds like a great idea to me.

grondo commented 3 days ago

Converted most of the testsuite to use flux start -S/-c directly instead of -o,-S or -o,-c etc. Removed WIP. (We'll see if this passes CI)

grondo commented 3 days ago

Thanks, coverage is low just because there's a lot of error conditions we can't hit in test. I'll set MWP.

codecov[bot] commented 3 days ago

Codecov Report

Attention: Patch coverage is 78.04878% with 9 lines in your changes missing coverage. Please review.

Project coverage is 83.63%. Comparing base (01d3650) to head (89c2016). Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
src/cmd/flux-start.c 78.04% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6452 +/- ## ========================================== + Coverage 83.61% 83.63% +0.01% ========================================== Files 523 523 Lines 87533 87538 +5 ========================================== + Hits 73192 73210 +18 + Misses 14341 14328 -13 ``` | [Files with missing lines](https://app.codecov.io/gh/flux-framework/flux-core/pull/6452?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [src/cmd/flux-start.c](https://app.codecov.io/gh/flux-framework/flux-core/pull/6452?src=pr&el=tree&filepath=src%2Fcmd%2Fflux-start.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL2NtZC9mbHV4LXN0YXJ0LmM=) | `83.94% <78.04%> (-0.38%)` | :arrow_down: | ... and [15 files with indirect coverage changes](https://app.codecov.io/gh/flux-framework/flux-core/pull/6452/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework)

🚨 Try these New Features: