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

ceph.in: drop the compatiiblity to handle non json commands #15508

Merged
merged 1 commit into from Jun 9, 2017

Conversation

Projects
None yet
3 participants
@tchaikov
Contributor

tchaikov commented Jun 6, 2017

we has been using json command since giant. no need to bring this cruft
into luminous.

Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov added the cleanup label Jun 6, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 6, 2017

@Liuchang0812 it's a follow-up inspired by your cleanup work =)

@Liuchang0812

This comment has been minimized.

Contributor

Liuchang0812 commented Jun 6, 2017

😄 I'm happy to review and test it.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 6, 2017

that'd be great! thanks.

@Liuchang0812

This comment has been minimized.

Contributor

Liuchang0812 commented Jun 6, 2017

Hi, Kefu. here are some problems with ctest -R "cephtool-test-*" --output-on-failure:

/home/liuchang/WorkSpace/ceph/qa/workunits/cephtool/test.sh:47: expect_false:  ceph mds set allow_new_snaps taco
2017-06-06 19:24:28.577335 7f85cb453700 -1 WARNING: all dangerous and experimental features are enabled.
2017-06-06 19:24:28.593196 7f85cb453700 -1 WARNING: all dangerous and experimental features are enabled.
Traceback (most recent call last):
  File "/home/liuchang/WorkSpace/ceph/build/bin/ceph", line 1102, in <module>
    retval = main()
  File "/home/liuchang/WorkSpace/ceph/build/bin/ceph", line 1037, in main
    verbose)
  File "/home/liuchang/WorkSpace/ceph/build/bin/ceph", line 567, in new_style_command
    inbuf=inbuf)
  File "/home/liuchang/WorkSpace/ceph/src/pybind/ceph_argparse.py", line 1352, in json_command
    argdict, outs))
RuntimeError: "{u'var': u'allow_new_snaps', 'prefix': u'mds set', u'val': u'taco'}": bad command or argument: value must be false|no|0 or true|yes|1
/home/liuchang/WorkSpace/ceph/qa/workunits/cephtool/test.sh:47: expect_false:  return 0

It seems that the logical of return code is broken.

@@ -1347,4 +1347,7 @@ def json_command(cluster, target=('mon', ''), prefix=None, argdict=None,
else:
raise
if ret == -errno.EINVAL:
raise RuntimeError('"{0}": bad command or argument: {1}'.format(

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 6, 2017

Contributor

Here is the problem. return json_command("foo") will get 0 when json_command raises exception here!

Maybe we shouldn't panic here, how about putting error message to outs only.

This comment has been minimized.

@tchaikov

tchaikov Jun 6, 2017

Contributor

yeah, i am dropping this hunk.

ceph.in: drop the compatiiblity to handle non json commands
we has been using json command since giant. no need to bring this cruft
into luminous.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@Liuchang0812

lgtm

@liewegas liewegas merged commit 642fe3b into ceph:master Jun 9, 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

@tchaikov tchaikov deleted the tchaikov:wip-ceph.in-cleanup branch Jun 9, 2017

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