openstack
/
gnocchi
Code Issues Proposed changes

allow required=True when patching resource type 

We have currently two limitations about resource type update.

* We cannot set required=True on a new attribute
* We cannot update a attribute

This change introduces the notion of 'options' for the description of
a new attribute or an attribute to update.

And implements the first item on top of this "option".

The option added is 'fill' to known to fill all row of existing
resources with a default value.
And 'fill' is required if 'required' is set to allow new
attribute with required=True.

Change-Id: If0bd609ed586b6fbe4fe7877ece237e55baa7d45
This commit is contained in:
Mehdi Abaakouk 2017-01-06 09:03:15 +01:00
parent 6f66b3fb87
commit 5f580161cb
8 changed files with 287 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
doc/source/rest.yaml
View File

@ -455,6 +455,11 @@
"path": "/attributes/awesome-stuff",
"value": {"type": "bool", "required": false}
},
{
"op": "add",
"path": "/attributes/required-stuff",
"value": {"type": "bool", "required": true, "options": {"fill": true}}
},
{
"op": "remove",
"path": "/attributes/prefix"

19
gnocchi/indexer/sqlalchemy.py
View File

@ -397,6 +397,7 @@ class SQLAlchemyIndexer(indexer.IndexerDriver):
try:
with self.facade.independent_writer() as session:
engine = session.connection()
rt = self._get_resource_type(session, name)
with self.facade.writer_connection() as connection:
@ -407,12 +408,22 @@ class SQLAlchemyIndexer(indexer.IndexerDriver):
for attr in del_attributes:
batch_op.drop_column(attr)
for attr in add_attributes:
# TODO(sileht): When attr.required is True, we
# have to pass a default. rest layer current
# protect us, requied = True is not yet allowed
server_default = attr.for_filling(
engine.dialect)
batch_op.add_column(sqlalchemy.Column(
attr.name, attr.satype,
nullable=not attr.required))
nullable=not attr.required,
server_default=server_default))
# We have all rows filled now, we can remove
# the server_default
if server_default is not None:
batch_op.alter_column(
column_name=attr.name,
existing_type=attr.satype,
existing_server_default=server_default,
existing_nullable=not attr.required,
server_default=None)
rt.state = "active"
rt.updated_at = utils.utcnow()

26
gnocchi/indexer/sqlalchemy_extension.py
View File

@ -20,19 +20,37 @@ import sqlalchemy_utils
from gnocchi import resource_type
class StringSchema(resource_type.StringSchema):
class SchemaMixin(object):
def for_filling(self, dialect):
# NOTE(sileht): This must be used only for patching resource type
# to fill all row with a default value and then switch back the
# server_default to None
if self.fill is None:
return None
# NOTE(sileht): server_default must be converted in sql element
return sqlalchemy.literal(self.fill)
class StringSchema(resource_type.StringSchema, SchemaMixin):
@property
def satype(self):
return sqlalchemy.String(self.max_length)
class UUIDSchema(resource_type.UUIDSchema):
class UUIDSchema(resource_type.UUIDSchema, SchemaMixin):
satype = sqlalchemy_utils.UUIDType()
def for_filling(self, dialect):
if self.fill is None:
return False # Don't set any server_default
return sqlalchemy.literal(
self.satype.process_bind_param(self.fill, dialect))
class NumberSchema(resource_type.NumberSchema):
class NumberSchema(resource_type.NumberSchema, SchemaMixin):
satype = sqlalchemy.Float(53)
class BoolSchema(resource_type.BoolSchema):
class BoolSchema(resource_type.BoolSchema, SchemaMixin):
satype = sqlalchemy.Boolean

60
gnocchi/resource_type.py
View File

@ -55,24 +55,63 @@ class InvalidResourceAttributeValue(InvalidResourceAttribute):
self.max = max
class InvalidResourceAttributeOption(InvalidResourceAttribute):
"""Error raised when the resource attribute name is invalid."""
def __init__(self, name, option, reason):
super(InvalidResourceAttributeOption, self).__init__(
"Option '%s' of resource attribute %s is invalid: %s" %
(option, str(name), str(reason)))
self.name = name
self.option = option
self.reason = reason
# NOTE(sileht): This is to store the behavior of some operations:
# * fill, to set a default value to all existing resource type
#
# in the future for example, we can allow to change the length of
# a string attribute, if the new one is shorter, we can add a option
# to define the behavior like:
# * resize = trunc or reject
OperationOptions = {
voluptuous.Optional('fill'): object
}
class CommonAttributeSchema(object):
meta_schema_ext = {}
schema_ext = None
def __init__(self, type, name, required):
def __init__(self, type, name, required, options=None):
if (len(name) > 63 or name in INVALID_NAMES
or not VALID_CHARS.match(name)):
raise InvalidResourceAttributeName(name)
self.name = name
self.required = required
self.fill = None
# options is set only when we update a resource type
if options is not None:
fill = options.get("fill")
if fill is None and required:
raise InvalidResourceAttributeOption(
name, "fill", "must not be empty if required=True")
elif fill is not None:
# Ensure fill have the correct attribute type
try:
self.fill = voluptuous.Schema(self.schema_ext)(fill)
except voluptuous.Error as e:
raise InvalidResourceAttributeOption(name, "fill", e)
@classmethod
def meta_schema(cls):
def meta_schema(cls, for_update=False):
d = {
voluptuous.Required('type'): cls.typename,
voluptuous.Required('required', default=True): bool
}
if for_update:
d[voluptuous.Required('options', default={})] = OperationOptions
if callable(cls.meta_schema_ext):
d.update(cls.meta_schema_ext())
else:
@ -94,12 +133,12 @@ class StringSchema(CommonAttributeSchema):
typename = "string"
def __init__(self, min_length, max_length, *args, **kwargs):
super(StringSchema, self).__init__(*args, **kwargs)
if min_length > max_length:
raise InvalidResourceAttributeValue(min_length, max_length)
self.min_length = min_length
self.max_length = max_length
super(StringSchema, self).__init__(*args, **kwargs)
meta_schema_ext = {
voluptuous.Required('min_length', default=0):
@ -131,12 +170,11 @@ class NumberSchema(CommonAttributeSchema):
typename = "number"
def __init__(self, min, max, *args, **kwargs):
super(NumberSchema, self).__init__(*args, **kwargs)
if max is not None and min is not None and min > max:
raise InvalidResourceAttributeValue(min, max)
self.min = min
self.max = max
super(NumberSchema, self).__init__(*args, **kwargs)
meta_schema_ext = {
voluptuous.Required('min', default=None): voluptuous.Any(
@ -182,9 +220,21 @@ class ResourceTypeSchemaManager(stevedore.ExtensionManager):
}
})
type_schemas = tuple([ext.plugin.meta_schema(for_update=True)
for ext in self.extensions])
self._schema_for_update = voluptuous.Schema({
"name": six.text_type,
voluptuous.Required("attributes", default={}): {
six.text_type: voluptuous.Any(*tuple(type_schemas))
}
})
def __call__(self, definition):
return self._schema(definition)
def for_update(self, definition):
return self._schema_for_update(definition)
def attributes_from_dict(self, attributes):
return ResourceTypeAttributes(
self[attr["type"]].plugin(name=name, **attr)

10
gnocchi/rest/__init__.py
View File

@ -755,7 +755,7 @@ class ResourceTypeController(rest.RestController):
# Validate that the whole new resource_type is valid
schema = pecan.request.indexer.get_resource_type_schema()
try:
rt_json_next = voluptuous.Schema(schema, required=True)(
rt_json_next = voluptuous.Schema(schema.for_update, required=True)(
rt_json_next)
except voluptuous.Error as e:
abort(400, "Invalid input: %s" % e)
@ -776,14 +776,6 @@ class ResourceTypeController(rest.RestController):
except resource_type.InvalidResourceAttribute as e:
abort(400, "Invalid input: %s" % e)
# TODO(sileht): Add a default field on an attribute
# to be able to fill non-nullable column on sql side.
# And obviousy remove this limitation
for attr in add_attrs:
if attr.required:
abort(400, ValueError("Adding required attributes is not yet "
"possible."))
try:
return pecan.request.indexer.update_resource_type(
self._name, add_attributes=add_attrs,

177
gnocchi/tests/gabbi/gabbits/resource-type.yaml
View File

@ -348,6 +348,47 @@ tests:
required: False
min_length: 0
max_length: 255
- op: add
path: /attributes/newfilled
value:
type: string
required: False
min_length: 0
max_length: 255
options:
fill: "filled"
- op: add
path: /attributes/newbool
value:
type: bool
required: True
options:
fill: True
- op: add
path: /attributes/newint
value:
type: number
required: True
min: 0
max: 255
options:
fill: 15
- op: add
path: /attributes/newstring
value:
type: string
required: True
min_length: 0
max_length: 255
options:
fill: "foobar"
- op: add
path: /attributes/newuuid
value:
type: uuid
required: True
options:
fill: "00000000-0000-0000-0000-000000000000"
- op: remove
path: /attributes/foobar
status: 200
@ -385,6 +426,62 @@ tests:
required: False
min_length: 0
max_length: 255
newfilled:
type: string
required: False
min_length: 0
max_length: 255
newstring:
type: string
required: True
min_length: 0
max_length: 255
newbool:
type: bool
required: True
newint:
type: number
required: True
min: 0
max: 255
newuuid:
type: uuid
required: True
- name: post a new resource attribute with missing fill
url: /v1/resource_type/my_custom_resource
method: patch
request_headers:
x-roles: admin
content-type: application/json-patch+json
data:
- op: add
path: /attributes/missing
value:
type: bool
required: True
options: {}
status: 400
response_strings:
- "Invalid input: Option 'fill' of resource attribute missing is invalid: must not be empty if required=True"
- name: post a new resource attribute with incorrect fill
url: /v1/resource_type/my_custom_resource
method: patch
request_headers:
x-roles: admin
content-type: application/json-patch+json
data:
- op: add
path: /attributes/incorrect
value:
type: number
required: True
options:
fill: "a-string"
status: 400
response_strings:
- "Invalid input: Option 'fill' of resource attribute incorrect is invalid: expected Real"
- name: get the new custom resource type
url: /v1/resource_type/my_custom_resource
@ -422,6 +519,65 @@ tests:
required: False
min_length: 0
max_length: 255
newfilled:
type: string
required: False
min_length: 0
max_length: 255
newstring:
type: string
required: True
min_length: 0
max_length: 255
newbool:
type: bool
required: True
newint:
type: number
required: True
min: 0
max: 255
newuuid:
type: uuid
required: True
- name: control new attributes of existing resource
GET: /v1/resource/my_custom_resource/d11edfca-4393-4fda-b94d-b05a3a1b3747
request_headers:
content-type: application/json
status: 200
response_json_paths:
$.id: d11edfca-4393-4fda-b94d-b05a3a1b3747
$.name: foo
$.newstuff: null
$.newfilled: "filled"
$.newbool: true
$.newint: 15
$.newstring: foobar
$.newuuid: "00000000-0000-0000-0000-000000000000"
- name: control new attributes of existing resource history
GET: /v1/resource/my_custom_resource/d11edfca-4393-4fda-b94d-b05a3a1b3747/history?sort=revision_end:asc-nullslast
request_headers:
content-type: application/json
response_json_paths:
$.`len`: 2
$[0].id: d11edfca-4393-4fda-b94d-b05a3a1b3747
$[0].name: bar
$[0].newstuff: null
$[0].newfilled: "filled"
$[0].newbool: true
$[0].newint: 15
$[0].newstring: foobar
$[0].newuuid: "00000000-0000-0000-0000-000000000000"
$[1].id: d11edfca-4393-4fda-b94d-b05a3a1b3747
$[1].name: foo
$[1].newstuff: null
$[1].newfilled: "filled"
$[1].newbool: true
$[1].newint: 15
$[1].newstring: foobar
$[1].newuuid: "00000000-0000-0000-0000-000000000000"
# Invalid patch
@ -476,6 +632,27 @@ tests:
required: False
min_length: 0
max_length: 255
newfilled:
type: string
required: False
min_length: 0
max_length: 255
newstring:
type: string
required: True
min_length: 0
max_length: 255
newbool:
type: bool
required: True
newint:
type: number
required: True
min: 0
max: 255
newuuid:
type: uuid
required: True
- name: delete/add the same resource attribute
url: /v1/resource_type/my_custom_resource

7
gnocchi/tests/test_indexer.py
View File

@ -1120,7 +1120,9 @@ class TestIndexerDriver(tests_base.TestCase):
# Update the resource type
add_attrs = mgr.resource_type_from_dict("indexer_test", {
"col2": {"type": "number", "required": False,
"max": 100, "min": 0}
"max": 100, "min": 0},
"col3": {"type": "number", "required": True,
"max": 100, "min": 0, "options": {'fill': 15}}
}, "creating").attributes
self.index.update_resource_type("indexer_test",
add_attributes=add_attrs)
@ -1128,6 +1130,7 @@ class TestIndexerDriver(tests_base.TestCase):
# Check the new attribute
r = self.index.get_resource("indexer_test", rid)
self.assertIsNone(r.col2)
self.assertEqual(15, r.col3)
self.index.update_resource("indexer_test", rid, col2=10)
@ -1139,6 +1142,8 @@ class TestIndexerDriver(tests_base.TestCase):
self.assertEqual(2, len(rl))
self.assertIsNone(rl[0].col2)
self.assertEqual(10, rl[1].col2)
self.assertEqual(15, rl[0].col3)
self.assertEqual(15, rl[1].col3)
# Deletion
self.assertRaises(indexer.ResourceTypeInUse,

6
releasenotes/notes/resource-type-required-attributes-f446c220d54c8eb7.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- When updating a resource attribute, it's now possible to pass the option
'fill' for each attribute to fill existing resources.
- required=True is now supported when updating resource type. This requires
the option 'fill' to be set.