Open Stack

> -----Original Message-----
> From: Andrew Laski [mailto:andrew at lascii.com]
> Sent: November 30, 2015 20:32
> 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.gibizer at ericsson.com> wrote:
> >>>> From: Andrew Laski [mailto:andrew at lascii.com]
> >>>> Sent: November 24, 2015 15:35
> >>>> On 11/24/15 at 10:26am, Balázs Gibizer wrote:
> >>>>>> From: Ryan Rossiter [mailto:rlrossit at 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:andrew at 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.gibizer at 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.

The differences between the REST and the Notification API
1)  REST consumed by normal and admin user while Notification is always consumed by 
admin only as it is published on the message bus. This means we might want to put more 
information in the Notification than what is in the REST API.
2) REST API has a version information in the HTTP header, 
the Notification needs the version explicitly in the payload as there is no header concept 
on the message bus. Changing the message envelope on the bus would cause
inconsistencies in the notification envelope between projects.
3) Notifications are triggered by the Server side events while REST API is triggered from 
the client side. This means that the content of the notification often has a different purpose
that a REST API response. For example we have the instance.update notification does not
simply contain an instance object but a lot of additional field (e.g. audit period,  old_state,
old_task_state)  as it is talks about an 'update' situation which is triggered by some state
change in the system and not an explicit REST request. Also even if a notification is triggered 
by a REST request the consumer of the notification does no know about the content of the 
REST request so the notification shall contain more information than a simple REST response
where the request is also part of the knowledge of the client.

Based on 3) I feel that we anyhow need to have a separate model for the Notifications and
I feel that depending on the Nova object representation gives use the flexibility to handle 1)
and 2).

Cheers,
Gibi
> 
> 
> >
> >	-Sean
> >
> >--
> >Sean Dague
> >http://dague.net
> >
> >_________________________________________________________
> ______________
> >___ OpenStack Development Mailing List (not for usage questions)
> >Unsubscribe:
> >OpenStack-dev-request at 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-
> request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev