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