Make manila-ui python3 compatible

In order to make manila-ui python3 compatible,
this patch set:
- replace dict.iteritems() with six.iteritems(dict)
- replace print msg with print(msg)
- replace dict.viewkeys() with error.keys()
- python 3 getting the keys() of a dict returns
  a dict_keys object instead of a list and hence
  sort() fails
- in python 2.x calling keys makes a copy of the key
  that you can iterate over while modifying the dict.
  This doesn't work in python 3.x because keys
  returns an iterator instead of a list.
  Another way is to use list to force a copy of the keys
  to be made.
- use getattr instead of relying on overriden __getattr__
  in the client to get params that not neccesarily need to
  be available (share_group_id and supports_share_migration)
- refactor ShareGroupTypeTests

Plus, enable py35 and py35dj20 jobs in tox and drop
old oslo-incubator files (install_venv.py)

Also add openstack-tox-py35 job to check python3 env.
openstack-tox-py35 should be defined in project-config later
but it is added to check python3 UT status.

Change-Id: I30582184904dc19e9cb4ca171b1fd2d161e09c48

.zuul.yaml

@@ -6,11 +6,19 @@
         - openstack-tox-lower-constraints:
             required-projects:
               openstack/horizon
+        # TODO(amotoki): Drop this after project-config patch defines it.
+        - openstack-tox-py35:
+            required-projects:
+              openstack/horizon
     gate:
       jobs:
         - openstack-tox-lower-constraints:
             required-projects:
               openstack/horizon
+        # TODO(amotoki): Drop this after project-config patch defines it.
+        - openstack-tox-py35:
+            required-projects:
+              openstack/horizon
 
 - job:
     name: manila-ui-dsvm

manila_ui/dashboards/admin/share_types/forms.py

@@ -96,7 +96,7 @@ class UpdateShareType(forms.SelfHandlingForm):
         # NOTE(vponomaryov): parse existing extra specs
         #                    to str view for textarea html element
         es_str = ""
-        for k, v in self.initial["extra_specs"].iteritems():
+        for k, v in self.initial["extra_specs"].items():
             es_str += "%s=%s\r\n" % (k, v)
         self.initial["extra_specs"] = es_str
 

manila_ui/dashboards/project/share_groups/forms.py

@@ -180,15 +180,15 @@ class CreateShareGroupForm(forms.SelfHandlingForm):
 
     def clean(self):
         cleaned_data = super(CreateShareGroupForm, self).clean()
-        errors = [k for k in self.errors.viewkeys()]
+        form_errors = list(self.errors)
 
-        for k in errors:
+        for error in form_errors:
-            sgt_name = k.split(self.st_field_name_prefix)[-1]
+            sgt_name = error.split(self.st_field_name_prefix)[-1]
             chosen_sgt = cleaned_data.get("sgt")
-            if (k.startswith(self.st_field_name_prefix) and
+            if (error.startswith(self.st_field_name_prefix) and
                     sgt_name != chosen_sgt):
-                cleaned_data[k] = "Not set"
+                cleaned_data[error] = "Not set"
-                self.errors.pop(k, None)
+                self.errors.pop(error, None)
 
         source_type = cleaned_data.get("source_type")
         if source_type != "snapshot":

manila_ui/dashboards/project/shares/forms.py

@@ -164,7 +164,7 @@ class CreateForm(forms.SelfHandlingForm):
                     del self.fields['snapshot']
             except Exception:
                 exceptions.handle(request, _("Unable to retrieve "
-                                  "share snapshots."))
+                                             "share snapshots."))
 
             if source_type_choices:
                 choices = ([('no_source_type',
@@ -176,17 +176,17 @@ class CreateForm(forms.SelfHandlingForm):
 
     def clean(self):
         cleaned_data = super(CreateForm, self).clean()
-        errors = [k for k in self.errors.viewkeys()]
+        form_errors = list(self.errors)
 
         # NOTE(vponomaryov): skip errors for share-network fields that are not
         # related to chosen share type.
-        for k in errors:
+        for error in form_errors:
-            st_name = k.split(self.sn_field_name_prefix)[-1]
+            st_name = error.split(self.sn_field_name_prefix)[-1]
             chosen_st = cleaned_data.get('share_type')
-            if (k.startswith(self.sn_field_name_prefix) and
+            if (error.startswith(self.sn_field_name_prefix) and
                     st_name != chosen_st):
-                cleaned_data[k] = 'Not set'
+                cleaned_data[error] = 'Not set'
-                self.errors.pop(k, None)
+                self.errors.pop(error, None)
 
         share_type = cleaned_data.get('share_type')
         if share_type:
@@ -299,7 +299,7 @@ class UpdateMetadataForm(forms.SelfHandlingForm):
     def __init__(self, *args, **kwargs):
         super(UpdateMetadataForm, self).__init__(*args, **kwargs)
         meta_str = ""
-        for k, v in self.initial["metadata"].iteritems():
+        for k, v in self.initial["metadata"].items():
             meta_str += "%s=%s\r\n" % (k, v)
         self.initial["metadata"] = meta_str
 

manila_ui/dashboards/utils.py

@@ -79,8 +79,7 @@ def metadata_to_str(metadata, meta_visible_limit=4, text_length_limit=25):
         return metadata
 
     meta = []
-    meta_keys = metadata.keys()
+    meta_keys = sorted(metadata.keys())
-    meta_keys.sort()
     meta_keys = meta_keys[:meta_visible_limit]
     for k in meta_keys:
         k_shortenned = k

manila_ui/tests/dashboards/admin/share_group_types/tests.py

@@ -12,9 +12,6 @@
 #    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 copy
-
 from django.core.urlresolvers import reverse
 import mock
 
@@ -29,49 +26,15 @@ class ShareGroupTypeTests(test.BaseAdminViewTests):
 
     def setUp(self):
         super(self.__class__, self).setUp()
-        self.sgt = copy.copy(test_data.share_group_type)
+        self.share_group_type = test_data.share_group_type
-        self.sgt_p = copy.copy(test_data.share_group_type_private)
+        self.share_group_type_alt = test_data.share_group_type_alt
-        self.url = reverse(
+        self.share_group_type_p = test_data.share_group_type_private
-            'horizon:admin:share_group_types:update', args=[self.sgt.id])
+        self.url = reverse('horizon:admin:share_group_types:update',
-        self.mock_object(
+                           args=[self.share_group_type.id])
-            api_manila, "share_group_type_get",
+        self.mock_object(api_manila, "share_group_type_get",
-            mock.Mock(return_value=self.sgt))
+                         mock.Mock(return_value=self.share_group_type))
 
-    def test_list_share_group_types_get(self):
+    def test_create_share_group_type(self):
-        sgts = [self.sgt, self.sgt_p]
-        self.mock_object(
-            api_manila, "share_group_type_list", mock.Mock(return_value=sgts))
-        self.mock_object(
-            api_manila, "share_type_list",
-            mock.Mock(return_value=[
-                test_data.share_type, test_data.share_type_private]))
-        self.mock_object(
-            api_manila, "share_group_type_get",
-            mock.Mock(side_effect=lambda req, sgt_id: (
-                self.sgt
-                if sgt_id in (self.sgt, self.sgt.id, self.sgt.name) else
-                self.sgt_p)))
-
-        res = self.client.get(INDEX_URL)
-
-        api_manila.share_group_type_list.assert_called_once_with(mock.ANY)
-        api_manila.share_type_list.assert_called_once_with(mock.ANY)
-        self.assertEqual(len(sgts), api_manila.share_group_type_get.call_count)
-        self.assertContains(res, "<h1>Share Group Types</h1>")
-        self.assertContains(res, 'href="/admin/share_group_types/create"')
-        self.assertContains(res, 'Delete Share Group Type</button>', len(sgts))
-        self.assertContains(res, 'Delete Share Group Types</button>', 1)
-        for sgt in (self.sgt, self.sgt_p):
-            for s in (
-                    'href="/admin/share_group_types/%s/update">' % sgt.id,
-                    'value="share_group_types__delete__%s"' % sgt.id):
-                self.assertContains(res, s, 1, 200)
-            self.assertContains(
-                res,
-                'href="/admin/share_group_types/%s/manage_access"' % sgt.id,
-                0 if sgt.is_public else 1)
-
-    def test_create_share_group_type_post(self):
         url = reverse('horizon:admin:share_group_types:create')
         data = {
             'method': u'CreateShareGroupTypeForm',
@@ -83,20 +46,23 @@ class ShareGroupTypeTests(test.BaseAdminViewTests):
         self.mock_object(
             api_manila, "share_group_type_create",
             mock.Mock(return_value=type(
-                'SGT', (object, ), {'id': 'sgt_id', 'name': 'sgt_name'})))
+                'ShareGroupType',
+                (object, ),
+                {'id': 'sgt_id', 'name': 'sgt_name'})))
         self.mock_object(api_manila, "share_group_type_set_specs")
         self.mock_object(
             api_manila, "share_type_list",
-            mock.Mock(return_value=[
+            mock.Mock(return_value=[type(
-                type('ST', (object, ), {'id': s_id, 'name': s_id + '_name'})
+                'ShareType',
+                (object, ),
+                {'id': s_id, 'name': s_id + '_name'})
                 for s_id in ('foo', 'bar')
             ]))
 
         res = self.client.post(url, data)
 
         api_manila.share_group_type_create.assert_called_once_with(
-            mock.ANY,
+            mock.ANY, data['name'],
-            data['name'],
             is_public=data['is_public'],
             share_types=['foo'])
         api_manila.share_type_list.assert_called_once_with(mock.ANY)
@@ -108,35 +74,36 @@ class ShareGroupTypeTests(test.BaseAdminViewTests):
         res = self.client.get(self.url)
 
         api_manila.share_group_type_get.assert_called_once_with(
-            mock.ANY, self.sgt.id)
+            mock.ANY, self.share_group_type.id)
         self.assertNoMessages()
         self.assertTemplateUsed(res, 'admin/share_group_types/update.html')
 
     def test_update_share_group_type_post(self):
-        form_data = {'group_specs': 'foo=bar'}
+        data = {'group_specs': 'foo=bar'}
-        data = {'group_specs': {'foo': 'bar'}}
+        form_data = {'group_specs': {'foo': 'bar'}}
         self.mock_object(api_manila, "share_group_type_set_specs")
 
-        res = self.client.post(self.url, form_data)
+        res = self.client.post(self.url, data)
 
         api_manila.share_group_type_set_specs.assert_called_once_with(
-            mock.ANY, self.sgt.id, data['group_specs'])
+            mock.ANY, self.share_group_type.id, form_data['group_specs'])
         self.assertRedirectsNoFollow(res, INDEX_URL)
 
     def test_delete_share_group_type(self):
-        data = {'action': 'share_group_types__delete__%s' % self.sgt.id}
+        data = {'action': 'share_group_types__delete__%s' %
+                self.share_group_type_alt.id}
         self.mock_object(api_manila, "share_group_type_delete")
         self.mock_object(
             api_manila, "share_group_type_list",
-            mock.Mock(return_value=[self.sgt]))
+            mock.Mock(return_value=[self.share_group_type_alt]))
         self.mock_object(
             api_manila, "share_type_list",
-            mock.Mock(return_value=[test_data.share_type]))
+            mock.Mock(return_value=[test_data.share_type_alt]))
 
         res = self.client.post(INDEX_URL, data)
 
         api_manila.share_group_type_delete.assert_called_once_with(
-            mock.ANY, self.sgt.id)
+            mock.ANY, self.share_group_type_alt.id)
         api_manila.share_group_type_list.assert_called_once_with(
             mock.ANY)
         self.assertRedirectsNoFollow(res, INDEX_URL)

manila_ui/tests/dashboards/admin/shares/replicas/tests.py

@@ -12,7 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import copy
 import ddt
 from django.core.urlresolvers import reverse
 import mock
@@ -140,7 +139,7 @@ class ReplicasTests(test.BaseAdminViewTests):
     @ddt.unpack
     def test_delete_not_allowed(self, replica_list, replica_id,
                                 replication_type):
-        share = copy.copy(self.share)
+        share = test_data.share
         share.replication_type = replication_type
         formData = {"action": "replicas__delete__%s" % replica_id}
         self.mock_object(api_manila, "share_replica_delete")
@@ -172,7 +171,7 @@ class ReplicasTests(test.BaseAdminViewTests):
     )
     @ddt.unpack
     def test_delete_allowed(self, replica_list, replica_id, replication_type):
-        share = copy.copy(self.share)
+        share = test_data.share
         share.replication_type = replication_type
         formData = {"action": "replicas__delete__%s" % replica_id}
         self.mock_object(api_manila, "share_replica_delete")

manila_ui/tests/dashboards/project/shares/replicas/tests.py

@@ -12,7 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import copy
 import ddt
 from django.core.urlresolvers import reverse
 import mock
@@ -205,7 +204,7 @@ class ReplicasTests(test.TestCase):
     @ddt.unpack
     def test_delete_not_allowed(self, replica_list, replica_id,
                                 replication_type):
-        share = copy.copy(self.share)
+        share = test_data.share
         share.replication_type = replication_type
         formData = {"action": "replicas__delete__%s" % replica_id}
         self.mock_object(api_manila, "share_replica_delete")
@@ -237,7 +236,7 @@ class ReplicasTests(test.TestCase):
     )
     @ddt.unpack
     def test_delete_allowed(self, replica_list, replica_id, replication_type):
-        share = copy.copy(self.share)
+        share = test_data.share
         share.replication_type = replication_type
         formData = {"action": "replicas__delete__%s" % replica_id}
         self.mock_object(api_manila, "share_replica_delete")

manila_ui/tests/dashboards/project/test_data.py

@@ -360,6 +360,17 @@ share_type_dhss_true = share_types.ShareType(
      'extra_specs': {'driver_handles_share_servers': True}}
 )
 
+share_type_alt = share_types.ShareType(
+    share_types.ShareTypeManager(FakeAPIClient),
+    {'id': 'share-type-id4',
+     'name': 'test-share-type4',
+     'share_type_access:is_public': True,
+     'extra_specs': {
+         'snapshot_support': True,
+         'driver_handles_share_servers': False}
+     }
+)
+
 share_group_type = share_group_types.ShareGroupType(
     share_group_types.ShareGroupTypeManager(FakeAPIClient),
     {'id': 'fake_share_group_type_id1',
@@ -387,6 +398,15 @@ share_group_type_dhss_true = share_group_types.ShareGroupType(
      'is_public': True}
 )
 
+share_group_type_alt = share_group_types.ShareGroupType(
+    share_group_types.ShareGroupTypeManager(FakeAPIClient),
+    {'id': 'fake_share_group_type_id4',
+     'name': 'fake_share_group_type_name',
+     'share_types': [share_type_alt.id],
+     'group_specs': {'k5': 'v5', 'k6': 'v6'},
+     'is_public': True}
+)
+
 share_group = share_groups.ShareGroup(
     share_groups.ShareGroupManager(FakeAPIClient),
     {'id': 'fake_share_group_id',

setup.cfg

@@ -16,7 +16,7 @@ classifier =
     Programming Language :: Python :: 2
     Programming Language :: Python :: 2.7
     Programming Language :: Python :: 3
-    Programming Language :: Python :: 3.4
+    Programming Language :: Python :: 3.5
 
 [files]
 packages =

tools/install_venv.py

@@ -1,154 +0,0 @@
-# Copyright 2012 United States Government as represented by the
-# Administrator of the National Aeronautics and Space Administration.
-# All Rights Reserved.
-#
-# Copyright 2012 OpenStack, LLC
-#
-# Copyright 2012 Nebula, Inc.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-Installation script for the OpenStack Dashboard development virtualenv.
-"""
-
-import os
-import subprocess
-import sys
-
-
-ROOT = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
-VENV = os.path.join(ROOT, '.venv')
-WITH_VENV = os.path.join(ROOT, 'tools', 'with_venv.sh')
-PIP_REQUIRES = os.path.join(ROOT, 'requirements.txt')
-TEST_REQUIRES = os.path.join(ROOT, 'test-requirements.txt')
-
-
-def die(message, *args):
-    print >> sys.stderr, message % args
-    sys.exit(1)
-
-
-def run_command(cmd, redirect_output=True, check_exit_code=True, cwd=ROOT,
-                die_message=None):
-    """
-    Runs a command in an out-of-process shell, returning the
-    output of that command.  Working directory is ROOT.
-    """
-    if redirect_output:
-        stdout = subprocess.PIPE
-    else:
-        stdout = None
-
-    proc = subprocess.Popen(cmd, cwd=cwd, stdout=stdout)
-    output = proc.communicate()[0]
-    if check_exit_code and proc.returncode != 0:
-        if die_message is None:
-            die('Command "%s" failed.\n%s', ' '.join(cmd), output)
-        else:
-            die(die_message)
-    return output
-
-
-HAS_EASY_INSTALL = bool(run_command(['which', 'easy_install'],
-                                    check_exit_code=False).strip())
-HAS_VIRTUALENV = bool(run_command(['which', 'virtualenv'],
-                                  check_exit_code=False).strip())
-
-
-def check_dependencies():
-    """Make sure virtualenv is in the path."""
-
-    print 'Checking dependencies...'
-    if not HAS_VIRTUALENV:
-        print 'Virtual environment not found.'
-        # Try installing it via easy_install...
-        if HAS_EASY_INSTALL:
-            print 'Installing virtualenv via easy_install...',
-            run_command(['easy_install', 'virtualenv'],
-                        die_message='easy_install failed to install virtualenv'
-                                    '\ndevelopment requires virtualenv, please'
-                                    ' install it using your favorite tool')
-            if not run_command(['which', 'virtualenv']):
-                die('ERROR: virtualenv not found in path.\n\ndevelopment '
-                    ' requires virtualenv, please install it using your'
-                    ' favorite package management tool and ensure'
-                    ' virtualenv is in your path')
-            print 'virtualenv installation done.'
-        else:
-            die('easy_install not found.\n\nInstall easy_install'
-                ' (python-setuptools in ubuntu) or virtualenv by hand,'
-                ' then rerun.')
-    print 'dependency check done.'
-
-
-def create_virtualenv(venv=VENV):
-    """Creates the virtual environment and installs PIP only into the
-    virtual environment
-    """
-    print 'Creating venv...',
-    run_command(['virtualenv', '-q', '--no-site-packages', VENV])
-    print 'done.'
-    print 'Installing pip in virtualenv...',
-    if not run_command([WITH_VENV, 'easy_install', 'pip']).strip():
-        die("Failed to install pip.")
-    print 'done.'
-    print 'Installing distribute in virtualenv...'
-    pip_install('distribute>=0.6.24')
-    print 'done.'
-
-
-def pip_install(*args):
-    args = [WITH_VENV, 'pip', 'install', '--upgrade'] + list(args)
-    run_command(args, redirect_output=False)
-
-
-def install_dependencies(venv=VENV):
-    print "Installing dependencies..."
-    print "(This may take several minutes, don't panic)"
-    pip_install('-r', TEST_REQUIRES)
-    pip_install('-r', PIP_REQUIRES)
-
-    # Tell the virtual env how to "import dashboard"
-    py = 'python%d.%d' % (sys.version_info[0], sys.version_info[1])
-    pthfile = os.path.join(venv, "lib", py, "site-packages", "dashboard.pth")
-    f = open(pthfile, 'w')
-    f.write("%s\n" % ROOT)
-
-
-def install_horizon():
-    print 'Installing horizon module in development mode...'
-    run_command([WITH_VENV, 'python', 'setup.py', 'develop'], cwd=ROOT)
-
-
-def print_summary():
-    summary = """
-Horizon development environment setup is complete.
-
-To activate the virtualenv for the extent of your current shell session you
-can run:
-
-$ source .venv/bin/activate
-"""
-    print summary
-
-
-def main():
-    check_dependencies()
-    create_virtualenv()
-    install_dependencies()
-    install_horizon()
-    print_summary()
-
-if __name__ == '__main__':
-    main()

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 minversion = 1.6
-envlist = py27,pep8,py27dj19,py27dj110
+envlist = py27,py35,pep8,py27dj19,py27dj110,py35dj20
 skipsdist = True
 
 [testenv]
@@ -15,6 +15,9 @@ commands = /bin/bash run_tests.sh -N --no-pep8 {posargs}
 [testenv:py27]
 setenv = DJANGO_SETTINGS_MODULE=manila_ui.tests.settings
 
+[testenv:py35]
+setenv = DJANGO_SETTINGS_MODULE=manila_ui.tests.settings
+
 [testenv:pep8]
 commands = flake8
 
@@ -37,6 +40,11 @@ basepython = python2.7
 commands = pip install django>=1.10,<1.11
            /bin/bash run_tests.sh -N --no-pep8 {posargs}
 
+[testenv:py35dj20]
+basepython = python3.5
+commands = pip install django>=2.0,<2.1
+           /bin/bash run_tests.sh -N --no-pep8 {posargs}
+
 [testenv:cover]
 commands = {toxinidir}/tools/cover.sh {posargs}