https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422021-06-28T20:53:18ZArvadosArvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=939572021-06-28T20:53:18ZTom Cleggtom@curii.com
<ul></ul><p>Check other CLI tools for the same bug while we're at it.</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=939592021-06-29T14:12:52ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul><p>started at 17840-unparsed-args</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=943022021-07-06T21:11:57ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> deleted (<del><i>Arvados Future Sprints</i></del>)</li></ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=982902021-11-08T15:51:54ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> set to <i>2021-11-10 sprint</i></li><li><strong>Assigned To</strong> set to <i>Tom Clegg</i></li></ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=983132021-11-08T21:44:57ZTom Cleggtom@curii.com
<ul></ul><p>17840-unparsed-args @ <a class="changeset" title="17840: Check for unparsed command line arguments. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac">40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2779/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2779/">developer-run-tests: #2779 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2779" alt="" /></a></a></p>
<p>retrying workbench1 tests at <a class="external" href="https://ci.arvados.org/job/developer-run-tests-apps-workbench-integration/2952/"<a href="https://ci.arvados.org/job/developer-run-tests-apps-workbench-integration/2952/">developer-run-tests-apps-workbench-integration: #2952 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests-apps-workbench-integration&build=2952" alt="" /></a></a></p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=983722021-11-10T13:42:00ZWard Vandewegeward@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>17840-unparsed-args @ <a class="changeset" title="17840: Check for unparsed command line arguments. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac">40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2779/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2779/">developer-run-tests: #2779 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2779" alt="" /></a></a></p>
<p>retrying workbench1 tests at <a class="external" href="https://ci.arvados.org/job/developer-run-tests-apps-workbench-integration/2952/"<a href="https://ci.arvados.org/job/developer-run-tests-apps-workbench-integration/2952/">developer-run-tests-apps-workbench-integration: #2952 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests-apps-workbench-integration&build=2952" alt="" /></a></a></p>
</blockquote>
<p>OK, there seems to be a fair bit going on here. Looking at keep-balance, before the patch:<br /><pre>
./keep-balance -x
flag provided but not defined: -x
Usage of ./keep-balance:
-commit-confirmed-fields
update collection fields (replicas_confirmed, storage_classes_confirmed, etc.) (default true)
-commit-pulls
send pull requests (make more replicas of blocks that are underreplicated or are not in optimal rendezvous probe order)
-commit-trash
send trash requests (delete unreferenced old blocks, and excess replicas of overreplicated blocks)
-config file
Site configuration file (default may be overridden by setting an ARVADOS_CONFIG environment variable) (default "/etc/arvados/config.yml")
-dump
dump details for each block to stdout
-legacy-crunch-dispatch-slurm-config file
Legacy crunch-dispatch-slurm configuration file (default "/etc/arvados/crunch-dispatch-slurm/crunch-dispatch-slurm.yml")
-legacy-git-httpd-config file
Legacy arv-git-httpd configuration file (default "/etc/arvados/git-httpd/git-httpd.yml")
-legacy-keepbalance-config file
Legacy keep-balance configuration file (default "/etc/arvados/keep-balance/keep-balance.yml")
-legacy-keepproxy-config file
Legacy keepproxy configuration file (default "/etc/arvados/keepproxy/keepproxy.yml")
-legacy-keepstore-config file
Legacy keepstore configuration file (default "/etc/arvados/keepstore/keepstore.yml")
-legacy-keepweb-config file
Legacy keep-web configuration file (default "/etc/arvados/keep-web/keep-web.yml")
-legacy-ws-config file
Legacy arvados-ws configuration file (default "/etc/arvados/ws/ws.yml")
-once
balance once and then exit
-pprof [addr]:port
serve Go profile data at [addr]:port
-skip-legacy
Don't load legacy config files
-version
Write version information to stdout and exit 0
</pre></p>
<p>With this patch:</p>
<pre>
./keep-balance -x
flag provided but not defined: -x
Usage of ./keep-balance:
-commit-confirmed-fields
update collection fields (replicas_confirmed, storage_classes_confirmed, etc.) (default true)
-commit-pulls
send pull requests (make more replicas of blocks that are underreplicated or are not in optimal rendezvous probe order)
-commit-trash
send trash requests (delete unreferenced old blocks, and excess replicas of overreplicated blocks)
-config file
Site configuration file (default may be overridden by setting an ARVADOS_CONFIG environment variable) (default "/etc/arvados/config.yml")
-dump
dump details for each block to stdout
-legacy-crunch-dispatch-slurm-config file
Legacy crunch-dispatch-slurm configuration file (default "/etc/arvados/crunch-dispatch-slurm/crunch-dispatch-slurm.yml")
-legacy-git-httpd-config file
Legacy arv-git-httpd configuration file (default "/etc/arvados/git-httpd/git-httpd.yml")
-legacy-keepbalance-config file
Legacy keep-balance configuration file (default "/etc/arvados/keep-balance/keep-balance.yml")
-legacy-keepproxy-config file
Legacy keepproxy configuration file (default "/etc/arvados/keepproxy/keepproxy.yml")
-legacy-keepstore-config file
Legacy keepstore configuration file (default "/etc/arvados/keepstore/keepstore.yml")
-legacy-keepweb-config file
Legacy keep-web configuration file (default "/etc/arvados/keep-web/keep-web.yml")
-legacy-ws-config file
Legacy arvados-ws configuration file (default "/etc/arvados/ws/ws.yml")
-once
balance once and then exit
-pprof [addr]:port
serve Go profile data at [addr]:port
-skip-legacy
Don't load legacy config files
-version
Write version information to stdout and exit 0
ERRO[0000] error parsing command line flags: flag provided but not defined: -x
</pre>
<p>The error message is now added at the bottom which is nice. Any chance we can easily get rid of the one at the top?</p>
<p>And with an invalid/extra argument, we now get:</p>
<pre>
./keep-balance x
ERRO[0000] unrecognized command line arguments: [x]
</pre>
<p>For consistency's sake, should we also print the help text when this happens?</p>
<p>I'm not sure what to think about the formatting of the errors (the leading ERRO\[0000\] is a bit wonky). Is it easy to omit that? Maybe replace with a more human friendly 'Error: ' ? Maybe with a blank line between the help text and the error message, to make it stand out more visually?</p>
<p>Otherwise, nice cleanup!</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=984022021-11-10T16:08:00ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> changed from <i>2021-11-10 sprint</i> to <i>2021-11-24 sprint</i></li></ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=984702021-11-11T19:54:50ZTom Cleggtom@curii.com
<ul></ul>17840-unparsed-args @ <a class="changeset" title="17840: Fix crash on config load error. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/47c3faf1e26be21190eeee7f266d44eb33a0aeb6">47c3faf1e26be21190eeee7f266d44eb33a0aeb6</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2791/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2791/">developer-run-tests: #2791 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2791" alt="" /></a></a>
<ul>
<li>de-duplicate much of the argument-parsing code and unify behavior across various programs</li>
<li>"-h" and "-help" print the usage notes, and exit 0</li>
<li>bad flags and missing/extra positional arguments cause an error message on stderr, and exit 2, but do not obscure the error message by printing the usage notes before/after</li>
</ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=985132021-11-15T16:30:13ZWard Vandewegeward@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
17840-unparsed-args @ <a class="changeset" title="17840: Fix crash on config load error. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/47c3faf1e26be21190eeee7f266d44eb33a0aeb6">47c3faf1e26be21190eeee7f266d44eb33a0aeb6</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2791/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2791/">developer-run-tests: #2791 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2791" alt="" /></a></a>
<ul>
<li>de-duplicate much of the argument-parsing code and unify behavior across various programs</li>
<li>"-h" and "-help" print the usage notes, and exit 0</li>
<li>bad flags and missing/extra positional arguments cause an error message on stderr, and exit 2, but do not obscure the error message by printing the usage notes before/after</li>
</ul>
</blockquote>
<p>This looks great! Any reason `cmd/arvados-package/cmd.go` was not given the same treatment? LGTM otherwise.</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=986702021-11-16T21:01:39ZTom Cleggtom@curii.com
<ul></ul>17840-unparsed-args @ <a class="changeset" title="17840: Merge branch 'main' Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/fb96637bf76fe8779e7a7e58f052b8f55ed76f4f">fb96637bf76fe8779e7a7e58f052b8f55ed76f4f</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2799/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2799/">developer-run-tests: #2799 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2799" alt="" /></a></a>
<ul>
<li>same treatment to arvados-package</li>
<li>merged main</li>
</ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=987452021-11-17T19:39:51ZWard Vandewegeward@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
17840-unparsed-args @ <a class="changeset" title="17840: Merge branch 'main' Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/fb96637bf76fe8779e7a7e58f052b8f55ed76f4f">fb96637bf76fe8779e7a7e58f052b8f55ed76f4f</a> -- <a class="external" href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2799/"<a href="https://ci.arvados.org/view/Developer/job/developer-run-tests/2799/">developer-run-tests: #2799 <img src="https://ci.arvados.org/buildStatus/icon?job=developer-run-tests&build=2799" alt="" /></a></a>
<ul>
<li>same treatment to arvados-package</li>
<li>merged main</li>
</ul>
</blockquote>
<p>LGTM, thanks!</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=987722021-11-18T16:52:34ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul><p>Applied in changeset arvados-private:commit:arvados|0f42105b1b59d1b5da764f34e6eb6a1137d7e1cb.</p> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=988682021-11-22T21:26:53ZWard Vandewegeward@curii.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-3 priority-4 priority-default closed" href="/issues/18465">Bug #18465</a>: arvados-dispatch-cloud starts compute nodes for "on hold" containers</i> added</li></ul> Arvados - Bug #17840: keep-balance should error out on extra unparsed command line flagshttps://dev.arvados.org/issues/17840?journal_id=1021112022-03-24T19:28:16ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Release</strong> set to <i>46</i></li></ul>