Code review comment for lp:~marcoceppi/python-jujuclient/py3-compat

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

callable is a builtin in python3..

On Fri, Jan 24, 2014 at 10:01 AM, Marco Ceppi <email address hidden> wrote:

> Marco Ceppi has proposed merging
> lp:~marcoceppi/python-jujuclient/py3-compat into lp:python-jujuclient.
>
> Requested reviews:
> Kapil Thangavelu (hazmat)
>
> For more details, see:
>
> https://code.launchpad.net/~marcoceppi/python-jujuclient/py3-compat/+merge/203073
>
> Make jujuclient python3 compatible
> --
>
> https://code.launchpad.net/~marcoceppi/python-jujuclient/py3-compat/+merge/203073
> You are requested to review the proposed merge of
> lp:~marcoceppi/python-jujuclient/py3-compat into lp:python-jujuclient.
>
> === modified file 'jujuclient.py'
> --- jujuclient.py 2014-01-22 22:29:15 +0000
> +++ jujuclient.py 2014-01-24 14:54:35 +0000
> @@ -48,17 +48,27 @@
> """
> # License: GPLv3
> # Author: Kapil Thangavelu <email address hidden>
> +from __future__ import print_function
>
> from base64 import b64encode
> from contextlib import contextmanager
>
> -import httplib
> import json
> import pprint
> import signal
> -import StringIO
> import logging
> import websocket
> +import collections
> +
> +try:
> + import httplib
> +except ImportError:
> + import http.client as httplib
> +
> +try:
> + import StringIO
> +except ImportError:
> + import io as StringIO
>
> # There are two pypi modules with the name websocket (python-websocket
> # and websocket) We utilize python-websocket, sniff and error if we
> @@ -223,7 +233,7 @@
> def _set_alarm(cls, timeout):
> try:
> handler = signal.getsignal(signal.SIGALRM)
> - if callable(handler):
> + if isinstance(handler, collections.Callable):
> if handler.__name__ == '_set_alarm':
> raise TimeoutWatchInProgress()
> raise RuntimeError(
> @@ -482,7 +492,7 @@
>
> def _prepare_strparams(self, d):
> r = {}
> - for k, v in d.items():
> + for k, v in list(d.items()):
> r[k] = str(v)
> return r
>
> @@ -734,7 +744,7 @@
> for change_set in self.watch:
> for change in change_set:
> self.process(*change)
> - if seen_initial and callable(callback):
> + if seen_initial and isinstance(callback,
> collections.Callable):
> callback(*change)
> if self.complete() is True:
> self.watch.stop()
> @@ -768,7 +778,7 @@
>
> def complete(self):
> state = {'pending': [], 'errors': []}
> - for k, v in self.units.items():
> + for k, v in list(self.units.items()):
> if v['Status'] == "error":
> state['errors'] = [v]
> elif v['Status'] != self.goal_state:
> @@ -798,7 +808,7 @@
> self.machines[data['Id']] = data
>
> def complete(self):
> - if self.machines.keys() == ['0']:
> + if list(self.machines.keys()) == ['0']:
> return True
>
>
> @@ -840,7 +850,7 @@
>
> def _translate(self, d):
> r = {}
> - for k, v in d.items():
> + for k, v in list(d.items()):
> if k in self.remove_keys:
> continue
> if k in self.skip_empty_keys and not v:
> @@ -902,21 +912,21 @@
> env.login(juju_token)
> watcher = env.get_watch(timeout=3)
>
> - print "Env info", env.info()
> + print("Env info", env.info())
>
> for change_set in watcher:
> for change in change_set:
> - print "state change", change
> + print("state change", change)
>
> env.deploy("test-blog", "cs:wordpress")
> env.deploy("test-db", "cs:mysql")
> env.add_relation("test-db", "test-blog")
>
> - print "waiting for changes for 30s"
> + print("waiting for changes for 30s")
> watcher.set_timeout(30)
> for change_set in watcher:
> for change in change_set:
> - print "state change", change
> + print("state change", change)
>
> env.destroy_service('test-blog')
> env.destroy_service('test-db')
>
>
>

« Back to merge proposal