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: support mon target in pybind #15409

Merged
merged 2 commits into from Jun 5, 2017

Conversation

Projects
None yet
3 participants
@Liuchang0812
Contributor

Liuchang0812 commented Jun 1, 2017

The first commit supports mon target in pybind.

I fix some format problems based on PEP8 in the second commit. I will revert the second commit if it's inappropriate.

@tchaikov mind taking a look?

@Liuchang0812

This comment has been minimized.

Contributor

Liuchang0812 commented Jun 1, 2017

local test result:

➜  build git:(wip-support-mon-target-in-pybind) ✗ ctest -R "cephtool-test-*"   --output-on-failure
Test project /home/liuchang/WorkSpace/ceph/build
    Start 4: cephtool-test-mds.sh
1/4 Test #4: cephtool-test-mds.sh .............   Passed   68.90 sec
    Start 5: cephtool-test-mon.sh
2/4 Test #5: cephtool-test-mon.sh .............   Passed  1459.36 sec
    Start 6: cephtool-test-osd.sh
3/4 Test #6: cephtool-test-osd.sh .............   Passed   24.87 sec
    Start 7: cephtool-test-rados.sh
4/4 Test #7: cephtool-test-rados.sh ...........   Passed  196.65 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) = 1749.83 sec
src/ceph.in Outdated
if 'mon.' not in name:
print('"ping" expects a monitor to ping; try "ping mon.<id>"', file=sys.stderr)
return 1
mon_id = name[len('mon.'):]
if (mon_id == '*') :
if (mon_id == '*'):

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

since you at this, could you remove the parentheses?

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

addressed

src/ceph.in Outdated
def ping_monitor(cluster_handle, name, timeout):
"""Ping a monitor

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

could drop this comment, it is barely helpful.

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

addressed

src/ceph.in Outdated
print('[Contacting monitor, timeout after %d seconds]' % timeout)
if verbose:
print('[Contacting monitor, timeout after %d seconds]' % timeout)
return do_extended_help(parser, childargs, ('mon', ''), ' '.join(childargs))

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

is this part of the pep8 change?

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

oh, sorry. i think it's an improvement. ceph always prints [Contacting monitor, timeout after 5 seconds] when user typed ceph --help, as following:

[root@node44 /data/seal/deploy]# ceph osd --help
[a lot of help messages]

 Monitor commands:
 =================
[Contacting monitor, timeout after 5 seconds]
2017-06-02 11:55:28.437591 7f49598db700 -1 WARNING: the following dangerous and experimental features are enabled: bluestore,rocksdb
auth add <entity> {<caps> [<caps>...]}   add auth info for <entity> from input
                                          file, or random key if no input is
                                          given, and/or any caps specified in
                                          the command
src/ceph.in Outdated
@@ -913,17 +921,14 @@ def main():
print(str(e), file=sys.stderr)
return 1
if parsed_args.help:

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

ditto.

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

it is a bug. i removed this change. it should be split to another PR.

[root@node44 /data/seal/deploy]# ceph ping --help

 Monitor commands: 
 =================
[Contacting monitor, timeout after 5 seconds]
"ping" requires a monitor name as argument: "ping mon.<id>"

This comment has been minimized.

@tchaikov

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

yes, we print Monitor commands: ===== [Contac..........] here, without any command explain. the problem is that:

  1. we print header of help command at first
  2. we check ping command. ( ceph ping --help exits here)
  3. we print boby of help command.

we should put 1 and 3 together.

This comment has been minimized.

@tchaikov

tchaikov Jun 2, 2017

Contributor

@Liuchang0812 sorry for the confusion, i mean, we should not include this change in the pep8 commit.

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 2, 2017

Contributor

: ) I will remove it soon

This comment has been minimized.

@Liuchang0812

Liuchang0812 Jun 5, 2017

Contributor

done.

@Liuchang0812

This comment has been minimized.

Contributor

Liuchang0812 commented Jun 2, 2017

jenkins test this please

@Liuchang0812

This comment has been minimized.

Contributor

Liuchang0812 commented Jun 5, 2017

➜  build git:(wip-support-mon-target-in-pybind) ✗ ctest -R "cephtool-test-*"   --output-on-failure
Test project /home/liuchang/WorkSpace/ceph/build
    Start 4: cephtool-test-mds.sh
1/4 Test #4: cephtool-test-mds.sh .............   Passed   78.04 sec
    Start 5: cephtool-test-mon.sh
2/4 Test #5: cephtool-test-mon.sh .............   Passed  1449.85 sec
    Start 6: cephtool-test-osd.sh
3/4 Test #6: cephtool-test-osd.sh .............   Passed   18.97 sec
    Start 7: cephtool-test-rados.sh
4/4 Test #7: cephtool-test-rados.sh ...........   Passed  199.65 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) = 1746.57 sec
➜  build git:(wip-support-mon-target-in-pybind) ✗ pep8 ../src/ceph.in --max-line-length=180       
../src/ceph.in:118:1: E402 module level import not at top of file
../src/ceph.in:119:1: E402 module level import not at top of file
../src/ceph.in:120:1: E402 module level import not at top of file
../src/ceph.in:121:1: E402 module level import not at top of file
../src/ceph.in:122:1: E402 module level import not at top of file
../src/ceph.in:123:1: E402 module level import not at top of file
../src/ceph.in:124:1: E402 module level import not at top of file
../src/ceph.in:125:1: E402 module level import not at top of file
../src/ceph.in:127:1: E402 module level import not at top of file
../src/ceph.in:132:1: E402 module level import not at top of file

@tchaikov tchaikov added the needs-qa label Jun 5, 2017

Liuchang0812 added some commits May 31, 2017

pybind: support mon target, and clean up tool
Signed-off-by: liuchang0812 <liuchang0812@gmail.com>
tool/ceph: pep8 clean up
Signed-off-by: liuchang0812 <liuchang0812@gmail.com>

@liewegas liewegas merged commit 52a9830 into ceph:master Jun 5, 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

@Liuchang0812 Liuchang0812 deleted the Liuchang0812:wip-support-mon-target-in-pybind branch Jun 8, 2017

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