14:10:53 <ihrachys> #startmeeting neutron_upgrades
14:10:54 <openstack> Meeting started Thu Feb 22 14:10:53 2018 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:10:55 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:10:58 <openstack> The meeting name has been set to 'neutron_upgrades'
14:11:24 <ihrachys> sorry for that
14:11:40 <ihrachys> #topic PTG
14:11:52 <lujinluo> not a problem
14:12:09 <ihrachys> lujinluo, I thought you were going to Dublin but then I saw you don't, as per ptg etherpad. right?
14:12:21 <TuanVu_> no problem at all, Ihar :)
14:12:31 <lujinluo> well, i am going. which etherpad do you mean?
14:12:44 <lujinluo> i remember putting my name in neutron etherpad
14:12:59 <ihrachys> hm. maybe it was a bad dream lol. let me find it.
14:13:40 <lujinluo> https://etherpad.openstack.org/p/neutron-ptg-rocky i put my name here
14:13:41 <ihrachys> the pad here: https://etherpad.openstack.org/p/neutron-ptg-rocky
14:13:54 <lujinluo> Line 22
14:14:40 <ihrachys> lujinluo, oh right! I mixed you and liuyulong in line 35. sorry for that.
14:14:43 <ihrachys> great
14:14:50 <lujinluo> nice
14:15:17 <lujinluo> speaking of that, since we have resolved the new engine facade conflict, i do not think we will have any OVO specific topics
14:15:33 <lujinluo> excepting for neutron-lib ones, proposed by boden?
14:15:43 <ihrachys> well, there are some neutron-lib
14:16:14 <ihrachys> also, enginefacade issue is not solved until all objects (and their plugin consuming code) switches to the new facade
14:16:24 <lujinluo> yeah, but boden says he won't be there. so i am wondering how the topics would proceed
14:17:11 <lujinluo> also, for the new engine facade, i want to bring this bug to your attention. it is purely about tests refactoring, but i think it worth your attention
14:17:19 <ihrachys> I walked through his specs that are related to OVO and both seem quite solid to just move forward with them (at least pieces that are OVO specific)
14:17:22 <lujinluo> https://bugs.launchpad.net/neutron/+bug/1750735
14:17:22 <openstack> Launchpad bug 1750735 in neutron "[OVO] UT fails when setting new_facade to True" [Undecided,New] - Assigned to Lujin Luo (luo-lujin)
14:18:36 <ihrachys> yeah we were in a hurry to deliver that so some tests are missing
14:19:00 <lujinluo> i went through his two specs as well. but i feel like i need more investigation to understand what he wants to implement, lol
14:20:30 <ihrachys> lujinluo, I see you assigned yourself to the bug. so the idea there would be to add a fake test object in test_base, and add a unit test class for that by inheriting from base 'iface' class. we don't necessarily need to switch an object to new_facade to trigger the tests, fake object will do for the start.
14:21:58 <lujinluo> yeah, i assigned myself but i am afraid i won't have time to code before PTG. but once i get back, i will follow your suggestion
14:22:18 <ihrachys> it's not critical (yet), as long as it's just unit tests
14:22:40 <ihrachys> as for boden specs, we can walk through them to see what he's up to
14:23:01 <ihrachys> you know the general goal of neutron-lib and what he does, don't you?
14:23:21 <ihrachys> he = boden
14:23:26 <lujinluo> actually no.. that's why i am struggling to understand the big picture
14:24:02 <ihrachys> ok. so in the past, we had a single neutron tree with all plugins
14:24:16 <ihrachys> cisco, midonet; lbaas, fwaas, ...
14:24:28 <ihrachys> everything imported from neutron.* and it was fine because it's same tree
14:24:46 <ihrachys> then the decision was made to split all but essential pieces into separate repos
14:24:52 <ihrachys> which was done around liberty
14:25:00 <ihrachys> (liberty as in release name)
14:25:16 <lujinluo> haha
14:25:38 <ihrachys> but then the problem became clear that whenever we change smth in neutron repo, we often break some stadium project because now we don't gate on their tests
14:25:53 <ihrachys> so neutron-lib is an attempt to answer to that challenge
14:26:27 <lujinluo> i see. this is what happened to bgp this time, right?
14:26:31 <ihrachys> instead of subprojects importing from neutron.*, they would import from neutron_lib.* that 1) keeps just the right amount of shared code; and 2) keeps API stability promise to consumers.
14:26:51 <lujinluo> we reverted some ovo changes in neutron repo, then bgp broke
14:27:07 <ihrachys> lujinluo, not exactly because what they were hit is something implicit - the context of callback execution changed facades
14:27:19 <ihrachys> but failure vector is same
14:27:41 <lujinluo> i see. then it makes sense to put ovo into neutron-lib
14:27:50 <lujinluo> at least part of ovo
14:28:20 <ihrachys> so now boden works for a bunch of cycles already moving pieces of neutron code into the library
14:28:41 <ihrachys> of course sometimes we don't just move but modify api a bit so that it makes more sense as public api
14:29:03 <lujinluo> thanks, i got the general idea now
14:29:34 <ihrachys> lujinluo, yes, one thing is, bagpipe imports neutron.objects.base to define their objects. while we try to keep this interface more or less stable, it may still break at some point.
14:29:44 <ihrachys> so now to the specs
14:30:37 <ihrachys> one spec is to decouple 'db api and utils'
14:31:18 <ihrachys> there are simple cases there, miscelaneous util helpers. they just go as is into neutron-lib.
14:31:53 <ihrachys> but there are cases where plugins just import 'mixin' classes with db implementation of some API endpoints verbatim
14:32:29 <ihrachys> like PortBindingMixin
14:33:02 <ihrachys> those bits are really not meant to be shared (as-is)
14:33:57 <ihrachys> so the spec discusses what to move and how to approach it.
14:34:06 <ihrachys> and the other spec is about OVO / models
14:34:51 <ihrachys> one pattern that a lot of subprojects will need is to e.g. add a port, or fetch a network in their plugin code
14:35:47 <ihrachys> currently it means f.e. either sqlalchemy query (that refers/imports models from neutron) or OVO get_object[s] (which also requires import of OVO definition)
14:36:36 <ihrachys> boden suggests that we introduce a wrapper that would allow projects to do e.g.: util.get_ovo_class('Port') and get the right class.
14:36:47 <ihrachys> then they just know that all OVO objects have get_objects
14:36:51 <ihrachys> so it just calls it
14:37:08 <ihrachys> and at no point there is a direct import to neutron OVO class definition
14:37:39 <lujinluo> yeah, this sounds reasonable
14:37:45 <ihrachys> and then neutron (and subprojects) will have a util helper to register OVO classes for that
14:39:04 <ihrachys> plugins can then even do things like: sfc_cls = util.get_ovo_class('SFCResource'); if sfc_cls: do some additional work specific to sfc
14:40:21 <ihrachys> ok that's the general background behind the goal there
14:40:38 <lujinluo> ok, i see. thanks for the explanation!
14:40:53 <lujinluo> his design sounds straightforward
14:42:47 <ihrachys> ok great. if you will have time, please try to join the sessions since they will probably touch on OVO and facade one way or another
14:43:22 <lujinluo> sure, i will be there
14:43:27 <ihrachys> #topic OVO patches
14:43:28 <ihrachys> https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
14:43:48 <ihrachys> https://review.openstack.org/#/c/507772/ "Use Network OVO in db_base_plugin"
14:44:04 <ihrachys> TuanVu_, I saw your email but didn't have time to dive in / reply
14:44:14 <TuanVu_> no problem, Ihar
14:44:25 <ihrachys> apart from what's in there, do you have more problems with the patch?
14:44:28 <TuanVu_> I really appreciate your great help so far
14:44:52 <TuanVu_> thanks, that's the only blocking point at this moment
14:45:08 <ihrachys> ok
14:45:27 <ihrachys> https://review.openstack.org/#/c/537320/ "Use Port OVO in neutron/db/external_net_db.py"
14:46:08 <ihrachys> lujinluo, are results already with the new_facade engine fix in?
14:46:29 <lujinluo> this one hit by the bug i mentioned before
14:46:39 <lujinluo> cause i need to switch new_facade to true
14:46:59 <ihrachys> ah I see
14:47:27 <ihrachys> is it the only place where we use Port object?
14:48:12 <lujinluo> we had other places using Port object before this one, but they do not conflict with new engine (as far as i checked)
14:48:35 <ihrachys> yeah but if you switch the object to new facade, it's for those other cases too no?
14:49:01 <lujinluo> oh, right. i need to switch those places to new facade
14:49:39 <ihrachys> right. that may be a problem.
14:49:41 <lujinluo> i will take a note about that somewhere..
14:50:01 <ihrachys> one crazy thing I was thinking about though is, what if we could detect which engine is used and use the right facade?
14:50:33 <ihrachys> so that when ovo call happens under writer.using, it does one thing, but another if under begin(...)
14:50:47 <lujinluo> this would be great!
14:50:53 <ihrachys> then you could have your Port object behave correctly in either context and not care about calling context
14:51:14 <ihrachys> right. now the question is, how to make it happen lol
14:51:23 <lujinluo> lol
14:51:49 <ihrachys> it may be the case that writer.using / begin leave some marker in session object that we could (ab)use for this
14:52:23 <lujinluo> we could check whether they do
14:52:33 <ihrachys> and if not, we could introduce our own facade entry points that do, and switch all code to using them
14:52:53 <lujinluo> i will take a note about this somewhere too..
14:54:24 <ihrachys> ok. that may be part of solution for facade problem
14:54:27 <ihrachys> at least for ovo
14:54:35 <lujinluo> lol
14:54:37 <ihrachys> we would still need to switch to new one in all places
14:54:53 <ihrachys> but not as synchronized and intertwined as it would be right now
14:55:22 <ihrachys> and if we have it, we could also just drop new_facade attribute since it would be of no use
14:55:38 <lujinluo> yes!
14:55:40 <ihrachys> dreams dreams dreams
14:55:46 <ihrachys> but i don't think it's impossible. we can do it!
14:55:47 <ihrachys> :)
14:55:53 <lujinluo> endless work!
14:56:26 <ihrachys> https://review.openstack.org/#/c/544206/ "Integration of (Distributed) Port Binding OVO"
14:56:38 <ihrachys> is it the patch we reverted a while ago? or was it a different one?
14:56:56 <lujinluo> it is the one we reverted
14:57:05 <lujinluo> now it is hit by the same bug
14:57:18 <lujinluo> cause i need to switch portbinding to new engine facade..
14:57:45 <ihrachys> ok, I see. we need a generic solution here, switching it across the board is hard and unsafe
14:57:58 <lujinluo> yes
14:58:24 <ihrachys> also, InvalidRequestError handling in _detach_db_obj is probably not needed with the new_engine patch since it added a wrapped subtransaction there
14:59:55 <lujinluo> i am not fully sure, but in test_base iface class is not working correctly (the one we want to refactor). i think i added invalid* to mitigate that. after the refactor, invalid* would not be necessary
15:00:28 <ihrachys> ok we need to have a look at specific cases. but just adding that handler is probably unsafe.
15:00:33 <lujinluo> https://github.com/openstack/neutron/blob/master/neutron/tests/unit/objects/test_base.py#L685 this class i mean
15:00:43 <lujinluo> true. will remove in next PS
15:01:18 <ihrachys> lujinluo, the class already mocks out expunge / refresh so I am not sure how one would get an error from there
15:01:30 <ihrachys> see lines 695-696
15:02:16 <ihrachys> ok let's revert this part. and think about generic solution to auto-detect engine facade version
15:02:19 <lujinluo> in my local test, f.e. https://github.com/openstack/neutron/blob/master/neutron/tests/unit/objects/test_base.py#L1007 is still calling expunge()
15:02:23 <ihrachys> we are out of time
15:02:28 <lujinluo> sure
15:02:47 <ihrachys> thanks for joining
15:02:47 <ihrachys> #endmeeting