Remove py33 incompatibilities

Except py33 compatibility changes were added unittests
to cover changed code.

Partially-implements blueprint py3-compatibility
Change-Id: Ib51e02ec69100a842ea1092e9f659ab1ebede671

manilaclient/base.py

@@ -23,6 +23,8 @@ import contextlib
 import hashlib
 import os
 
+import six
+
 from manilaclient import exceptions
 from manilaclient.openstack.common import strutils
 from manilaclient import utils
@@ -104,7 +106,8 @@ class Manager(utils.HookableMixin):
         # pair
         username = utils.env('OS_USERNAME', 'MANILA_USERNAME')
         url = utils.env('OS_URL', 'MANILA_URL')
-        uniqifier = hashlib.md5(username + url).hexdigest()
+        uniqifier = hashlib.md5(username.encode('utf-8') +
+                                url.encode('utf-8')).hexdigest()
 
         cache_dir = os.path.expanduser(os.path.join(base_dir, uniqifier))
 
@@ -197,7 +200,7 @@ class ManagerWithFind(Manager):
         the Python side.
         """
         found = []
-        searches = kwargs.items()
+        searches = list(kwargs.items())
 
         for obj in self.list():
             try:
@@ -251,7 +254,7 @@ class Resource(object):
         return None
 
     def _add_details(self, info):
-        for (k, v) in info.iteritems():
+        for (k, v) in six.iteritems(info):
             try:
                 setattr(self, k, v)
             except AttributeError:
@@ -270,7 +273,7 @@ class Resource(object):
             return self.__dict__[k]
 
     def __repr__(self):
-        reprkeys = sorted(k for k in self.__dict__.keys() if k[0] != '_' and
+        reprkeys = sorted(k for k in self.__dict__ if k[0] != '_' and
                           k != 'manager')
         info = ", ".join("%s=%s" % (k, getattr(self, k)) for k in reprkeys)
         return "<%s %s>" % (self.__class__.__name__, info)

manilaclient/client.py

@@ -11,7 +11,11 @@ from __future__ import print_function
 
 import logging
 import os
-import urlparse
+try:
+    import urlparse
+except ImportError:
+    import urllib.parse as urlparse
+
 try:
     from eventlet import sleep
 except ImportError:
@@ -380,7 +384,7 @@ def get_client_class(version):
         client_path = version_map[str(version)]
     except (KeyError, ValueError):
         msg = "Invalid client version '%s'. must be one of: %s" % (
-            (version, ', '.join(version_map.keys())))
+            (version, ', '.join(version_map)))
         raise exceptions.UnsupportedVersion(msg)
 
     return utils.import_class(client_path)

manilaclient/exceptions.py

@@ -142,7 +142,7 @@ def from_response(response, body):
         message = "n/a"
         details = "n/a"
         if hasattr(body, 'keys'):
-            error = body[body.keys()[0]]
+            error = body[list(body)[0]]
             message = error.get('message', None)
             details = error.get('details', None)
         return cls(code=response.status_code, message=message, details=details,

manilaclient/extension.py

@@ -29,7 +29,7 @@ class Extension(utils.HookableMixin):
 
     def _parse_extension_module(self):
         self.manager_class = None
-        for attr_name, attr_value in self.module.__dict__.items():
+        for attr_name, attr_value in list(self.module.__dict__.items()):
             if attr_name in self.SUPPORTED_HOOKS:
                 self.add_hook(attr_name, attr_value)
             elif utils.safe_issubclass(attr_value, base.Manager):

manilaclient/shell.py

@@ -29,6 +29,8 @@ import os
 import pkgutil
 import sys
 
+import six
+
 from manilaclient import client
 from manilaclient import exceptions as exc
 import manilaclient.extension
@@ -422,9 +424,9 @@ class OpenStackManilaShell(object):
         """
         commands = set()
         options = set()
-        for sc_str, sc in self.subcommands.items():
+        for sc_str, sc in list(self.subcommands.items()):
             commands.add(sc_str)
-            for option in sc._optionals._option_string_actions.keys():
+            for option in sc._optionals._option_string_actions:
                 options.add(option)
 
         commands.remove('bash-completion')
@@ -455,14 +457,18 @@ class OpenStackHelpFormatter(argparse.HelpFormatter):
 
 def main():
     try:
-        OpenStackManilaShell().main(map(strutils.safe_decode, sys.argv[1:]))
+        if sys.version_info >= (3, 0):
+            OpenStackManilaShell().main(sys.argv[1:])
+        else:
+            OpenStackManilaShell().main(
+                map(strutils.safe_decode, sys.argv[1:]))
     except KeyboardInterrupt:
         print("... terminating manila client", file=sys.stderr)
         sys.exit(130)
     except Exception as e:
         logger.debug(e, exc_info=1)
         message = e.message
-        if not isinstance(message, basestring):
+        if not isinstance(message, six.string_types):
             message = str(message)
         print("ERROR: %s" % strutils.safe_encode(message), file=sys.stderr)
         sys.exit(1)

manilaclient/utils.py

@@ -5,6 +5,7 @@ import sys
 import uuid
 
 import prettytable
+import six
 
 from manilaclient import exceptions
 from manilaclient.openstack.common import strutils
@@ -70,7 +71,7 @@ def get_resource_manager_extra_kwargs(f, args, allow_conflicts=False):
         hook_name = hook.__name__
         hook_kwargs = hook(args)
 
-        conflicting_keys = set(hook_kwargs.keys()) & set(extra_kwargs.keys())
+        conflicting_keys = set(list(hook_kwargs)) & set(list(extra_kwargs))
         if conflicting_keys and not allow_conflicts:
             msg = ("Hook '%(hook_name)s' is attempting to redefine attributes "
                    "'%(conflicting_keys)s'" % {
@@ -129,7 +130,14 @@ def pretty_choice_list(l):
     return ', '.join("'%s'" % i for i in l)
 
 
-def print_list(objs, fields, formatters={}):
+def _print(pt, order):
+    if sys.version_info >= (3, 0):
+        print(pt.get_string(sortby=order))
+    else:
+        print(strutils.safe_encode(pt.get_string(sortby=order)))
+
+
+def print_list(objs, fields, formatters={}, order_by=None):
     mixed_case_fields = ['serverId']
     pt = prettytable.PrettyTable([f for f in fields], caching=False)
     pt.aligns = ['l' for f in fields]
@@ -148,14 +156,17 @@ def print_list(objs, fields, formatters={}):
                 row.append(data)
         pt.add_row(row)
 
-    print(strutils.safe_encode(pt.get_string(sortby=fields[0])))
+    if order_by is None:
+        order_by = fields[0]
+
+    _print(pt, order_by)
 
 
 def print_dict(d, property="Property"):
     pt = prettytable.PrettyTable([property, 'Value'], caching=False)
     pt.aligns = ['l', 'l']
-    [pt.add_row(list(r)) for r in d.iteritems()]
+    [pt.add_row(list(r)) for r in six.iteritems(d)]
-    print(strutils.safe_encode(pt.get_string(sortby=property)))
+    _print(pt, property)
 
 
 def find_resource(manager, name_or_id):
@@ -167,9 +178,12 @@ def find_resource(manager, name_or_id):
     except exceptions.NotFound:
         pass
 
+    if sys.version_info <= (3, 0):
+        name_or_id = strutils.safe_decode(name_or_id)
+
     # now try to get entity as uuid
     try:
-        uuid.UUID(strutils.safe_decode(name_or_id))
+        uuid.UUID(name_or_id)
         return manager.get(name_or_id)
     except (ValueError, exceptions.NotFound):
         pass
@@ -208,7 +222,7 @@ def find_share(cs, share):
 
 def _format_servers_list_networks(server):
     output = []
-    for (network, addresses) in server.networks.items():
+    for (network, addresses) in list(server.networks.items()):
         if len(addresses) == 0:
             continue
         addresses_csv = ', '.join(addresses)

manilaclient/v1/limits.py

@@ -11,7 +11,7 @@ class Limits(base.Resource):
 
     @property
     def absolute(self):
-        for (name, value) in self._info['absolute'].items():
+        for (name, value) in list(self._info['absolute'].items()):
             yield AbsoluteLimit(name, value)
 
     @property

manilaclient/v1/quota_classes.py

@@ -51,7 +51,7 @@ class QuotaClassSetManager(base.ManagerWithFind):
             }
         }
 
-        for key in body['quota_class_set'].keys():
+        for key in list(body['quota_class_set']):
             if body['quota_class_set'][key] is None:
                 body['quota_class_set'].pop(key)
 

manilaclient/v1/quotas.py

@@ -53,7 +53,7 @@ class QuotaSetManager(base.ManagerWithFind):
             },
         }
 
-        for key in body['quota_set'].keys():
+        for key in list(body['quota_set']):
             if body['quota_set'][key] is None:
                 body['quota_set'].pop(key)
         if user_id:

manilaclient/v1/security_services.py

@@ -12,7 +12,11 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-import urllib
+
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 from manilaclient import exceptions
@@ -131,10 +135,8 @@ class SecurityServiceManager(base.Manager):
         :rtype: list of :class:`SecurityService`
         """
         if search_opts:
-            query_string = urllib.urlencode([(key, value)
+            query_string = urlencode(
-                                             for (key, value)
+                sorted([(k, v) for (k, v) in list(search_opts.items()) if v]))
-                                             in search_opts.items()
-                                             if value])
             if query_string:
                 query_string = "?%s" % query_string
         else:

manilaclient/v1/services.py

@@ -13,7 +13,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import urllib
+import six
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 
@@ -38,10 +42,8 @@ class ServiceManager(base.Manager):
         """
         query_string = ''
         if search_opts:
-            query_string = urllib.urlencode([(key, value)
+            query_string = urlencode(
-                                             for (key, value)
+                sorted([(k, v) for (k, v) in six.iteritems(search_opts) if v]))
-                                             in search_opts.items()
-                                             if value])
             if query_string:
                 query_string = "?%s" % query_string
         return self._list(RESOURCES_PATH + query_string, RESOURCES_NAME)

manilaclient/v1/share_networks.py

@@ -12,7 +12,11 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-import urllib
+
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 from manilaclient import exceptions
@@ -142,10 +146,8 @@ class ShareNetworkManager(base.ManagerWithFind):
         :rtype: list of :class:`NetworkInfo`
         """
         if search_opts:
-            query_string = urllib.urlencode([(key, value)
+            query_string = urlencode(
-                                             for (key, value)
+                sorted([(k, v) for (k, v) in list(search_opts.items()) if v]))
-                                             in search_opts.items()
-                                             if value])
             if query_string:
                 query_string = "?%s" % query_string
         else:

manilaclient/v1/share_servers.py

@@ -13,7 +13,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import urllib
+import six
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 
@@ -52,7 +56,7 @@ class ShareServerManager(base.Manager):
         # +---------------------+------------------------------------+
         # | details:instance_id |35203a78-c733-4b1f-b82c-faded312e537|
         # +---------------------+------------------------------------+
-        for k, v in server._info["backend_details"].iteritems():
+        for k, v in six.iteritems(server._info["backend_details"]):
             server._info["details:%s" % k] = v
         return server
 
@@ -71,7 +75,8 @@ class ShareServerManager(base.Manager):
         """
         query_string = ''
         if search_opts:
-            opts = [(k, v) for (k, v) in search_opts.items() if v]
+            opts = sorted(
-            query_string = urllib.urlencode(opts)
+                [(k, v) for (k, v) in six.iteritems(search_opts) if v])
+            query_string = urlencode(opts)
             query_string = '?' + query_string if query_string else ''
         return self._list(RESOURCES_PATH + query_string, RESOURCES_NAME)

manilaclient/v1/share_snapshots.py

@@ -14,7 +14,10 @@
 #    under the License.
 """Interface for shares extension."""
 
-import urllib
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 
@@ -77,10 +80,8 @@ class ShareSnapshotManager(base.ManagerWithFind):
         :rtype: list of :class:`ShareSnapshot`
         """
         if search_opts:
-            query_string = urllib.urlencode([(key, value)
+            query_string = urlencode(
-                                             for (key, value)
+                sorted([(k, v) for (k, v) in list(search_opts.items()) if v]))
-                                             in search_opts.items()
-                                             if value])
             if query_string:
                 query_string = "?%s" % (query_string,)
         else:

manilaclient/v1/shares.py

@@ -16,7 +16,10 @@
 
 import collections
 import re
-import urllib
+try:
+    from urllib import urlencode  # noqa
+except ImportError:
+    from urllib.parse import urlencode  # noqa
 
 from manilaclient import base
 from manilaclient import exceptions
@@ -164,10 +167,8 @@ class ShareManager(base.ManagerWithFind):
         :rtype: list of :class:`Share`
         """
         if search_opts:
-            query_string = urllib.urlencode([(key, value)
+            query_string = urlencode(
-                                             for (key, value)
+                sorted([(k, v) for (k, v) in list(search_opts.items()) if v]))
-                                             in search_opts.items()
-                                             if value])
             if query_string:
                 query_string = "?%s" % (query_string,)
         else:
@@ -214,7 +215,7 @@ class ShareManager(base.ManagerWithFind):
         """Get access list to the share."""
         access_list = self._action("os-access_list", share)[1]["access_list"]
         if access_list:
-            t = collections.namedtuple('Access', access_list[0].keys())
+            t = collections.namedtuple('Access', list(access_list[0]))
             return [t(*value.values()) for value in access_list]
         else:
             return []

manilaclient/v1/shell.py

@@ -82,7 +82,7 @@ def _find_share_network(cs, share_network):
 
 def _translate_keys(collection, convert):
     for item in collection:
-        keys = item.__dict__.keys()
+        keys = item.__dict__
         for from_key, to_key in convert:
             if from_key in keys and to_key not in keys:
                 setattr(item, to_key, item._info[from_key])
@@ -360,8 +360,7 @@ def do_metadata(cs, args):
     if args.action == 'set':
         cs.shares.set_metadata(share, metadata)
     elif args.action == 'unset':
-        cs.shares.delete_metadata(share, sorted(metadata.keys(),
+        cs.shares.delete_metadata(share, sorted(list(metadata), reverse=True))
-                                  reverse=True))
 
 
 @utils.arg('share', metavar='<share>',
@@ -403,10 +402,7 @@ def do_delete(cs, args):
             share_ref.delete()
         except Exception as e:
             failure_count += 1
-            if 'Access was denied' in e.message:
+            print("Delete for share %s failed: %s" % (share_ref.id, e))
-                print('Error occurred while deleting share %s' % share_ref.id)
-            else:
-                print(e.message)
 
     if failure_count == len(args.share):
         raise exceptions.CommandError("Unable to delete any of the specified "

tests/fakes.py

@@ -9,13 +9,12 @@ places where actual behavior differs from the spec.
 from __future__ import print_function
 
 
-def assert_has_keys(dict, required=[], optional=[]):
+def assert_has_keys(dictonary, required=[], optional=[]):
-    keys = dict.keys()
     for k in required:
         try:
-            assert k in keys
+            assert k in dictonary
         except AssertionError:
-            extra_keys = set(keys).difference(set(required + optional))
+            extra_keys = set(dictonary).difference(set(required + optional))
             raise AssertionError("found unexpected keys: %s" %
                                  list(extra_keys))
 

tests/test_shell.py

@@ -1,8 +1,8 @@
-import cStringIO
 import re
 import sys
 
 import fixtures
+from six import moves
 from testtools import matchers
 
 from manilaclient import exceptions
@@ -29,7 +29,7 @@ class ShellTest(utils.TestCase):
     def shell(self, argstr):
         orig = sys.stdout
         try:
-            sys.stdout = cStringIO.StringIO()
+            sys.stdout = moves.StringIO()
             _shell = manilaclient.shell.OpenStackManilaShell()
             _shell.main(argstr.split())
         except SystemExit:

tests/v1/fake_clients.py

@@ -12,7 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import urlparse
+try:
+    import urlparse
+except ImportError:
+    import urllib.parse as urlparse
 
 from manilaclient import client as base_client
 from manilaclient.v1 import client
@@ -104,7 +107,7 @@ class FakeHTTPClient(base_client.HTTPClient):
         return (200, {}, quota_set)
 
     def put_os_quota_sets_test(self, body, **kw):
-        assert body.keys() == ['quota_set']
+        assert list(body) == ['quota_set']
         fakes.assert_has_keys(body['quota_set'],
                               required=['tenant_id'])
         quota_set = {
@@ -137,7 +140,7 @@ class FakeHTTPClient(base_client.HTTPClient):
         return (200, {}, quota_class_set)
 
     def put_os_quota_class_sets_test(self, body, **kw):
-        assert body.keys() == ['quota_class_set']
+        assert list(body) == ['quota_class_set']
         fakes.assert_has_keys(body['quota_class_set'],
                               required=['class_name'])
         quota_class_set = {

tests/v1/fakes.py

@@ -75,13 +75,16 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
     def post_shares_1234_action(self, body, **kw):
         _body = None
         resp = 202
-        assert len(body.keys()) == 1
+        assert len(list(body)) == 1
-        action = body.keys()[0]
+        action = list(body)[0]
         if action == 'os-allow_access':
-            assert body[action].keys() == ['access_type', 'access_to']
+            expected = ['access_to', 'access_type']
+            actual = sorted(list(body[action]))
+            err_msg = "expected '%s', actual is '%s'" % (expected, actual)
+            assert expected == actual, err_msg
             _body = {'access': {}}
         elif action == 'os-deny_access':
-            assert body[action].keys() == ['access_id']
+            assert list(body[action]) == ['access_id']
         elif action == 'os-access_list':
             assert body[action] is None
         elif action == 'os-reset_status':

tests/v1/test_security_services.py

@@ -103,7 +103,7 @@ class SecurityServiceTest(utils.TestCase):
         filters = OrderedDict([('all_tenants', 1),
                                ('status', 'ERROR'),
                                ('network', 'fake_network')])
-        expected_postfix = '?all_tenants=1&status=ERROR&network=fake_network'
+        expected_postfix = '?all_tenants=1&network=fake_network&status=ERROR'
 
         with mock.patch.object(self.manager, '_list',
                                mock.Mock(return_value=None)):

tests/v1/test_services.py

@@ -31,4 +31,28 @@ class ServicesTest(utils.TestCase):
             self.manager.list()
             self.manager._list.assert_called_once_with(
                 services.RESOURCES_PATH,
-                services.RESOURCES_NAME)
+                services.RESOURCES_NAME,
+            )
+
+    def test_list_services_with_one_search_opt(self):
+        host = 'fake_host'
+        query_string = "?host=%s" % host
+        with mock.patch.object(self.manager, '_list',
+                               mock.Mock(return_value=None)):
+            self.manager.list({'host': host})
+            self.manager._list.assert_called_once_with(
+                services.RESOURCES_PATH + query_string,
+                services.RESOURCES_NAME,
+            )
+
+    def test_list_services_with_two_search_opts(self):
+        host = 'fake_host'
+        binary = 'fake_binary'
+        query_string = "?binary=%s&host=%s" % (binary, host)
+        with mock.patch.object(self.manager, '_list',
+                               mock.Mock(return_value=None)):
+            self.manager.list({'binary': binary, 'host': host})
+            self.manager._list.assert_called_once_with(
+                services.RESOURCES_PATH + query_string,
+                services.RESOURCES_NAME,
+            )

tests/v1/test_share_servers.py

@@ -42,6 +42,29 @@ class ShareServersTest(utils.TestCase):
                 share_servers.RESOURCES_PATH,
                 share_servers.RESOURCES_NAME)
 
+    def test_list_with_one_search_opt(self):
+        host = 'fake_host'
+        query_string = "?host=%s" % host
+        with mock.patch.object(self.manager, '_list',
+                               mock.Mock(return_value=None)):
+            self.manager.list({'host': host})
+            self.manager._list.assert_called_once_with(
+                share_servers.RESOURCES_PATH + query_string,
+                share_servers.RESOURCES_NAME,
+            )
+
+    def test_list_with_two_search_opts(self):
+        host = 'fake_host'
+        status = 'fake_status'
+        query_string = "?host=%s&status=%s" % (host, status)
+        with mock.patch.object(self.manager, '_list',
+                               mock.Mock(return_value=None)):
+            self.manager.list({'host': host, 'status': status})
+            self.manager._list.assert_called_once_with(
+                share_servers.RESOURCES_PATH + query_string,
+                share_servers.RESOURCES_NAME,
+            )
+
     def test_get(self):
         server = FakeShareServer()
         with mock.patch.object(self.manager, '_get',
@@ -51,8 +74,8 @@ class ShareServersTest(utils.TestCase):
             self.manager._get.assert_called_once_with(
                 "%s/%s" % (share_servers.RESOURCES_PATH, share_server_id),
                 share_servers.RESOURCE_NAME)
-            self.assertTrue("details:fake_key1" in server._info.keys())
+            for key in ["details:fake_key1", "details:fake_key2"]:
-            self.assertTrue("details:fake_key2" in server._info.keys())
+                self.assertTrue(key in list(server._info))
 
     def test_details(self):
         with mock.patch.object(self.manager, '_get',

tests/v1/test_shell.py

@@ -101,7 +101,7 @@ class ShellTest(utils.TestCase):
     def test_snapshot_list_filter_status_and_share_id(self):
         self.run_command('snapshot-list --status=available --share-id=1234')
         self.assert_called('GET', '/snapshots/detail?'
-                           'status=available&share_id=1234')
+                           'share_id=1234&status=available')
 
     def test_rename(self):
         # basic rename with positional agruments

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 distribute = False
-envlist = py26,py27,pep8
+envlist = py26,py27,py33,pep8
 minversion = 1.6
 skipsdist = True