Merge lp://qastaging/~harlowja/cloud-init/py2-3 into lp://qastaging/~cloud-init-dev/cloud-init/trunk

Proposed by Joshua Harlow
Status: Rejected
Rejected by: Scott Moser
Proposed branch: lp://qastaging/~harlowja/cloud-init/py2-3
Merge into: lp://qastaging/~cloud-init-dev/cloud-init/trunk
Diff against target: 1969 lines (+317/-205)
59 files modified
cloudinit/config/cc_apt_configure.py (+3/-1)
cloudinit/config/cc_debug.py (+4/-2)
cloudinit/config/cc_landscape.py (+1/-1)
cloudinit/config/cc_mcollective.py (+6/-5)
cloudinit/config/cc_phone_home.py (+4/-2)
cloudinit/config/cc_puppet.py (+4/-3)
cloudinit/config/cc_resolv_conf.py (+3/-2)
cloudinit/config/cc_seed_random.py (+2/-1)
cloudinit/config/cc_ssh.py (+4/-2)
cloudinit/config/cc_yum_add_repo.py (+4/-3)
cloudinit/distros/__init__.py (+20/-19)
cloudinit/distros/arch.py (+3/-1)
cloudinit/distros/freebsd.py (+6/-4)
cloudinit/distros/net_util.py (+4/-1)
cloudinit/distros/parsers/hostname.py (+1/-1)
cloudinit/distros/parsers/hosts.py (+1/-1)
cloudinit/distros/parsers/resolv_conf.py (+1/-1)
cloudinit/distros/parsers/sys_conf.py (+5/-5)
cloudinit/distros/rhel.py (+3/-1)
cloudinit/distros/sles.py (+3/-1)
cloudinit/ec2_utils.py (+4/-5)
cloudinit/handlers/__init__.py (+1/-1)
cloudinit/handlers/boot_hook.py (+1/-1)
cloudinit/handlers/cloud_config.py (+1/-1)
cloudinit/handlers/shell_script.py (+1/-1)
cloudinit/handlers/upstart_job.py (+1/-1)
cloudinit/helpers.py (+7/-5)
cloudinit/log.py (+4/-3)
cloudinit/mergers/__init__.py (+3/-1)
cloudinit/mergers/m_dict.py (+3/-1)
cloudinit/mergers/m_list.py (+3/-1)
cloudinit/mergers/m_str.py (+6/-4)
cloudinit/netinfo.py (+5/-3)
cloudinit/signal_handler.py (+1/-1)
cloudinit/sources/DataSourceConfigDrive.py (+3/-1)
cloudinit/sources/DataSourceEc2.py (+4/-2)
cloudinit/sources/DataSourceMAAS.py (+5/-2)
cloudinit/sources/DataSourceOVF.py (+5/-3)
cloudinit/sources/DataSourceSmartOS.py (+7/-5)
cloudinit/sources/__init__.py (+6/-4)
cloudinit/sources/helpers/openstack.py (+4/-2)
cloudinit/ssh_util.py (+3/-3)
cloudinit/stages.py (+10/-9)
cloudinit/type_utils.py (+25/-6)
cloudinit/url_helper.py (+14/-7)
cloudinit/user_data.py (+6/-4)
cloudinit/util.py (+70/-45)
packages/bddeb (+1/-0)
packages/brpm (+2/-0)
requirements.txt (+3/-0)
tests/unittests/test_data.py (+7/-6)
tests/unittests/test_datasource/test_nocloud.py (+1/-1)
tests/unittests/test_datasource/test_openstack.py (+2/-3)
tests/unittests/test_distros/test_netconfig.py (+2/-3)
tests/unittests/test_handler/test_handler_locale.py (+3/-3)
tests/unittests/test_handler/test_handler_seed_random.py (+1/-1)
tests/unittests/test_handler/test_handler_set_hostname.py (+3/-3)
tests/unittests/test_handler/test_handler_timezone.py (+3/-3)
tests/unittests/test_handler/test_handler_yum_add_repo.py (+4/-3)
To merge this branch: bzr merge lp://qastaging/~harlowja/cloud-init/py2-3
Reviewer Review Type Date Requested Status
cloud-init Commiters Pending
Review via email: mp+225240@code.qastaging.launchpad.net

Description of the change

Gets the basic integration of six usage going.

This change does the following:

- Moves to using the new locations of modules (using six as needed)
  - urlparse moved, configparser moved, stringio...
- Fixes the octal usage that we had previously (0o644 is the new way that works)
- The utils load_file() now decodes the files by default from binary -> utf-8 (unless decode=False, decode=False seems needed for the configobj module to correctly work)
- The utils write_file() now decodes to binary before writing (from unicode) as needed
- Adjust tests to work correctly using the new way of load/writing files

To post a comment you must log in.
986. By Joshua Harlow

Fix urllib.quote moving to a new location

987. By Joshua Harlow

Consistently return the unicode/text version of responses

988. By Joshua Harlow

Explicitly use response.contents instead of str(contents)

To avoid using a function that has different meaning in
python 2 and python 3 instead prefer the explict access
of the contents attribute instead (which will now always
be unicode) to avoid the subtle issues that will happen
if we continue to use str() instead.

989. By Joshua Harlow

Fix the configparser being required to use stringio and not bytesio

990. By Joshua Harlow

Fix types that changed/moved

991. By Joshua Harlow

Remove another comparison for (str, basestring)

992. By Joshua Harlow

Fix all iteritems() usage and remove (str, basestring) usage

Revision history for this message
Bohuslav "Slavek" Kabrda (bkabrda) wrote :

I'm also interested in cloud-init being compatible with Python 3. I don't understand cloud-init codebase that much, but the patch looks fine. My question is, what minimal Python version are you targeting? I'm assuming 2.6 and higher? If so, I'd recommend using dict.{items,keys,values} instead of six.iter{items,keys,values}(dict) - it's more readable (but really just a nitpick).

Revision history for this message
Joshua Harlow (harlowja) wrote :

2.6 and higher, as for the dict stuff, meh.

Revision history for this message
Zane Bitter (zaneb) wrote :

Looks good to me.

Minor point: "import pickle" in http://bazaar.launchpad.net/~harlowja/cloud-init/py2-3/revision/983/cloudinit/stages.py could be "from six.moves import cPickle as pickle" to avoid slowing down the existing Python 2 code.

Revision history for this message
Barry Warsaw (barry) wrote :

Is this branch behind trunk? I had some merge conflicts which aren't showing up in this MP.

I'm working on a new branch, highly inspired by this one, which should be more current against trunk, and use tox to ensure py2/3 compatibility. Most of the code changes here I agree with (except the iteritems ones -- I'm with Bohuslav on that :).

We have to disable the cheetah test in py3 since that package is not compatible. The biggest problem will be with the use of mocker which isn't py3 compatible. Not sure how to deal with that yet.

Revision history for this message
Joshua Harlow (harlowja) wrote :

Likely is behind (seeing that its not updated in a while); if you are working on a newer branch (derived from this one) that's cool and probably means we don't need this one at that point. Maybe time to slowly (or fastly) move to mock then...

Revision history for this message
Barry Warsaw (barry) wrote :

Here's my WIP branch: lp:~barry/cloud-init/py2-3

This largely merges Joshua's branch here, albeit manually because of aforementioned problems. It adds tox support for bilingual test runs, and the Python 2.7 test suite passes fully for me (try `tox -e py27`).

My next steps will be to look at replacing mocker and then repairing the Python 3 test suite. I'll also need to scrounge up a Python 2.6 to make sure support for that version hasn't broken.

I'll create a MP for my branch and welcome all comments and contributions!

Revision history for this message
Scott Moser (smoser) wrote :

dont know what the right way to do this is, but this is effectively merged. so no need for this review, so i rejected it.

Unmerged revisions

992. By Joshua Harlow

Fix all iteritems() usage and remove (str, basestring) usage

991. By Joshua Harlow

Remove another comparison for (str, basestring)

990. By Joshua Harlow

Fix types that changed/moved

989. By Joshua Harlow

Fix the configparser being required to use stringio and not bytesio

988. By Joshua Harlow

Explicitly use response.contents instead of str(contents)

To avoid using a function that has different meaning in
python 2 and python 3 instead prefer the explict access
of the contents attribute instead (which will now always
be unicode) to avoid the subtle issues that will happen
if we continue to use str() instead.

987. By Joshua Harlow

Consistently return the unicode/text version of responses

986. By Joshua Harlow

Fix urllib.quote moving to a new location

985. By Joshua Harlow

Fix up the unittests due to new changes

984. By Joshua Harlow

Adjust a bunch of moved StringIO imports

983. By Joshua Harlow

Fix a bunch more octal changes and import moves

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.