openstack
/
heat
Code Issues Proposed changes

Deprecate using required=True with default value 

The current schema of constraints allow to use default values for
required=True. However we never use it except in unittests, because
user should set it always, which looks like not required property.
This patch adds checking for constraint schema class with warning
message.

Change-Id: Ic6ace30742e887069f119a14ffb091f6bac9e682
This commit is contained in:
Sergey Kraynev 2014-08-25 09:39:44 -04:00
parent c7ed6f3ef7
commit 815d5b6ba5
3 changed files with 47 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
heat/engine/constraints.py
View File

@ -14,6 +14,7 @@
import collections
import numbers
import re
import warnings
from oslo_utils import strutils
import six
@ -84,6 +85,10 @@ class Schema(collections.Mapping):
raise exception.InvalidSchemaError(
message=_('Invalid type (%s)') % self.type)
if required and default is not None:
warnings.warn("Option 'required=True' should not be used with "
"any 'default' value ({0})".format(default))
self.description = description
self.required = required

46
heat/tests/test_constraints.py
View File

@ -11,7 +11,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import mock
import six
from heat.common import exception
@ -21,6 +21,26 @@ from heat.tests import common
class SchemaTest(common.HeatTestCase):
@mock.patch('warnings.warn')
def test_warn_required_with_default(self, mock_warn):
constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True)
msg = ("Option 'required=True' should not be used with any 'default' "
"value (wibble)")
mock_warn.assert_called_once_with(msg)
@mock.patch('warnings.warn')
def test_without_warn_only_default(self, mock_warn):
constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble')
self.assertEqual(0, mock_warn.call_count)
@mock.patch('warnings.warn')
def test_without_warn_only_required(self, mock_warn):
constraints.Schema(constraints.Schema.STRING, 'A string',
required=True)
self.assertEqual(0, mock_warn.call_count)
def test_range_schema(self):
d = {'range': {'min': 5, 'max': 10}, 'description': 'a range'}
r = constraints.Range(5, 10, description='a range')
@ -92,13 +112,13 @@ class SchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'constraints': [
{'length': {'min': 4, 'max': 8}},
]
}
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
self.assertEqual(d, dict(s))
@ -111,7 +131,7 @@ class SchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'constraints': [
{'length': {'min': 4, 'max': 8}},
]
@ -120,7 +140,7 @@ class SchemaTest(common.HeatTestCase):
'required': False,
}
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
l = constraints.Schema(constraints.Schema.LIST, 'A list', schema=s)
self.assertEqual(d, dict(l))
@ -134,7 +154,7 @@ class SchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'constraints': [
{'length': {'min': 4, 'max': 8}},
]
@ -143,7 +163,7 @@ class SchemaTest(common.HeatTestCase):
'required': False,
}
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
m = constraints.Schema(constraints.Schema.MAP, 'A map',
schema={'Foo': s})
@ -162,7 +182,7 @@ class SchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'constraints': [
{'length': {'min': 4, 'max': 8}},
]
@ -174,7 +194,7 @@ class SchemaTest(common.HeatTestCase):
'required': False,
}
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
m = constraints.Schema(constraints.Schema.MAP, 'A map',
schema={'Foo': s})
@ -231,13 +251,13 @@ class SchemaTest(common.HeatTestCase):
def test_schema_validate_good(self):
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
self.assertIsNone(s.validate())
def test_schema_validate_fail(self):
s = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Range(max=4)])
err = self.assertRaises(exception.InvalidSchemaError, s.validate)
self.assertIn('Range constraint invalid for String',
@ -245,7 +265,7 @@ class SchemaTest(common.HeatTestCase):
def test_schema_nested_validate_good(self):
nested = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
s = constraints.Schema(constraints.Schema.MAP, 'A map',
schema={'Foo': nested})
@ -253,7 +273,7 @@ class SchemaTest(common.HeatTestCase):
def test_schema_nested_validate_fail(self):
nested = constraints.Schema(constraints.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Range(max=4)])
s = constraints.Schema(constraints.Schema.MAP, 'A map',
schema={'Foo': nested})

22
heat/tests/test_properties.py
View File

@ -31,7 +31,7 @@ class PropertySchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'update_allowed': False,
'immutable': False,
'constraints': [
@ -39,7 +39,7 @@ class PropertySchemaTest(common.HeatTestCase):
]
}
s = properties.Schema(properties.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
self.assertEqual(d, dict(s))
@ -52,7 +52,7 @@ class PropertySchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'update_allowed': False,
'immutable': False,
'constraints': [
@ -65,7 +65,7 @@ class PropertySchemaTest(common.HeatTestCase):
'immutable': False,
}
s = properties.Schema(properties.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
l = properties.Schema(properties.Schema.LIST, 'A list', schema=s)
self.assertEqual(d, dict(l))
@ -79,7 +79,7 @@ class PropertySchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'update_allowed': False,
'immutable': False,
'constraints': [
@ -92,7 +92,7 @@ class PropertySchemaTest(common.HeatTestCase):
'immutable': False,
}
s = properties.Schema(properties.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
m = properties.Schema(properties.Schema.MAP, 'A map',
schema={'Foo': s})
@ -111,7 +111,7 @@ class PropertySchemaTest(common.HeatTestCase):
'type': 'string',
'description': 'A string',
'default': 'wibble',
'required': True,
'required': False,
'update_allowed': False,
'immutable': False,
'constraints': [
@ -129,7 +129,7 @@ class PropertySchemaTest(common.HeatTestCase):
'immutable': False,
}
s = properties.Schema(properties.Schema.STRING, 'A string',
default='wibble', required=True,
default='wibble',
constraints=[constraints.Length(4, 8)])
m = properties.Schema(properties.Schema.MAP, 'A map',
schema={'Foo': s})
@ -162,7 +162,6 @@ class PropertySchemaTest(common.HeatTestCase):
'Type': 'String',
'Description': 'a string',
'Default': 'wibble',
'Required': True,
'Implemented': False,
'MinLength': 4,
'MaxLength': 8,
@ -172,7 +171,7 @@ class PropertySchemaTest(common.HeatTestCase):
self.assertEqual(properties.Schema.STRING, s.type)
self.assertEqual('a string', s.description)
self.assertEqual('wibble', s.default)
self.assertTrue(s.required)
self.assertFalse(s.required)
self.assertEqual(3, len(s.constraints))
self.assertFalse(s.immutable)
@ -1618,7 +1617,6 @@ class PropertiesValidationTest(common.HeatTestCase):
nested_schema = {'Key': {'Type': 'String',
'Required': True},
'Value': {'Type': 'String',
'Required': True,
'Default': 'fewaf'}}
schema = {'foo': {'Type': 'Map', 'Schema': nested_schema}}
@ -1707,7 +1705,6 @@ class PropertiesValidationTest(common.HeatTestCase):
child_schema = {'Key': {'Type': 'String',
'Required': True},
'Value': {'Type': 'Boolean',
'Required': True,
'Default': True}}
list_schema = {'Type': 'Map', 'Schema': child_schema}
schema = {'foo': {'Type': 'List', 'Schema': list_schema}}
@ -1727,7 +1724,6 @@ class PropertiesValidationTest(common.HeatTestCase):
child_schema = {'Key': {'Type': 'String',
'Required': True},
'Value': {'Type': 'Boolean',
'Required': True,
'Default': True}}
map_schema = {'boo': {'Type': 'Map', 'Schema': child_schema}}
schema = {'foo': {'Type': 'Map', 'Schema': map_schema}}