Skip to content
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

pybind/ceph_argparse: accept flexible req #20791

Merged
merged 1 commit into from Mar 9, 2018

Conversation

Projects
None yet
3 participants
@guzhongyan
Copy link
Contributor

commented Mar 8, 2018

python bool var is 'True/False' not 'true/false'
found this bug while using cmd 'ceph osd ls-tree'
with weird output:

$ ceph osd ls-tree
Error ENOENT: "" does not exist

Signed-off-by: Gu Zhongyan guzhongyan@360.cn

@guzhongyan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2018

with the fix, the output makes sense:
$ceph osd ls-tree
Invalid command: missing required parameter name()
osd ls-tree {<int[0-]>} : show OSD ids under bucket in the CRUSH map
Error EINVAL: invalid command

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

i'd suggest change the constructor of argdesc in ceph_argparse.py. simpler this way, and more error prone.

@jcsp

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

I agree with @tchaikov -- better to make the argparse code more flexible instead of changing all the command definitions.

@guzhongyan guzhongyan force-pushed the guzhongyan:fix-pycmd branch from 72c2ca8 to 2c4c35e Mar 9, 2018

@guzhongyan guzhongyan changed the title mon,mgr: fix CLI bool value for python parsing pybind/ceph_argparse: accept flexible req Mar 9, 2018

@guzhongyan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2018

Thank you and updated the commit according your comments.

@@ -618,7 +618,7 @@ def __init__(self, t, name=None, n=1, req=True, **kwargs):
else:
self.t = t
self.typeargs = kwargs
self.req = bool(req == True or req == 'True')
self.req = bool(req == True or req == 'True' or req == 'true')

This comment has been minimized.

Copy link
@tchaikov

tchaikov Mar 9, 2018

Contributor

could also put

self.req = req in (True, 'True', 'true')

better readability this way.

This comment has been minimized.

Copy link
@guzhongyan

guzhongyan Mar 9, 2018

Author Contributor

Thanks. Done.

@guzhongyan guzhongyan force-pushed the guzhongyan:fix-pycmd branch from 2c4c35e to a352925 Mar 9, 2018

pybind/ceph_argparse: accept flexible req
True and true both acceptable

Signed-off-by: Gu Zhongyan <guzhongyan@360.cn>

@guzhongyan guzhongyan force-pushed the guzhongyan:fix-pycmd branch from a352925 to 9bdf43e Mar 9, 2018

@jcsp

jcsp approved these changes Mar 9, 2018

@tchaikov tchaikov merged commit b296f5c into ceph:master Mar 9, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@guzhongyan guzhongyan deleted the guzhongyan:fix-pycmd branch Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.