Re: Review Request 26688: Fix errors in help rendering:
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- (Updated Oct. 15, 2014, 12:41 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/
---
(Updated Oct. 15, 2014, 12:46 p.m.)
Review request for Aurora, Joshua Cohen and Zameer Manji.
Bugs: aurora-831
https://issues.apache.org/jira/browse/aurora-831
Repository: aurora
Description (updated)
---
- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.
Diffs
-
src/main/python/apache/aurora/client/cli/__init__.py
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/main/python/apache/aurora/client/cli/options.py
dc76c25b90acb9610e40b939e65c3cabf032649f
src/main/python/apache/aurora/client/cli/standalone_client.py
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1
src/test/python/apache/aurora/client/cli/test_help.py
f73c8a3778b7d118ea2865f213b442a607fb4a7d
Diff: https://reviews.apache.org/r/26688/diff/
Testing (updated)
---
New help output:
{noformat}
[sun-wukong incubator-aurora (deprecate)]$ ./dist/aurora2.pex help cron
deschedule
Usage for verb cron deschedule:
deschedule [--verbose-logging] [--logging-level=numeric_level]
[--error-log-dir=error-log-dir] [--bind=var=value] CLUSTER/ROLE/ENV/NAME
Options:
--bind=var=value
Bind a pystachio variable name to a value. Multiple flags may be used
to specify multiple values.
CLUSTER/ROLE/ENV/NAME
Fully specified job key, in CLUSTER/ROLE/ENV/NAME format
--verbose-logging
Show verbose logging, including all logs up to level INFO (equivalent
to --logging-level=20)
--logging-level=numeric_level
Set logging to a specific numeric level.
--error-log-dir=error-log-dir
Directory location where error files containing stack traces should be
written. If the directory doesn't exist, it will be created
Remove the cron schedule for a job.
{noformat}
Thanks,
Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
On Oct. 14, 2014, 12:46 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_help.py, line 75 https://reviews.apache.org/r/26688/diff/1/?file=720844#file720844line75 Are option names guaranteed to be unique? If not this test could potentially pass if any help output contains a plugin option name, not necessarily the help output for the command to which the plugin was registered. It's also possible for a plugin option name to appear in the help for another option, and not on its own, which would cause this test to succeed even if the plugin options themselves are not properly displayed? I guess what I'm getting at is would it be better to test for more than just the appearance of a string at any point in the output? (This may be based on incomplete understanding of how the client registers commands/options). I'm trying to keep the test from being overly sensitive to changes. The problem with a lot of output testing is that it's incredibly brittle - even tiny changes to the output require the test to get rewritten. So when possible, I'd prefer to have tightly focused tests, which look for specific problems. We know that we had a rendering bug that was causing plugin options to get omitted from usage strings - so this test tries to specifically check that that's not happening anymore. If it fails, we know exactly what the problem is. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/#review56541 --- On Oct. 15, 2014, 12:41 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- (Updated Oct. 15, 2014, 12:41 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/#review56733
---
src/main/python/apache/aurora/client/cli/standalone_client.py
https://reviews.apache.org/r/26688/#comment97118
Is this a python standard for log levels? Could the help show all possible
values?
- Zameer Manji
On Oct. 15, 2014, 9:46 a.m., Mark Chu-Carroll wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/
---
(Updated Oct. 15, 2014, 9:46 a.m.)
Review request for Aurora, Joshua Cohen and Zameer Manji.
Bugs: aurora-831
https://issues.apache.org/jira/browse/aurora-831
Repository: aurora
Description
---
- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.
Diffs
-
src/main/python/apache/aurora/client/cli/__init__.py
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/main/python/apache/aurora/client/cli/options.py
dc76c25b90acb9610e40b939e65c3cabf032649f
src/main/python/apache/aurora/client/cli/standalone_client.py
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1
src/test/python/apache/aurora/client/cli/test_help.py
f73c8a3778b7d118ea2865f213b442a607fb4a7d
Diff: https://reviews.apache.org/r/26688/diff/
Testing
---
New help output:
{noformat}
[sun-wukong incubator-aurora (deprecate)]$ ./dist/aurora2.pex help cron
deschedule
Usage for verb cron deschedule:
deschedule [--verbose-logging] [--logging-level=numeric_level]
[--error-log-dir=error-log-dir] [--bind=var=value] CLUSTER/ROLE/ENV/NAME
Options:
--bind=var=value
Bind a pystachio variable name to a value. Multiple flags may be used
to specify multiple values.
CLUSTER/ROLE/ENV/NAME
Fully specified job key, in CLUSTER/ROLE/ENV/NAME format
--verbose-logging
Show verbose logging, including all logs up to level INFO (equivalent
to --logging-level=20)
--logging-level=numeric_level
Set logging to a specific numeric level.
--error-log-dir=error-log-dir
Directory location where error files containing stack traces should be
written. If the directory doesn't exist, it will be created
Remove the cron schedule for a job.
{noformat}
Thanks,
Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/#review56737
---
src/main/python/apache/aurora/client/cli/standalone_client.py
https://reviews.apache.org/r/26688/#comment97119
It's the standard python loglevels. The number is basically any positive
integer from 0 to 50.
The underlying assumption here is that this is an option that's only going
to be used by someone who knows what they're doing, most likely someone
debugging the client. Users will generally use --verbose-logging instead of
setting a specific value.
Since this is an option that's going to show up in *every* command's help,
I'd really like to keep it's helpline as concise as possible.
How about adding numeric level as defined by python standard logging?
- Mark Chu-Carroll
On Oct. 15, 2014, 12:46 p.m., Mark Chu-Carroll wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/
---
(Updated Oct. 15, 2014, 12:46 p.m.)
Review request for Aurora, Joshua Cohen and Zameer Manji.
Bugs: aurora-831
https://issues.apache.org/jira/browse/aurora-831
Repository: aurora
Description
---
- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.
Diffs
-
src/main/python/apache/aurora/client/cli/__init__.py
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/main/python/apache/aurora/client/cli/options.py
dc76c25b90acb9610e40b939e65c3cabf032649f
src/main/python/apache/aurora/client/cli/standalone_client.py
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1
src/test/python/apache/aurora/client/cli/test_help.py
f73c8a3778b7d118ea2865f213b442a607fb4a7d
Diff: https://reviews.apache.org/r/26688/diff/
Testing
---
New help output:
{noformat}
[sun-wukong incubator-aurora (deprecate)]$ ./dist/aurora2.pex help cron
deschedule
Usage for verb cron deschedule:
deschedule [--verbose-logging] [--logging-level=numeric_level]
[--error-log-dir=error-log-dir] [--bind=var=value] CLUSTER/ROLE/ENV/NAME
Options:
--bind=var=value
Bind a pystachio variable name to a value. Multiple flags may be used
to specify multiple values.
CLUSTER/ROLE/ENV/NAME
Fully specified job key, in CLUSTER/ROLE/ENV/NAME format
--verbose-logging
Show verbose logging, including all logs up to level INFO (equivalent
to --logging-level=20)
--logging-level=numeric_level
Set logging to a specific numeric level.
--error-log-dir=error-log-dir
Directory location where error files containing stack traces should be
written. If the directory doesn't exist, it will be created
Remove the cron schedule for a job.
{noformat}
Thanks,
Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/
---
(Updated Oct. 15, 2014, 1:57 p.m.)
Review request for Aurora, Joshua Cohen and Zameer Manji.
Changes
---
Update description of the logging level parameter.
Bugs: aurora-831
https://issues.apache.org/jira/browse/aurora-831
Repository: aurora
Description
---
- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.
Diffs (updated)
-
src/main/python/apache/aurora/client/cli/__init__.py
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/main/python/apache/aurora/client/cli/options.py
dc76c25b90acb9610e40b939e65c3cabf032649f
src/main/python/apache/aurora/client/cli/standalone_client.py
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1
src/test/python/apache/aurora/client/cli/test_help.py
f73c8a3778b7d118ea2865f213b442a607fb4a7d
Diff: https://reviews.apache.org/r/26688/diff/
Testing
---
New help output:
{noformat}
[sun-wukong incubator-aurora (deprecate)]$ ./dist/aurora2.pex help cron
deschedule
Usage for verb cron deschedule:
deschedule [--verbose-logging] [--logging-level=numeric_level]
[--error-log-dir=error-log-dir] [--bind=var=value] CLUSTER/ROLE/ENV/NAME
Options:
--bind=var=value
Bind a pystachio variable name to a value. Multiple flags may be used
to specify multiple values.
CLUSTER/ROLE/ENV/NAME
Fully specified job key, in CLUSTER/ROLE/ENV/NAME format
--verbose-logging
Show verbose logging, including all logs up to level INFO (equivalent
to --logging-level=20)
--logging-level=numeric_level
Set logging to a specific numeric level.
--error-log-dir=error-log-dir
Directory location where error files containing stack traces should be
written. If the directory doesn't exist, it will be created
Remove the cron schedule for a job.
{noformat}
Thanks,
Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/#review56750
---
Ship it!
Ship It!
- Zameer Manji
On Oct. 15, 2014, 10:57 a.m., Mark Chu-Carroll wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26688/
---
(Updated Oct. 15, 2014, 10:57 a.m.)
Review request for Aurora, Joshua Cohen and Zameer Manji.
Bugs: aurora-831
https://issues.apache.org/jira/browse/aurora-831
Repository: aurora
Description
---
- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.
Diffs
-
src/main/python/apache/aurora/client/cli/__init__.py
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/main/python/apache/aurora/client/cli/options.py
dc76c25b90acb9610e40b939e65c3cabf032649f
src/main/python/apache/aurora/client/cli/standalone_client.py
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1
src/test/python/apache/aurora/client/cli/test_help.py
f73c8a3778b7d118ea2865f213b442a607fb4a7d
Diff: https://reviews.apache.org/r/26688/diff/
Testing
---
New help output:
{noformat}
[sun-wukong incubator-aurora (deprecate)]$ ./dist/aurora2.pex help cron
deschedule
Usage for verb cron deschedule:
deschedule [--verbose-logging] [--logging-level=numeric_level]
[--error-log-dir=error-log-dir] [--bind=var=value] CLUSTER/ROLE/ENV/NAME
Options:
--bind=var=value
Bind a pystachio variable name to a value. Multiple flags may be used
to specify multiple values.
CLUSTER/ROLE/ENV/NAME
Fully specified job key, in CLUSTER/ROLE/ENV/NAME format
--verbose-logging
Show verbose logging, including all logs up to level INFO (equivalent
to --logging-level=20)
--logging-level=numeric_level
Set logging to a specific numeric level.
--error-log-dir=error-log-dir
Directory location where error files containing stack traces should be
written. If the directory doesn't exist, it will be created
Remove the cron schedule for a job.
{noformat}
Thanks,
Mark Chu-Carroll
Review Request 26688: Fix errors in help rendering:
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/#review56541 --- This looks good to me, just a couple of questions about the tests. Speaking of, can you update testing done and add an example of the new help output? Also, David's on vacation, so you'll want to add someone else to this review. src/test/python/apache/aurora/client/cli/test_help.py https://reviews.apache.org/r/26688/#comment96897 Are option names guaranteed to be unique? If not this test could potentially pass if any help output contains a plugin option name, not necessarily the help output for the command to which the plugin was registered. It's also possible for a plugin option name to appear in the help for another option, and not on its own, which would cause this test to succeed even if the plugin options themselves are not properly displayed? I guess what I'm getting at is would it be better to test for more than just the appearance of a string at any point in the output? (This may be based on incomplete understanding of how the client registers commands/options). src/test/python/apache/aurora/client/cli/test_help.py https://reviews.apache.org/r/26688/#comment96898 Are there any other varieties of metavars? could we have int metavars that are similarly unset, resulting in help output that just says int rather than str? - Joshua Cohen On Oct. 14, 2014, 3:07 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- (Updated Oct. 14, 2014, 3:07 p.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/#review56545 --- src/test/python/apache/aurora/client/cli/test_help.py https://reviews.apache.org/r/26688/#comment96900 Yes, they are guaranteed to be unique. The argparse framework that this is built on checks uniqueness, and raises an exception if there's any ambiguity. src/test/python/apache/aurora/client/cli/test_help.py https://reviews.apache.org/r/26688/#comment96902 Anything can have an unset metavar, but most of the time, that isn't a problem. The default metavar for an int is int - but in help strings, that works well. For example, you'll often see [--port=int]. That's good, and I don't think we want that to be an error. Saying that the value is expected to be int is telling you something essential about the value expected for the parameter. str is different, because it's the default type value for any structured input - pathnames, config names, tunnel descriptors, instance lists, usernames, etc. Saying str doesn't tell you anything meaningful about what's expected. - Mark Chu-Carroll On Oct. 14, 2014, 11:07 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- (Updated Oct. 14, 2014, 11:07 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 26688: Fix errors in help rendering:
On Oct. 14, 2014, 1 p.m., Mark Chu-Carroll wrote: src/test/python/apache/aurora/client/cli/test_help.py, line 75 https://reviews.apache.org/r/26688/diff/1/?file=720844#file720844line75 Yes, they are guaranteed to be unique. The argparse framework that this is built on checks uniqueness, and raises an exception if there's any ambiguity. Joshua Cohen wrote: What about the second question (could an option name appearing elsewhere in the help output cause this test to improperly pass)? I don't think so - that would require *every* command to have option names that were superstrings of every option in the plugins. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/#review56545 --- On Oct. 14, 2014, 11:07 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/ --- (Updated Oct. 14, 2014, 11:07 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: aurora-831 https://issues.apache.org/jira/browse/aurora-831 Repository: aurora Description --- - Put plugin-generated options into the correct order. - Include the option-name in the detailed help list. - Add missing metavars. Diffs - src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 src/main/python/apache/aurora/client/cli/options.py dc76c25b90acb9610e40b939e65c3cabf032649f src/main/python/apache/aurora/client/cli/standalone_client.py 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 src/test/python/apache/aurora/client/cli/test_help.py f73c8a3778b7d118ea2865f213b442a607fb4a7d Diff: https://reviews.apache.org/r/26688/diff/ Testing --- Thanks, Mark Chu-Carroll
