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

Python 3 compatibility of Ceph CLI #9702

Merged
merged 6 commits into from Jul 6, 2016
Merged

Python 3 compatibility of Ceph CLI #9702

merged 6 commits into from Jul 6, 2016

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Jun 14, 2016

This replaces tabs with whitespace, fixes syntax and builtins (most controversial change would be .iteritems() -> .items() but should be fine).

There was also one bug with the validity check that was undetected on Python 2.

Compatibility down to Python 2.6 remains.

Signed-off-by: Oleh Prypin <oleh@pryp.in>
Signed-off-by: Oleh Prypin <oleh@pryp.in>
Signed-off-by: Oleh Prypin <oleh@pryp.in>
Signed-off-by: Oleh Prypin <oleh@pryp.in>
@oprypin
Copy link
Contributor Author

oprypin commented Jun 15, 2016

What happened to the check? I don't see what went wrong from the output.

@jdurgin
Copy link
Member

jdurgin commented Jun 15, 2016

The check failure is unrelated - it's one that snuck in recently and appears in the bot sometimes but has eluded manual reproduction so far.

@jdurgin
Copy link
Member

jdurgin commented Jun 15, 2016

The changes look good, nice and straightforward. Pushed to wip-ceph-argparse-py3 and will run that through the rados suite once it's built.

@oprypin
Copy link
Contributor Author

oprypin commented Jun 15, 2016

Don't merge yet, I've discovered other needed changes when porting the ceph script. Maybe it should be one bigger pull request?

@jdurgin
Copy link
Member

jdurgin commented Jun 15, 2016

alright, sounds like a good idea to make a larger pr since the ceph_argparse tests don't cover as much as the cli tests do

@oprypin oprypin changed the title Python 3 compatibility of ceph_argparse Python 3 compatibility of Ceph CLI Jun 15, 2016
@oprypin
Copy link
Contributor Author

oprypin commented Jun 15, 2016

I continued in this pull request.

This QA suite finished successfully for me, ran like this (from src):

$ PATH="$PATH:$(readlink -f .)" PYTHON=python3 ../qa/workunits/cephtool/test.sh --asok-does-not-need-root

This needs a fresh vstart environment

It doesn't work from just the default environment, because it also needs rados and cephfs bindings built for Python 3 specifically (I put their .py/.so files right inside pybind but there is probably a better way).

@jdurgin
Copy link
Member

jdurgin commented Jun 24, 2016

lgtm, as discussed on irc we can merge this now since it doesn't break python 2, and fix any remaining python3 issues once we get python3 integration tests working

@jdurgin jdurgin merged commit fffb8fc into ceph:master Jul 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants