On 11/30/15 at 07:32am, Sean Dague wrote:
On 11/24/2015 10:09 AM, John Garbutt wrote:
On 24 November 2015 at 15:00, Balázs Gibizer
<balazs.gibi...@ericsson.com> wrote:
From: Andrew Laski [mailto:and...@lascii.com]
Sent: November 24, 2015 15:35
On 11/24/15 at 10:26am, Balázs Gibizer wrote:
From: Ryan Rossiter [mailto:rlros...@linux.vnet.ibm.com]
Sent: November 23, 2015 22:33
On 11/23/2015 2:23 PM, Andrew Laski wrote:
On 11/23/15 at 04:43pm, Balázs Gibizer wrote:
From: Andrew Laski [mailto:and...@lascii.com]
Sent: November 23, 2015 17:03
On 11/23/15 at 08:54am, Ryan Rossiter wrote:
On 11/23/2015 5:33 AM, John Garbutt wrote:
On 20 November 2015 at 09:37, Balázs Gibizer
<balazs.gibi...@ericsson.com> wrote:
<snip>
<snip>
There is a bit I am conflicted/worried about, and thats when we
start including verbatim, DB objects into the notifications. At
least you can now quickly detect if that blob is something
compatible with your current parsing code. My preference is
really to keep the Notifications as a totally separate object
tree, but I am sure there are many cases where that ends up
being seemingly stupid duplicate work. I am not expressing this
well in text form :(
Are you saying we don't want to be willy-nilly tossing DB
objects across the wire? Yeah that was part of the rug-pulling
of just having the payload contain an object. We're
automatically tossing everything with the object then, whether
or not some of that was supposed to be a secret. We could add
some sort of property to the field like
dont_put_me_on_the_wire=True (or I guess a
notification_ready() function that helps an object sanitize
itself?) that the notifications will look at to know if it puts
that on the wire-serialized dict, but that's adding a lot more
complexity and work to a pile that's already growing rapidly.
I don't want to be tossing db objects across the wire. But I
also am not convinced that we should be tossing the current
objects over the wire either.
You make the point that there may be things in the object that
shouldn't be exposed, and I think object version bumps is another
thing to watch out for.
So far the only object that has been bumped is Instance but in
doing so no notifications needed to change. I think if we just
put objects into notifications we're coupling the notification
versions to db or RPC changes unnecessarily. Some times they'll
move together but other times, like moving flavor into
instance_extra, there's no reason to bump notifications.
Sanitizing existing versioned objects before putting them to the
wire is not hard to do.
You can see an example of doing it in
https://review.openstack.org/#/c/245678/8/nova/objects/service.py,
cm
L382.
We don't need extra effort to take care of minor version bumps
because that does not break a well written consumer. We do have to
take care of the major version bumps but that is a rare event and
therefore can be handled one by one in a way John suggested, by
keep sending the previous major version for a while too.
That review is doing much of what I was suggesting. There is a
separate notification and payload object. The issue I have is that
within the ServiceStatusPayload the raw Service object and version
is being dumped, with the filter you point out. But I don't think
that consumers really care about tracking Service object versions
and dealing with compatibility there, it would be easier for them
to track the ServiceStatusPayload version which can remain
relatively stable even if Service is changing to adapt to db/RPC changes.
Not only do they not really care about tracking the Service object
versions, they probably also don't care about what's in that filter list.
But I think you're getting on the right track as to where this needs
to go. We can integrate the filtering into the versioning of the payload.
But instead of a blacklist, we turn the filter into a white list. If
the underlying object adds a new field that we don't want/care if
people know about, the payload version doesn't have to change. But if
we add something (or if we're changing the existing fields) that we
want to expose, we then assert that we need to update the version of
the payload, so the consumer can look at the payload and say "oh, in
1.x, now I get _______" and can add the appropriate checks/compat.
Granted with this you can get into rebase nightmares ([1] still
haunts me in my sleep), but I don't see us frantically changing the
exposed fields all too often. This way gives us some form of
pseudo-pinning of the subobject. Heck, in this method, we could even
pass the whitelist on the wire right? That way we tell the consumer
explicitly what's available to them (kinda like a fake schema).
I think see your point, and it seems like a good way forward. Let's
turn the black list to a white list. Now I'm thinking about creating a
new Field type something like WhiteListedObjectField which get a type
name (as the ObjectField) but also get a white_list that describes which
fields needs to be used from the original type.
Then this new field serializes only the white listed fields from the
original type and only forces a version bump on the parent object if
one of the white_listed field changed or a new field added to the
white_list.
What it does not solve out of the box is the transitive dependency. If
today we Have an o.vo object having a filed to another o.vo object and
we want to put the first object into a notification payload but want to
white_list fields from the second o.vo then our white list needs to be
able to handle not just first level fields but subfields too. I guess
this is doable but I'm wondering if we can avoid inventing a syntax
expressing something like 'field.subfield.subsubfield'
in the white list.
Rather than a whitelist/blacklist why not just define the schema of the
notification within the notification object and then have the object code
handle pulling the appropriate fields, converting formats if necessary, from
contained objects. Something like:
class ServicePayloadObject(NovaObject):
SCHEMA = {'host': ('service', 'host'),
'binary': ('service', 'binary'),
'compute_node_foo': ('compute_node', 'foo'),
}
fields = {
'service': fields.ObjectField('Service'),
'compute_node': fields.ObjectField('ComputeNode'),
}
def populate_schema(self):
self.compute_node = self.service.compute_node
notification = {}
for key, (obj, field) in schema.iteritems():
notification[key] = getattr(getattr(self, obj), field)
Then object changes have no effect on the notifications unless there's a
major version bump in which case a SCHEMA_VNEXT could be defined if
necessary.
Nice idea I will try it. Thanks! It is seems to avoid the sub object field
white lists
problem as the needed notification field can always be pulled directly from an
object field.
+1
This is my preference, specific notification objects that are
independently versioned.
It feels like time saving to re-use existing objects, but it breaks
the interface really.
Ok, so that means we'll now have:
* REST representation (strongly versioned / documented)
* Notification representation (strongly versioned / documented )
* Nova objects representation (strongly versioned / documented only in code)
* Nova db objects (versioned by schema, documentated only in code)
If the notifications are not going to be raw Nova objects, I think we
need to really think about why they aren't the REST objects. Having a
whole other additional interface surface seems really really weird.
Having notifications use the REST representation makes a lot of sense to
me, for those things that have a REST representation. My point has
always just been that I think the raw Nova objects is wrong because the
versions move for things that notification consumers don't care about at
all, like a new remotable method being added to the object.
-Sean
--
Sean Dague
http://dague.net
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
