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. Thanks, johnthetubaguy __________________________________________________________________________ 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
