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: fix empty string check #15500

Merged
merged 1 commit into from Jun 6, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented Jun 5, 2017

This partially reverts commit a1214a7.

Fixes: http://tracker.ceph.com/issues/20135
Signed-off-by: Sage Weil sage@redhat.com

@@ -924,12 +924,12 @@ def validate(args, signature, flags=0, partial=False):
# no arg, but not required? Continue consuming mysig
# in case there are later required args
if myarg == None and not desc.req:
if not myarg and not desc.req:

This comment has been minimized.

@tchaikov

tchaikov Jun 6, 2017

Contributor

how about

if myarg in (None, []) and not desc.req:

?

otherwise this change breaks the feature #15282 wanted to introduce. because if a is an empty string, we take it as an invalid param.

pybind/ceph_argparse: fix no arg check
This fixes breakage from commit a1214a7
that caused a [] to be appended to some json argument lists.

Fixes: http://tracker.ceph.com/issues/20135
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas changed the title from Partially revert "Pass empty string to clear mantle balancer" to pybind/ceph_argparse: fix empty string check Jun 6, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 6, 2017

retest this please

@liewegas liewegas merged commit 4a88367 into ceph:master Jun 6, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@dmick

This comment has been minimized.

dmick commented on f37ab31 Jun 7, 2017

I wonder if this should not be "if not myarg", but haven't looked to see if it could be another falsey value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment