17:00:19 <gibi> #startmeeting nova notification
17:00:19 <openstack> Meeting started Tue Jun  5 17:00:19 2018 UTC and is due to finish in 60 minutes.  The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:21 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:23 <openstack> The meeting name has been set to 'nova_notification'
17:00:46 <mriedem> o/
17:00:48 <gibi> hi
17:01:01 <gibi> I have one think to discuss today
17:01:49 <gibi> the complex server group policy code change tries to rename and retype the policies attribute of the server group https://review.openstack.org/#/c/563401/9/nova/notifications/objects/server_group.py
17:01:55 <gibi> in the notification object
17:02:18 <gibi> either we do a major version bump on the notifiation and delete the old policies field
17:02:40 <gibi> or do a minor bump and keep both the old policies field and add the new policy field
17:03:06 <mriedem> i wasn't expecting a major version bump - was more expecting along the lines of what he's doing in the InstanceGroup object https://review.openstack.org/#/c/563375/11/nova/objects/instance_group.py
17:03:11 <mriedem> deprecate policies, add policy
17:03:31 <gibi> OK, so the instance group still has the old polcies field available
17:03:34 <gibi> that helps
17:03:54 <mriedem> yes although it doesn't set it
17:04:13 <gibi> so we don't have the value to populate the notification payload
17:04:48 <gibi> we might reuse the obj_make_compatible call to get the value back
17:05:02 <mriedem> true
17:05:14 <mriedem> i thought we didn't use obj_make_compatible in the versioned objects though
17:05:45 <mriedem> i.e. are you suggesting we set both policies and policy in the notification payload?
17:06:25 <gibi> to keep the meaning of the minor bump we should populate the policies field in the payload
17:06:47 <gibi> as somebody using the parser for the older minor version still try to use that
17:07:25 <mriedem> have we done a major bump in the versioned notification objects yet?
17:07:26 <gibi> it only make sense to me to to send the old field if we send real value in that field
17:08:24 <gibi> no, this would be the first major bump on the payload
17:08:53 <gibi> the NotificationPublisher has version 2.2 but that is not directly serialized into the notification
17:09:16 <mriedem> is there any problem in sending a new major version of this payload?
17:09:30 <mriedem> we don't know who or how these are consumed so it's hard to say
17:09:39 <gibi> sending is easy
17:09:52 <gibi> but as you said we don't know who is receiving it
17:10:15 <gibi> btw, how we keep the REST API compatible for the older microversions regardin the polcies field?
17:10:18 <mriedem> i defer to what you think's best here
17:10:34 <mriedem> the caller of the rest api gets the response body back based on the version
17:10:47 <mriedem> with notifications, no one is calling with the version they want
17:10:52 <mriedem> they just get what we give them
17:11:41 <gibi> mriedem: I mean with <2.62 the REST API should return a polcies field with real value
17:12:01 <gibi> mriedem: so we have the data available
17:12:09 <gibi> I mean have to have the data available
17:12:34 <mriedem> yes
17:12:34 <gibi> https://review.openstack.org/#/c/567534/9/nova/api/openstack/compute/server_groups.py@102
17:12:37 <mriedem> that's true for the rest api
17:13:14 <gibi> the API does a db query like objects.InstanceGroup.get_by_uuid(context, id)
17:13:25 <gibi> it does not specify the object version
17:13:29 <gibi> so I assume it gets the latest
17:14:03 <gibi> and still assumes that the group.polcies field is populated in the object
17:14:55 <gibi> so I don't see how the notification codepath cannot make the same assumption
17:14:57 <mriedem> because of https://review.openstack.org/#/c/563375/11/nova/objects/instance_group.py@185
17:16:37 <mriedem> i'm not sure where you're going with this, if you want the notification payload to just populate both fields (if possible) then i think that's fine
17:17:05 <gibi> mriedem: yeah, I would go with the minor bump and populate both fields
17:17:13 <mriedem> probably with a note that the policies field is for legacy compat
17:17:17 <gibi> mriedem: I might misunderstand your statement about not haveing the old field populated
17:17:46 <mriedem> i meant https://review.openstack.org/#/c/563375/11/nova/objects/instance_group.py@158
17:19:36 <gibi> so for the new groups the polcies field will not be set and the REST API < 2.63 will return sort of false information for those as well
17:20:32 <gibi> or I'm confused
17:20:33 <gibi> :0
17:20:38 <gibi> what will https://review.openstack.org/#/c/563375/11/nova/objects/instance_group.py@207 do
17:21:00 <gibi> anyhow I will comment on the patch
17:21:22 <gibi> bottom line, i try to go with the minor bump if we can have the old polcies field populated
17:21:22 <mriedem> it looks like when pulling one of these out of the database, both the policies and policy fields will be set
17:21:25 <mriedem> so the rest api can go either way
17:22:11 <gibi> hm, but the value of 'policy' will be someting complex so putting that into the policies field will not work
17:22:32 <mriedem> policy is an object with a policy field, which is the policy name
17:22:33 <gibi> as the polcies field is just a sting
17:22:38 <mriedem> so it sets policies = [policy.policy]
17:22:41 <mriedem> yes
17:22:43 <mriedem> list of strings
17:23:25 <gibi> OK L 177 does that but L 207 does polcies = [policy]
17:23:41 <mriedem> i think this is a good opportunity for you to leave comments on the review :)
17:23:44 <mriedem> and for me to have lunch
17:23:46 <gibi> exactly
17:23:51 <gibi> I will do that
17:24:02 <gibi> thanks for your time
17:24:07 <mriedem> yw
17:24:08 <mriedem> ttyl
17:24:08 <gibi> have a good lunch
17:24:14 <gibi> #endmeeting