17:00:09 <gibi> #startmeeting nova notification
17:00:10 <openstack> Meeting started Tue Jan  3 17:00:09 2017 UTC and is due to finish in 60 minutes.  The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:11 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:14 <openstack> The meeting name has been set to 'nova_notification'
17:00:24 <gibi> happy new year!
17:00:47 <MikeG451> Happy New Year to you too!!
17:00:52 <aunnam> hi gibi, happy new year
17:01:22 * mriedem creeps
17:01:28 <sneti> Happy new year!!
17:01:44 <gibi> OK let's get started
17:02:04 <gibi> short recap where we are
17:02:25 <gibi> versioned notification has patches up to review
17:02:52 <gibi> I update the priority etherpad two days ago
17:03:19 <gibi> the falvor notifications needs some changes
17:03:54 <gibi> we agreed that the flavor.delete payload shall contain the full payload class instead of just the uuid
17:04:22 <gibi> alos
17:04:24 <gibi> also
17:04:58 <gibi> the lazy loaded field projects needs some thinking still
17:05:37 <gibi> currently the projects doesnt show up in the sample
17:06:32 <gibi> the additions for searchlight needed some extra work due to the tags field
17:06:35 <gibi> that is also lazy loaded
17:06:35 <mriedem> gibi: i thought that was because the test didn't add a tenant to the flavor?
17:07:13 <gibi> mriedem: can be, but I suspect that it is related to the lazy loadness of the projects field
17:07:29 <gibi> mriedem: let me fetch a link
17:08:21 <gibi> mriedem: https://github.com/openstack/nova/blob/master/nova/notifications/objects/base.py#L97
17:08:40 <gibi> mriedem: if the projects is not already loaded then the schema population will not load it
17:09:09 <gibi> mriedem: in the other hand if we remove L97 then other things start to fail
17:09:37 <gibi> mriedem: I field a related bug https://bugs.launchpad.net/nova/+bug/1653221
17:09:39 <openstack> Launchpad bug 1653221 in OpenStack Compute (nova) "availability_zone field is missing from the service.update notification payload" [Undecided,In progress] - Assigned to Balazs Gibizer (balazs-gibizer)
17:10:33 <gibi> the fix is here https://review.openstack.org/415857 but it need some iteration before I can remove L97
17:11:37 <gibi> anyhow the projects field needs some extra care
17:12:55 <mriedem> hmm,
17:13:00 <mriedem> on https://github.com/openstack/nova/blob/master/nova/notifications/objects/base.py#L97
17:13:25 <mriedem> i wonder if we could check if the field is nullable and if so, just set it to None for the notification - but that doesn't fix flavor.projects which isn't nullable
17:13:32 <mriedem> and the actual value in the db might not be null
17:15:11 <gibi> mriedem: the only problem currently is with Service.availability_zone that is not lazy loaded but also not initialized  which leads to an error
17:15:23 <mriedem> i wonder why https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L422 doesn't include projects
17:15:26 <gibi> mriedem: after we initialize that to None I can remove L97
17:15:50 <gibi> mriedem: I have no clue
17:17:28 <mriedem> i suppose because we only ever get the extra_specs on the initial get https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L291
17:17:30 <mriedem> from the db
17:18:41 <mriedem> and yeah we only lazy-load projects on the Flavor object https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L350
17:18:57 <mriedem> that's probably just how it's always been in the DB API so it's modeled the same here
17:19:55 <mriedem> we could try to lazy-load the field in https://github.com/openstack/nova/blob/master/nova/notifications/objects/base.py#L97 and catch the ObjectActionError and just ignore it if we can't load the field..
17:20:40 <gibi> mriedem: yes, I'd like to remove L97, to trigger lazy load
17:20:59 <mriedem> anyway, we can move on
17:21:03 <gibi> mriedem: however I think we shall not ignore ObjectActionError but we should avoid it
17:21:06 * mriedem has to leave in about 10 min
17:21:09 <gibi> Ok
17:21:25 <gibi> so the next is the searchlight additions
17:21:34 <gibi> the basic additions are OK
17:21:45 <gibi> the instance.tags also lazy loaded
17:22:01 <gibi> but there we agreed to make that field loaded by default
17:22:26 <gibi> patches are up for that as well
17:23:00 <gibi> and last but not least
17:23:15 <gibi> the json schema bp also has some nova patch up
17:24:13 <gibi> we still have some dependency issue with the oslo_versioned notifications lib I guess
17:24:58 <gibi> that is all info I have for bps
17:25:13 <gibi> we have couple of weeks until the freeze
17:25:34 <mriedem> yeah 1/19 is lib freeze
17:25:41 <mriedem> and then a week after that is feature freeze
17:25:54 <gibi> so we have to focus on things that already up for review
17:26:46 <gibi> I will try to keep the priority etherpad up to date to help the cores
17:26:48 <mriedem> li yingjun pointed out something with the flavor delete payload
17:26:51 <mriedem> https://review.openstack.org/#/c/398171/
17:27:03 <mriedem> they said that only flavorid is set when the flavor is destroyed https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/flavor_manage.py#L46-L46
17:27:15 <mriedem> ah because we don't look it up from the db first
17:27:25 <mriedem> we rely on destroy to either pass or fail
17:27:46 <gibi> looking...
17:28:50 <gibi> mriedem: is it some optimization not to look up the whole flavor from the db first?
17:29:05 <gibi> mriedem: seems weird for me
17:30:03 <mriedem> i left a comment in there,
17:30:11 <mriedem> before deleting the flavor, we look it up,
17:30:21 <mriedem> we could return that result and use it to create a Flavor object used for the delete notification
17:30:40 <mriedem> anyway, we can move that to the review
17:30:45 <mriedem> i've got to head out
17:30:52 <gibi> mriedem: cool, thanks for the comment
17:31:55 <gibi> OK, I have nothing else
17:32:06 <gibi> I'm still in a bit of vacation mode until Thursday
17:32:11 <gibi> any questions?
17:32:46 <aunnam> I have question on serachlight BP
17:32:54 <gibi> shoot
17:33:05 <aunnam> I am working on adding key_name field to InstancePayload. In the BP it was mentioned to add list of KeyPair objects containing at least the
17:33:05 <aunnam> name field of the original KeyPair objects based on instance.keypairs
17:33:20 <aunnam> If we just need the key_name then can we directly get it from instance.key_name field https://github.com/openstack/nova/blob/77b72a5b107acf130e7404cc080ba721f474db8f/nova/objects/instance.py#L125
17:33:20 <aunnam> instead of adding the Keypair object?
17:33:44 <aunnam> Searchlight team also asked only for key_name field as the api returns it http://paste.openstack.org/show/xbv2CwtHnhhl1nLLiJeN/
17:34:38 <gibi> how does the intance.key_name relates the instance.keypairs ?
17:35:43 <aunnam> the key_name field is the name of the keypair right?
17:36:19 <gibi> but keypairs could hold a list of keys, while key_name can only refer to a single keypair
17:37:02 <gibi> aunnam: seems like key_name is an old thing https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L943
17:37:10 <aunnam> so an instance can have many keys?
17:37:56 <gibi> as far as I understand, yes
17:38:34 <aunnam> ok, then i will double check
17:38:50 <aunnam> keypair is also lazy loaded
17:38:58 <gibi> yes, it seems
17:39:28 <aunnam> so should I follow the same approach as tags?
17:39:48 <gibi> good question
17:40:20 <gibi> if I could remove https://github.com/openstack/nova/blob/master/nova/notifications/objects/base.py#L97 this if then lazy load would not be a problem
17:40:33 <gibi> as the populate_schema would trigger the load anyhow
17:41:06 <gibi> based on the discussion with mriedem above I think this would be the better approach
17:41:16 <gibi> so you don't have to make it loaded by default
17:41:34 <sneti> gibi, populate_schema would be called for each notification right?
17:41:37 <gibi> but I have to do some work on https://review.openstack.org/#/c/415857/ to remve L97
17:42:00 <gibi> sneti: if the notification uses SCHEMA then yes, the populate_schema is called
17:42:15 <gibi> to copy the data from the normal nova ovo to the notification ovo
17:42:45 <aunnam> gibi, then I will follow your patch and wait till your change
17:42:59 <sneti> yes.. so for fields that are lazy loaded, it is loaded for each notification
17:43:15 <gibi> aunnam: OK. I will finish that patch as soon as I return from vacation (Thursday)
17:43:29 <sneti> this was the reason we decided to change tags to default right
17:43:31 <aunnam> gibi:thanks
17:43:31 <gibi> sneti: yes, that would cause a load as soon as L97 is removed
17:44:04 <gibi> sneti: yes, in case of tags we decided to load it by default instead of lazy load
17:44:59 <sneti> I thought this should be the same all lazy loaded fields to avoid db calls
17:45:43 <gibi> if every code patch of the object to a notification that will load the given field then it looks nicer to load that field by default
17:46:11 <gibi> load by default and lazy load has a similar db impact just the lazy load happens later
17:46:27 <gibi> but I'm not an expert here
17:46:47 <gibi> if we have doubts then we should bring this up again with the cores
17:47:34 <sneti> gibi, yes. also can you check the comments on this when you have time: https://review.openstack.org/#/c/407228/13/nova/compute/utils.py
17:48:21 <gibi> sneti: I will check it
17:48:38 <sneti> gibi, thanks!
17:49:16 <gibi> any other questions/
17:49:17 <gibi> ?
17:51:43 <gibi> OK then lets close this for today
17:51:51 <gibi> thanks for showing up
17:52:03 <gibi> see you around
17:52:12 <gibi> #endmeeting