Refactored Watcher codebase to add py34 support

Even though Watcher was mentioning python 3.4 as supported, it
really wasn't the case as all the unit tests were not passing in this
version of Python.

This patchset fixes all the failing tests in Python 3.4 while
keeping Watcher Python 2.7 compatible.

DocImpact
BugImpact
Change-Id: Ie74acc08ef0a2899349a4b419728c89e416a18cb
This commit is contained in:
Vincent Françoise
2015-12-08 11:06:52 +01:00
committed by Jean-Emile DARTOIS
parent b1fe7a5f3d
commit d934971458
21 changed files with 134 additions and 57 deletions

View File

@@ -199,7 +199,7 @@ class ActionCollection(collection.Collection):
reverse = True if kwargs['sort_dir'] == 'desc' else False
collection.actions = sorted(
collection.actions,
key=lambda action: action.next_uuid,
key=lambda action: action.next_uuid or '',
reverse=reverse)
collection.next = collection.get_next(limit, url=url, **kwargs)
@@ -229,7 +229,6 @@ class ActionsController(rest.RestController):
sort_key, sort_dir, expand=False,
resource_url=None,
action_plan_uuid=None, audit_uuid=None):
limit = api_utils.validate_limit(limit)
sort_dir = api_utils.validate_sort_dir(sort_dir)

View File

@@ -28,10 +28,18 @@ JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchException,
def validate_limit(limit):
if limit is not None and limit <= 0:
if limit is None:
return CONF.api.max_limit
if limit <= 0:
# Case where we don't a valid limit value
raise wsme.exc.ClientSideError(_("Limit must be positive"))
return min(CONF.api.max_limit, limit) or CONF.api.max_limit
if limit and not CONF.api.max_limit:
# Case where we don't have an upper limit
return limit
return min(CONF.api.max_limit, limit)
def validate_sort_dir(sort_dir):

View File

@@ -13,7 +13,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.
"""
Middleware to replace the plain text message body of an error
response with one formatted so the client can parse it.
@@ -24,9 +23,9 @@ Based on pecan.middleware.errordocument
import json
from xml import etree as et
import webob
from oslo_log import log
import six
import webob
from watcher.common.i18n import _
from watcher.common.i18n import _LE
@@ -69,8 +68,9 @@ class ParsableErrorMiddleware(object):
app_iter = self.app(environ, replacement_start_response)
if (state['status_code'] // 100) not in (2, 3):
req = webob.Request(environ)
if (req.accept.best_match(['application/json', 'application/xml'])
== 'application/xml'):
if (req.accept.best_match(['application/json', 'application/xml']
) == 'application/xml'
):
try:
# simple check xml is valid
body = [et.ElementTree.tostring(
@@ -83,9 +83,13 @@ class ParsableErrorMiddleware(object):
+ '</error_message>']
state['headers'].append(('Content-Type', 'application/xml'))
else:
if six.PY3:
app_iter = [i.decode('utf-8') for i in app_iter]
body = [json.dumps({'error_message': '\n'.join(app_iter)})]
if six.PY3:
body = [item.encode('utf-8') for item in body]
state['headers'].append(('Content-Type', 'application/json'))
state['headers'].append(('Content-Length', len(body[0])))
state['headers'].append(('Content-Length', str(len(body[0]))))
else:
body = app_iter
return body

View File

@@ -44,7 +44,7 @@ CONF.register_opts(exc_log_opts)
def _cleanse_dict(original):
"""Strip all admin_password, new_pass, rescue_pass keys from a dict."""
return dict((k, v) for k, v in original.iteritems() if "_pass" not in k)
return dict((k, v) for k, v in six.iteritems(original) if "_pass" not in k)
class WatcherException(Exception):
@@ -77,7 +77,7 @@ class WatcherException(Exception):
# kwargs doesn't match a variable in the message
# log the issue and the kwargs
LOG.exception(_LE('Exception in string format operation'))
for name, value in kwargs.iteritems():
for name, value in six.iteritems(kwargs):
LOG.error("%s: %s" % (name, value))
if CONF.fatal_exception_format_errors:
@@ -231,7 +231,7 @@ class PatchError(Invalid):
class BaseException(Exception):
def __init__(self, desc=""):
if (not isinstance(desc, basestring)):
if (not isinstance(desc, six.string_types)):
raise IllegalArgumentException(
"Description must be an instance of str")

View File

@@ -20,8 +20,8 @@
from oslo_config import cfg
from oslo_log import log
from urlparse import urljoin
from urlparse import urlparse
from six.moves.urllib.parse import urljoin
from six.moves.urllib.parse import urlparse
from keystoneclient.auth.identity import generic
from keystoneclient import session as keystone_session

View File

@@ -11,6 +11,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import six
from tempest_lib import exceptions as lib_exc
from tempest.api.infra_optim.admin import base
@@ -27,7 +29,7 @@ class TestAuditTemplate(base.BaseInfraOptimTest):
def _assertExpected(self, expected, actual):
# Check if not expected keys/values exists in actual response body
for key, value in expected.iteritems():
for key, value in six.iteritems(expected):
if key not in ('created_at', 'updated_at', 'deleted_at'):
self.assertIn(key, actual)
self.assertEqual(value, actual[key])

View File

@@ -313,7 +313,7 @@ class BasicConsolidation(BaseStrategy):
cpu_capacity = model.get_resource_from_id(
ResourceType.cpu_cores).get_capacity(vm)
total_cores_used = cpu_capacity * (vm_cpu_utilization / 100)
total_cores_used = cpu_capacity * (vm_cpu_utilization / 100.0)
return self.calculate_weight(model, vm, total_cores_used,
0,

View File

@@ -53,7 +53,7 @@ def make_class_properties(cls):
for name, field in supercls.fields.items():
if name not in cls.fields:
cls.fields[name] = field
for name, typefn in cls.fields.iteritems():
for name, typefn in six.iteritems(cls.fields):
def getter(self, name=name):
attrname = get_attrname(name)
@@ -351,12 +351,13 @@ class WatcherObject(object):
NOTE(danms): May be removed in the future.
"""
for name in self.fields.keys() + self.obj_extra_fields:
for name in list(self.fields.keys()) + self.obj_extra_fields:
if (hasattr(self, get_attrname(name)) or
name in self.obj_extra_fields):
yield name, getattr(self, name)
items = lambda self: list(self.iteritems())
def items(self):
return list(self.iteritems())
def __getitem__(self, name):
"""For backwards-compatibility with dict-based objects.
@@ -540,7 +541,7 @@ def obj_to_primitive(obj):
return [obj_to_primitive(x) for x in obj]
elif isinstance(obj, WatcherObject):
result = {}
for key, value in obj.iteritems():
for key, value in six.iteritems(obj):
result[key] = obj_to_primitive(value)
return result
else:

View File

@@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import unicode_literals
import json
import mock
@@ -68,14 +70,14 @@ class TestContextHook(base.BaseTestCase):
class TestNoExceptionTracebackHook(api_base.FunctionalTest):
TRACE = [
u'Traceback (most recent call last):',
u' File "/opt/stack/watcher/watcher/openstack/common/rpc/amqp.py",'
'Traceback (most recent call last):',
' File "/opt/stack/watcher/watcher/openstack/common/rpc/amqp.py",'
' line 434, in _process_data\\n **args)',
u' File "/opt/stack/watcher/watcher/openstack/common/rpc/'
' File "/opt/stack/watcher/watcher/openstack/common/rpc/'
'dispatcher.py", line 172, in dispatch\\n result ='
' getattr(proxyobj, method)(context, **kwargs)']
MSG_WITHOUT_TRACE = "Test exception message."
MSG_WITH_TRACE = MSG_WITHOUT_TRACE + "\n" + "\n".join(TRACE)
MSG_WITH_TRACE = "{0}\n{1}".format(MSG_WITHOUT_TRACE, "\n".join(TRACE))
def setUp(self):
super(TestNoExceptionTracebackHook, self).setUp()
@@ -94,7 +96,7 @@ class TestNoExceptionTracebackHook(api_base.FunctionalTest):
def test_hook_remote_error_success(self):
test_exc_type = 'TestException'
self.root_convert_mock.side_effect = messaging.rpc.RemoteError(
test_exc_type, self.MSG_WITHOUT_TRACE, self.TRACE)
test_exc_type, self.MSG_WITHOUT_TRACE, "\n".join(self.TRACE))
response = self.get_json('/', path_prefix='', expect_errors=True)
@@ -104,7 +106,7 @@ class TestNoExceptionTracebackHook(api_base.FunctionalTest):
# rare thing (happens due to wrong deserialization settings etc.)
# we don't care about this garbage.
expected_msg = ("Remote error: %s %s"
% (test_exc_type, self.MSG_WITHOUT_TRACE) + "\n[u'")
% (test_exc_type, self.MSG_WITHOUT_TRACE))
actual_msg = json.loads(response.json['error_message'])['faultstring']
self.assertEqual(expected_msg, actual_msg)

View File

@@ -0,0 +1,59 @@
# -*- encoding: utf-8 -*-
# Copyright (c) 2015 b<>com
#
# 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.
from __future__ import unicode_literals
from oslo_config import cfg
import wsme
from watcher.api.controllers.v1 import utils as v1_utils
from watcher.tests import base
class TestApiUtilsValidScenarios(base.TestCase):
scenarios = [
("limit=None + max_limit=None",
{"limit": None, "max_limit": None, "expected": None}),
("limit=None + max_limit=1",
{"limit": None, "max_limit": 1, "expected": 1}),
# ("limit=0 + max_limit=None",
# {"limit": 0, "max_limit": None, "expected": 0}),
("limit=1 + max_limit=None",
{"limit": 1, "max_limit": None, "expected": 1}),
("limit=1 + max_limit=1",
{"limit": 1, "max_limit": 1, "expected": 1}),
("limit=2 + max_limit=1",
{"limit": 2, "max_limit": 1, "expected": 1}),
]
def test_validate_limit(self):
cfg.CONF.set_override("max_limit", self.max_limit, group="api")
actual_limit = v1_utils.validate_limit(self.limit)
self.assertEqual(actual_limit, self.expected)
class TestApiUtilsInvalidScenarios(base.TestCase):
scenarios = [
("limit=0 + max_limit=None", {"limit": 0, "max_limit": None}),
]
def test_validate_limit_invalid_cases(self):
cfg.CONF.set_override("max_limit", self.max_limit, group="api")
self.assertRaises(
wsme.exc.ClientSideError, v1_utils.validate_limit, self.limit
)

View File

@@ -18,6 +18,7 @@ Utils for testing the API service.
import datetime
import json
import six
from watcher.api.controllers.v1 import action as action_ctrl
from watcher.api.controllers.v1 import action_plan as action_plan_ctrl
from watcher.api.controllers.v1 import audit as audit_ctrl
@@ -76,7 +77,9 @@ class FakeMemcache(object):
def remove_internal(values, internal):
# NOTE(yuriyz): internal attributes should not be posted, except uuid
int_attr = [attr.lstrip('/') for attr in internal if attr != '/uuid']
return dict([(k, v) for (k, v) in values.iteritems() if k not in int_attr])
return dict(
(k, v) for (k, v) in six.iteritems(values) if k not in int_attr
)
def audit_post_data(**kw):

View File

@@ -367,20 +367,21 @@ class TestListAction(api_base.FunctionalTest):
uuid=utils.generate_uuid(),
next=id_ + 1)
response = self.get_json('/actions/')
reference_uuids = [(s['next_uuid'] if 'next_uuid' in s else None)
for s in response['actions']]
reference_uuids = [
s.get('next_uuid', '') for s in response['actions']
]
response = self.get_json('/actions/?sort_key=next_uuid')
self.assertEqual(5, len(response['actions']))
uuids = [(s['next_uuid'] if 'next_uuid' in s else None)
uuids = [(s['next_uuid'] if 'next_uuid' in s else '')
for s in response['actions']]
self.assertEqual(sorted(reference_uuids), uuids)
response = self.get_json('/actions/?sort_key=next_uuid&sort_dir=desc')
self.assertEqual(5, len(response['actions']))
uuids = [(s['next_uuid'] if 'next_uuid' in s else None)
uuids = [(s['next_uuid'] if 'next_uuid' in s else '')
for s in response['actions']]
self.assertEqual(sorted(reference_uuids, reverse=True), uuids)

View File

@@ -38,19 +38,19 @@ class TestListGoal(api_base.FunctionalTest):
self._assert_goal_fields(response['goals'][0])
def test_get_one(self):
goal_name = CONF.watcher_goals.goals.keys()[0]
goal_name = list(CONF.watcher_goals.goals.keys())[0]
response = self.get_json('/goals/%s' % goal_name)
self.assertEqual(goal_name, response['name'])
self._assert_goal_fields(response)
def test_detail(self):
goal_name = CONF.watcher_goals.goals.keys()[0]
goal_name = list(CONF.watcher_goals.goals.keys())[0]
response = self.get_json('/goals/detail')
self.assertEqual(goal_name, response['goals'][0]["name"])
self._assert_goal_fields(response['goals'][0])
def test_detail_against_single(self):
goal_name = CONF.watcher_goals.goals.keys()[0]
goal_name = list(CONF.watcher_goals.goals.keys())[0]
response = self.get_json('/goals/%s/detail' % goal_name,
expect_errors=True)
self.assertEqual(404, response.status_int)

View File

@@ -123,7 +123,7 @@ class TestJsonPatchType(base.TestCase):
'value': {'cat': 'meow'}}]
ret = self._patch_json(valid_patches, False)
self.assertEqual(200, ret.status_int)
self.assertEqual(sorted(valid_patches), sorted(ret.json))
self.assertEqual(valid_patches, ret.json)
def test_cannot_update_internal_attr(self):
patch = [{'path': '/internal', 'op': 'replace', 'value': 'foo'}]
@@ -244,7 +244,6 @@ class TestJsonType(base.TestCase):
vts = str(types.jsontype)
self.assertIn(str(wtypes.text), vts)
self.assertIn(str(int), vts)
self.assertIn(str(long), vts)
self.assertIn(str(float), vts)
self.assertIn(str(types.BooleanType), vts)
self.assertIn(str(list), vts)

View File

@@ -24,6 +24,7 @@ from oslo_log import log
from oslotest import base
import pecan
from pecan import testing
import six
import testscenarios
from watcher.common import context as watcher_context
@@ -101,7 +102,7 @@ class TestCase(BaseTestCase):
def config(self, **kw):
"""Override config options for a test."""
group = kw.pop('group', None)
for k, v in kw.iteritems():
for k, v in six.iteritems(kw):
CONF.set_override(k, v, group)
def path_get(self, project_file=None):

View File

@@ -16,7 +16,7 @@
from __future__ import absolute_import
from __future__ import unicode_literals
from SocketServer import BaseServer
from six.moves.socketserver import BaseServer
import types

View File

@@ -44,7 +44,7 @@ class TestDBManageRunApp(TestCase):
cfg.CONF.register_opt(cfg.Opt("func"), group="command")
cfg.CONF.set_override("func", m_func, group="command")
# Only append if the command is not None
m_sys.argv = filter(None, ["watcher-db-manage", self.command])
m_sys.argv = list(filter(None, ["watcher-db-manage", self.command]))
dbmanage.main()
self.assertEqual(m_func.call_count, 1)

View File

@@ -14,8 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import exceptions
from watcher.decision_engine.event.consumer_factory import EventConsumerFactory
from watcher.decision_engine.messaging.events import Events
from watcher.tests import base
@@ -26,6 +24,6 @@ class TestEventConsumerFactory(base.TestCase):
event_consumer_factory = EventConsumerFactory()
def test_factory_with_unknown_type(self):
self.assertRaises(exceptions.AssertionError,
self.assertRaises(AssertionError,
self.event_consumer_factory.factory,
Events.ALL)

View File

@@ -30,7 +30,7 @@ class TestMapping(base.BaseTestCase):
model = fake_cluster.generate_scenario_4_with_2_hypervisors()
vms = model.get_all_vms()
keys = vms.keys()
keys = list(vms.keys())
vm = vms[keys[0]]
if vm.uuid != 'VM_0':
vm = vms[keys[1]]
@@ -68,7 +68,7 @@ class TestMapping(base.BaseTestCase):
fake_cluster = FakerModelCollector()
model = fake_cluster.generate_scenario_4_with_2_hypervisors()
vms = model.get_all_vms()
keys = vms.keys()
keys = list(vms.keys())
vm0 = vms[keys[0]]
hyp0 = model.mapping.get_node_from_vm_id(vm0.uuid)
vm1 = vms[keys[1]]
@@ -83,7 +83,7 @@ class TestMapping(base.BaseTestCase):
fake_cluster = FakerModelCollector()
model = fake_cluster.generate_scenario_4_with_2_hypervisors()
vms = model.get_all_vms()
keys = vms.keys()
keys = list(vms.keys())
vm0 = vms[keys[0]]
id = "{0}".format(uuid.uuid4())
hypervisor = Hypervisor()
@@ -97,7 +97,7 @@ class TestMapping(base.BaseTestCase):
fake_cluster = FakerModelCollector()
model = fake_cluster.generate_scenario_4_with_2_hypervisors()
vms = model.get_all_vms()
keys = vms.keys()
keys = list(vms.keys())
vm0 = vms[keys[0]]
hyp0 = model.mapping.get_node_from_vm_id(vm0.uuid)

View File

@@ -79,21 +79,21 @@ class TestBasicConsolidation(base.BaseTestCase):
sercon.ceilometer = MagicMock(
statistic_aggregation=self.fake_metrics.mock_get_statistics)
vm_0 = cluster.get_vm_from_id("VM_0")
vm_0_score = 0.0
vm_0_score = 0.023333333333333317
self.assertEqual(sercon.calculate_score_vm(vm_0, cluster), vm_0_score)
vm_1 = cluster.get_vm_from_id("VM_1")
vm_1_score = 0.0
vm_1_score = 0.023333333333333317
self.assertEqual(sercon.calculate_score_vm(vm_1, cluster),
vm_1_score)
vm_2 = cluster.get_vm_from_id("VM_2")
vm_2_score = 0.0
vm_2_score = 0.033333333333333326
self.assertEqual(sercon.calculate_score_vm(vm_2, cluster), vm_2_score)
vm_6 = cluster.get_vm_from_id("VM_6")
vm_6_score = 0.0
vm_6_score = 0.02666666666666669
self.assertEqual(sercon.calculate_score_vm(vm_6, cluster), vm_6_score)
vm_7 = cluster.get_vm_from_id("VM_7")
vm_7_score = 0.0
vm_7_score = 0.013333333333333345
self.assertEqual(sercon.calculate_score_vm(vm_7, cluster), vm_7_score)
def test_basic_consolidation_score_vm_disk(self):
@@ -102,7 +102,7 @@ class TestBasicConsolidation(base.BaseTestCase):
sercon.ceilometer = MagicMock(
statistic_aggregation=self.fake_metrics.mock_get_statistics)
vm_0 = cluster.get_vm_from_id("VM_0")
vm_0_score = 0.0
vm_0_score = 0.023333333333333355
self.assertEqual(sercon.calculate_score_vm(vm_0, cluster), vm_0_score)
def test_basic_consolidation_weight(self):
@@ -158,8 +158,8 @@ class TestBasicConsolidation(base.BaseTestCase):
all_vms = model.get_all_vms()
all_hyps = model.get_all_hypervisors()
vm0 = all_vms[all_vms.keys()[0]]
hyp0 = all_hyps[all_hyps.keys()[0]]
vm0 = all_vms[list(all_vms.keys())[0]]
hyp0 = all_hyps[list(all_hyps.keys())[0]]
sercon.check_migration(model, hyp0, hyp0, vm0)
@@ -169,7 +169,7 @@ class TestBasicConsolidation(base.BaseTestCase):
model = fake_cluster.generate_scenario_4_with_2_hypervisors()
all_hyps = model.get_all_hypervisors()
hyp0 = all_hyps[all_hyps.keys()[0]]
hyp0 = all_hyps[list(all_hyps.keys())[0]]
sercon.check_threshold(model, hyp0, 1000, 1000, 1000)

View File

@@ -266,7 +266,7 @@ class _TestObject(object):
self.assertEqual([('bar', 'bar'), ('foo', 123)],
sorted(obj.items(), key=lambda x: x[0]))
self.assertEqual([('bar', 'bar'), ('foo', 123)],
sorted(list(obj.iteritems()), key=lambda x: x[0]))
sorted(list(obj.items()), key=lambda x: x[0]))
def test_load(self):
obj = MyObj(self.context)