17:00:24 <gibi> #startmeeting nova notification
17:00:25 <openstack> Meeting started Tue Mar 28 17:00:24 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:26 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:28 <openstack> The meeting name has been set to 'nova_notification'
17:00:40 <gibi> o/
17:01:01 <aunnam_> o/
17:01:51 <gibi> let's wait a bit for others to join
17:03:58 <gibi> it seems just the two of us are here today
17:04:22 <gibi> aunnam_: I saw the BDM patch
17:04:39 <gibi> aunnam_: I think it is moving to the right direction
17:04:46 <gibi> aunnam_: I will continue reviewing that
17:05:13 <gibi> aunnam_: do you have any questions about my recent comments?
17:05:26 <aunnam_> gibi, thanks for the review, just going through your comments
17:05:54 <aunnam_> so you want us send notifications only when instance is booted with a volume?
17:06:19 <aunnam_> not like boot an instance and attach a volume to it
17:07:36 <gibi> there is an instance.volume_attach notification emitted when a user attach an extra volume to an instance after boot
17:08:23 <gibi> that notification will also contain the BDM as the payload of that notification also uses the InstancePayload your patch extends
17:08:31 <gibi> my comment was more about the functional test
17:09:28 <gibi> in the functional test I think it would be better to boot the instance with a volume instead of adding the volume later
17:10:18 <gibi> the attach volume case will be tested anyhow here https://review.openstack.org/#/c/401992/
17:10:27 <aunnam_> ok, in the functional it would make more sense to boot the instance with volume as the attaching test case already covered in transformation patch
17:10:41 <gibi> aunnam_: exactly
17:10:48 <aunnam_> ok understood
17:10:51 <gibi> cool
17:11:14 <aunnam_> i'll make refactoring as you suggested for _get_bdms method
17:11:21 <gibi> thanks
17:11:25 <aunnam_> and one more question
17:11:43 <aunnam_> in the BP it was mentioned to add a conf option
17:12:20 <aunnam_> on whether to emit BDM's notifications or not
17:12:34 <aunnam_> should we consider that now?
17:13:24 <gibi> let me check the instance.update code path
17:13:37 <aunnam_> ok
17:15:18 <gibi> I see, so now in any case when we want to send an instance notification we query the db for the bdm in _get_bdms
17:15:37 <gibi> this could mean and increased db load
17:15:54 <aunnam_> ya query is needed in all cases
17:16:37 <gibi> I guess is some cases when the code calls notify_about_instance_action() the bdm is already loaded from the db but not in every case
17:17:20 <gibi> so we load it in _get_bdms to make it available
17:17:45 <gibi> It would be good to know how big the problem with the increase db load due to this
17:18:24 <gibi> If this is not acceptable in every case then we need that extra config option
17:18:56 <gibi> I will leave some comment about this in the review and I will try to pull in others to get more opinions
17:19:05 <gibi> but it is possible that at the end we need the new config
17:20:03 <aunnam_> ok, lets discuss it on the patch to get more feedback
17:20:22 <gibi> yeah, I don't feel I can decide alone
17:20:50 <gibi> and also we have to consider that there is some improvement about this here as well https://review.openstack.org/#/c/428260/
17:21:13 <gibi> but this ^^ does not solve every case
17:21:56 <gibi> do you have any other question?
17:22:13 <aunnam_> In your next weekly status to mailing list can you mark searchlight patches as high priority, it is becoming hard to maintain those patches as there is dependency chain of 4-5 patches.
17:22:13 <aunnam_> Whenever a versioned notifications patch gets merged, need to change all the patches in series.
17:24:12 <gibi> good point so far it seemed that normal transformations got more review bandwidth than the searchlight bp
17:24:27 <gibi> I will try it differently next week
17:24:43 <gibi> maybe different order, or more directly link to the searchlight patches
17:25:08 <gibi> btw this week transformation patches volume_attach and _detach are also needed for searchlight
17:25:16 <gibi> but understand your pain about the merge conflicts
17:25:54 <aunnam_> saw the comment on tags patch today, looks like it needs to be changed, more work for sneti as she has to change around 35 files:)
17:26:58 <aunnam_> as for tags even in feature we don't get new fields other than tag name?
17:27:00 <gibi> yeah, a bit of monkey work to change all these
17:28:07 <aunnam_> s/feature/future
17:28:26 <gibi> I think the whole concept of tags is about is to attach arbitrary strings to some resource, in this case to the instance. So I think it is safe to assume it won't change
17:28:55 <aunnam_> ok
17:31:49 <gibi> anyithing else on your side?
17:32:02 <aunnam_> nothing
17:32:08 <aunnam_> thanks for the info
17:34:10 <gibi> anybody else has anything to discuss?
17:35:14 <gibi> It seems that this was all for today then
17:35:20 <gibi> aunnam_: thanks for the discussion
17:35:32 <gibi> aunnam_: I will add some comments to the BDM review
17:35:39 <gibi> let's close this.
17:35:46 <gibi> #endmeeting