14:00:31 #startmeeting neutron_upgrades 14:00:31 Meeting started Thu Apr 19 14:00:31 2018 UTC and is due to finish in 60 minutes. The chair is lujinluo. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:00:32 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 14:00:35 The meeting name has been set to 'neutron_upgrades' 14:00:39 o/ 14:00:41 o/ 14:01:11 o/ 14:01:44 First thing first. mlavalle has some concerns about this patch, https://review.openstack.org/#/c/556667/ 14:01:55 not sure if ihar gets time to check the comments already 14:02:13 Hi mlavalle 14:02:16 Hi Ihar 14:02:17 admitedly my concern my be unwarranted 14:02:22 Hi Luo-san 14:02:27 hey TuanVu 14:02:36 hey TuanVu ! 14:02:58 mlavalle: I saw point on more comments needed. agreed. 14:03:06 but i agree that, we may need more comments to help future readers 14:03:13 I won't be able to get to this patch till Monday though so that's a bummer. 14:04:57 ok, i will try to put some comments but i need your review/double check just in case i misunderstand anything 14:04:57 I see there is another comment there not just about comments though 14:05:41 mlavalle: I didn't have a chance to think about it. how critical is the weak ref comment? is there a chance for getting this in with the caveat I will follow up on Monday? 14:06:49 ihar: yeah, as long as we answer to ourselves the question soon and feel comfortable that we are not get in trouble, I am fine 14:07:12 so maybe let's revise the patch with comments, get it in 14:07:13 ok. I will put it in my list to follow up. 14:07:36 and then work next week on the weakreference concern 14:07:41 does that work? 14:07:54 if lujinluo is able to handle the comments part, sure 14:08:15 do you want to do that lujinluo? 14:08:34 i can help with the comments but maybe not weakreference 14:08:58 I will help. it is a good exercise for you and me lujinluo. We better get very familiar with this piece of coe :-) 14:08:59 yeah not weak ref. I will need to take a look into it but seems from the comment it's a legit thing. 14:09:12 yes you better are, it's as you said esoteric 14:09:17 mlavalle: exactly ;) 14:09:56 cool! 14:10:10 #actionitem mlavalle and lujinluo help with the comments in auto expire fk patch 14:10:23 lujinluo: it's #action btw 14:10:38 thanks, i was wondering why the magic did not happen 14:10:46 LOL 14:10:54 #action mlavalle and lujinluo help with the comments in auto expire fk patch 14:11:51 ok, anyway.. next one is the auto detect old and new engine facade https://review.openstack.org/#/c/553617/17 14:12:28 i understand that we need the former patch before get this one in, but it is worth to mention the status 14:12:55 yeah, it's good to go, just need to nudge the base one 14:13:11 \o/ 14:13:24 #topic OVO 14:13:32 https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db 14:14:00 first one is new https://review.openstack.org/#/c/562489/ 14:14:15 CI so green, I can't stop being amazed 14:14:25 i think TuanVu is trying to tackle joins here 14:14:34 yes 14:15:00 if there's any concern, please feel free to let me know 14:15:00 wow, yeah, all green. so good for our eyes 14:15:24 ihar: i actually have a concern of the way we deal with joins 14:15:42 ok this patch moved the code but it didn't solve the issue that we pass sqlalchemy queries around 14:15:47 previously we just moved all the methods to object 14:15:52 which should not be the way we interact with ovo objects 14:16:01 yes, i think eventually we need a way to deal with joins 14:17:04 i mean, we can rewrite all the joins with get_object and filter in python, but do you prefer if we can have something like join_object()? ihar 14:17:31 in this particular case, why not adding a new extra filter called 'service_type' and handle it accordingly in ovo? 14:18:08 no, i mean. if we do not limit to this particular case 14:18:19 because most of the remaining work is about joins 14:18:59 lujinluo: we already have get_object[s] as a common interface. it would be nice if we can stick to it, just expand it for the Subnet class to support a new extra filter for service_type. Then you could just Subnet.get_objects(context, network_id=nid, service_type=service_type) and not care about implementation complexity 14:20:01 in general, I would recommend expanding standard interfaces like get_object[s] to support additional filters (of course it requires some if/else branching in their implementations depending on which filters are passed). 14:20:28 we may add custom getters like get_subnets_by_service_type(...) if it seems hard to achieve right now 14:21:03 but we should never have any part of sqlalchemy interfaces / types be exposed (so no queries passed around as arguments, or models returned from ovo methods) 14:21:04 is the corllary that at this point you see the OVO interface as stable and the rest of the work should happen underneath? 14:21:55 mlavalle: more or less, I think what we have is a good interface that we should try not to expand with custom methods without clear reason. 14:22:04 ok, so you would prefer that we manipulate with get_object(s) 14:22:23 ihar: that's good insight. thanks 14:22:24 thing is, if some other plugin wants to use ovo objects and needs to filter by service_type, what should they do? 14:22:51 supposedly we are moving base interface into neutron-lib and, as per late boden's patch, provide a registry to get objects from 14:23:02 yeap 14:23:34 but all the external plugins know about objects received from the registry is that they all have standard methods like get_object[s] or update. there is no way for them to learn that there is get_subnets_by_service_type too. 14:24:13 if we implement it the right away with standard interfaces, plugins will be able to transparently request the needed filters without caring about db table structures. 14:24:28 exactly 14:24:36 yeah, so we have to add extra filters to get_object() not extra methods 14:24:40 lujinluo: yes I would prefer get_object[s], and I strongly disagree with passing queries as arguments 14:24:49 agree 14:25:20 lujinluo: yes, that would be ideal. unless of course we have a good reason for a custom method. (like it's too hard to do it; then we may get back to it later) 14:27:00 I left a comment along these lines 14:27:18 i see. i was testing a local patch with 3 tables joins 14:28:01 thanks for the comment, Ihar 14:28:02 so with that being said, i should revise corresponding get_object(s) 14:28:12 now, I don't disagree that making joins work through standard interfaces is quite an undertaking. 14:28:44 but we should try it. maybe once we actually try it, we learn something that would defeat all I just said. 14:28:56 but unless we try on a patch or two, we don't know 14:29:03 got it 14:29:26 TuanVu: does it make sense? 14:29:34 yes, I agree 14:29:54 in other words, let's force ourselvers to find a good reason to change the current API 14:30:26 ++ 14:31:01 ok, next one https://review.openstack.org/#/c/507772/ 14:31:58 I see rebase helped with InvalidRequestError, that's good. 14:32:06 still a failure with query count though. 14:32:07 as I mentioned on Gerrit, there's still problem with "duplicated queries" for "networksecuritybindings" and "externalnetworks" 14:32:19 yes 14:33:09 I don't have a good answer except that I can look into it on Monday 14:33:24 thank you in advance, Ihar :) 14:34:19 i think i have the same issue with port. so i need to double check tmr or later too 14:34:36 next is segment https://review.openstack.org/#/c/559414/ 14:34:51 hongbin has addressed all the comments from ihar 14:35:27 o/ 14:35:44 wow, hi hongbin_ i did not expect you would be here 14:35:54 he is everywhere 14:36:00 ^^ 14:36:01 LOL 14:36:07 omnipresent 14:36:19 ack, +2 14:36:35 man, this green ci is amazing 14:36:43 but i think we need to wait for the fk patch before +w, right? 14:37:01 we can +w whenever we like as long as it's depends-on (which it is) 14:37:10 zuul will not merge it until all dependencies are met 14:37:27 understood! 14:37:54 next one https://review.openstack.org/#/c/561834/ 14:38:23 i think it needs some more work wrt joins 14:38:35 yes 14:38:38 yes it's the same issues as discussed above 14:38:40 as above discussion 14:39:17 ok, the next two patches are mine.. and i apologize for not being able to work on them last week 14:39:57 i will respin them before our next meeting 14:40:36 ok. afair we wanted to base port stuff on mlavalle's multiple port bindings rfe implementation 14:40:58 yeap 14:41:00 yes, but it seems mlavalle 's patch is in merge conflict 14:41:04 so it requires quite some work I suspect 14:41:10 lujinluo: I'll fix it 14:41:23 thanks, i will rebase once it is ready 14:41:29 :-) 14:41:40 ok, that's all for ovo 14:41:46 mlavalle: please do! that rfe is one of things I promised to get to done before I move to other places. :) 14:41:46 #topic open discussion 14:42:16 ihar: thanks. I think we can merge it soon 14:42:18 ihar: ohh, you just reminded us of some heart-breaking stuff 14:42:47 tha you are moving to other places ;( 14:42:51 that* 14:42:58 to the topic of open discussion, I have nothing specific to discuss 14:43:17 I just one to mention one thing 14:43:36 yes, please 14:43:38 This past Monday I added a topic to the Neutron weekly meeting on OVO backlog 14:43:46 #link https://wiki.openstack.org/wiki/Network/Meetings#Blueprints 14:44:09 thank you! 14:44:15 I mentioned it during the meeting and I will continue doing so every week 14:44:28 thank you very much, mlavalle 14:44:29 so people can see the backlog and help if they have bandwidth 14:44:30 is it why we are having hongbin_ here ? lol 14:44:50 😂 14:44:55 no, as I said, hongbin_ is omnipresent 14:45:13 \it doesn't have to do with any of my efforts 14:45:18 LOL, i see. but this is really appreciated anyway 14:45:23 yeah 14:45:49 it would be great if there's more people working on remaining OVO parts 14:46:17 yeah btw one thing to mention in this venue 14:46:35 I already mentioned it passing above but to make sure everyone is on the same page 14:46:53 we merged boden's patch https://review.openstack.org/#/c/553838/ that adds OVO registry 14:47:33 tl;dr projects (incl. neutron) will register their objects and then other subprojects will be able to get classes from this registry by name. 14:47:45 so we get rid of module import dependency between two 14:48:03 nice 14:48:28 it's new thing, not released yet, but I believe boden will follow up with adoption once it is 14:48:41 thanks for this info, Ihar 14:49:01 need to take closer look to follow 14:49:12 we will release neutron-lib next week 14:49:21 this is partially the reason why I am more concerned about custom methods now because we are going big league now, officially exposing ovo as part of neutron interfaces. 14:49:22 it will be included in that release 14:49:46 and plugins may start catching up to this interface more now 14:50:13 (so far we had just a bunch that use ovo in any capacity) 14:50:27 yeah, i will keep that in mind too. need to consider more 14:50:56 ok that's all I have 14:51:07 thanks! 14:51:12 o/ 14:51:22 dose anyone have anything else? 14:51:30 thank you, Ihar :) 14:51:34 if not, we can call it a day! 14:51:52 it seems like we can indeed 14:51:53 Good night you ak 14:51:55 o/ 14:51:59 #endmeeting