14:02:13 <ihrachys> #startmeeting neutron_upgrades
14:02:14 <openstack> Meeting started Thu Feb  1 14:02:13 2018 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:02:16 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:02:18 <openstack> The meeting name has been set to 'neutron_upgrades'
14:02:27 <lujinluo> o/
14:02:32 <ihrachys> hi lujinluo
14:02:39 <lujinluo> hi ihrachys
14:02:40 <TuanVu> Hi everybody
14:03:06 <ihrachys> o/ TuanVu
14:03:56 <ihrachys> we are in freeze mode for master right now, nothing except patches explicitly targeted for queens-rc1 and gate fixes land
14:04:07 <ihrachys> also, the gate is afaik unstable
14:04:31 <ihrachys> I was distracted by some pre-release activities and lagged on reviews for OVO, sorry for that
14:04:52 <ihrachys> let's see what we have here though: https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
14:05:21 <ihrachys> first being https://review.openstack.org/#/c/506037/ "Part II of Integrate Port OVO"
14:05:42 <ihrachys> I believe port work is stalled because of port bindings. or at least the plan was to get back to bindings and complete it first, correct?
14:05:51 <lujinluo> correct
14:06:02 <ihrachys> I don't see the revert of the revert proposed (at least not in the topic)
14:06:21 <lujinluo> i was revising port binding locally, not ready for review yet
14:06:47 <ihrachys> ok. do you have an idea what was wrong with postgres there?
14:07:15 <lujinluo> sadly not yet. the postgresql backend devstack is not working properly
14:07:19 <lujinluo> i need more time
14:07:52 <ihrachys> sure
14:08:17 <ihrachys> another issue there was mixed engine facades usage introduced by the patch
14:08:43 <lujinluo> speaking of that, i found an old patch by ann to change ovo base to new engine facade
14:09:02 <ihrachys> lujinluo, yeah there was one at some point. have a link handy?
14:09:06 <lujinluo> would that be one of the options we can use to mitigate the incompatibility?
14:09:11 <lujinluo> let me check
14:10:26 <ihrachys> lujinluo, yeah, we either replace with new facade, or completely remove it from there, but the latter would require more changes, and probably better to be left for later, if at all done.
14:11:19 <ihrachys> lujinluo, the problem to solve is that this is base class, so we would need to change it everywhere where OVO objects are used in one go.
14:11:20 <lujinluo> https://review.openstack.org/#/c/403870/ this one
14:11:34 <ihrachys> thanks
14:12:03 <ihrachys> yeah that's the right path - what the patch attempts to do
14:12:40 <ihrachys> lujinluo, are you going to respin the patch?
14:12:59 <lujinluo> i think i need to check with Ann first, if she would continue or not
14:13:05 <lujinluo> if not, i think i can respin it
14:13:26 <ihrachys> yeah. I don't think she was working on the blueprint for awhile but better check
14:13:45 <lujinluo> will add that to my to-do list ;)
14:14:17 <ihrachys> mlavalle was also saying he will look into completion of engine facade switch blueprint, so maybe you could co-operate
14:14:45 <ihrachys> and since you afaiu go to Dublin, you could hash out some details there
14:15:11 <lujinluo> ok, thanks! i will check that with Miguel too
14:16:01 <ihrachys> great
14:16:16 <ihrachys> next is https://review.openstack.org/#/c/537325/ "Use Meter Label OVO in neutron/db/metering/metering_db.py"
14:17:05 <ihrachys> I believe that's one patch that I failed to review since last week (at least it's already in my todo list)
14:17:47 <lujinluo> yeah, you said you were afraid of fetching twice from db
14:18:22 <ihrachys> you solved that I guess?
14:18:43 <lujinluo> sadly no...
14:19:39 <ihrachys> ok I will have a look, maybe it's just too complex to do it
14:20:00 <lujinluo> maybe, but please take a look
14:21:00 <ihrachys> ack
14:21:48 <ihrachys> next is "Use Port OVO in neutron/db/external_net_db.py" https://review.openstack.org/#/c/537320/
14:22:39 <ihrachys> lujinluo, you are saying there get_objects don't explicitly use engine facade
14:23:06 <ihrachys> but: https://github.com/openstack/neutron/blob/master/neutron/objects/base.py#L556
14:24:02 <lujinluo> yes, this is the old style db engine but it is not nested session. this is what confuses me
14:24:40 <lujinluo> previously, it is nested transaction and new engine facade cannot co-exist, right?
14:24:45 <ihrachys> it opens subtransaction using old method, using old facade
14:24:57 <ihrachys> subtransactions=True == nested actually
14:25:05 <ihrachys> it's just same thing written in a different way
14:25:27 <lujinluo> i see. i will hold that patch back before we solve the co-existance issue
14:26:07 <ihrachys> right. that's why it's so tricky to switch to new facade for OVO: if you change it in get_objects trying to fix it for e.g. Ports, you also change it for all other OVOs and need to switch to new facade in the same patch
14:26:30 <ihrachys> we could probably do something smart like - put a tag on an object saying 'use new facade'
14:26:42 <ihrachys> and then base class would do it just for this new object
14:26:48 <ihrachys> and then switch them one by one
14:27:01 <ihrachys> that would reduce the scope of each patch, easier to land then
14:27:06 <ihrachys> what do you think
14:27:34 <ihrachys> at some point when we switched everything we would remove the tag feature and use just the new one
14:27:53 <ihrachys> tag == class attribute
14:28:15 <lujinluo> hmm, i thought we would be changing base ovo to new engine facade once and for all, no?
14:29:21 <ihrachys> lujinluo, well yes, but do we want to have a huge patch doing all the things at once (and struggle to merge it because of all conflicts and lack of reviews), or split it?
14:29:44 <ihrachys> if we can do the migration piece by piece I think that would be a good thing
14:30:08 <lujinluo> ihrachys: i see your point.
14:30:14 <lujinluo> smaller patches are better
14:31:40 <ihrachys> ok let's figure out if it's easy to do it this way. otherwise I suspect that the patch Ann had would need to rise in size quite a bit to cover for all new OVO usage we introduced since it was initially written
14:32:36 <lujinluo> yeah, i will try the tag approach locally recent to see if it would work as we expect
14:32:39 <ihrachys> as for the patch in question, I will have a look at whether those two methods indeed are called from new context. if so, we should be able to merge after freeze.
14:33:19 <lujinluo> ok, thanks
14:34:10 <ihrachys> next is https://review.openstack.org/#/c/521797/ "Use Router OVO in external_net_db"
14:36:17 <ihrachys> based on latest comments, I guess I need to revisit test coverage there
14:36:59 <ihrachys> next is https://review.openstack.org/#/c/507772/ "Use Network OVO in db_base_plugin"
14:37:24 <TuanVu> I’m still fixing “queries_constant” unit tests.
14:37:25 <TuanVu> After removing “reload shared attribute” parts (I thought this is the only place where it’s gone wrong), the tests are still failing.
14:37:36 <TuanVu> 
14:37:39 <TuanVu> So at this moment, I’m trying to find other places where the number of queries is increased.
14:37:40 <TuanVu> I just need more time to debug the code, so no question at this moment yet.
14:37:54 <ihrachys> ack
14:38:12 <ihrachys> yeah those queries counting tests are usually tricky
14:38:50 <TuanVu> yes, hopefully I can solve it soon
14:39:05 <TuanVu> if there's any problem, may I ask you via email, Ihar?
14:39:10 <ihrachys> next is "Use Router OVO in l3_db" https://review.openstack.org/#/c/530182/
14:39:21 <ihrachys> this one still sits on pending comments
14:39:28 <ihrachys> TuanVu, sure!
14:39:37 <TuanVu> thank you in advance, Ihar :)
14:39:49 <ihrachys> anyone knows xujun?
14:40:02 <lujinluo> no
14:40:08 <ihrachys> I've never seen xujun in irc
14:40:12 <TuanVu> me either
14:40:47 <hungpv_> Maybe he's blocked in China?
14:41:14 <lujinluo> hungpv_: well, I do not think China blocks IRC though
14:41:15 <ihrachys> what do you mean blocked
14:41:28 <lujinluo> like Google services
14:41:54 <ihrachys> oh right, the Great Firewall
14:42:44 <ihrachys> I asked xujun in gerrit whether she (he?) is going to respin it, or we could take it over
14:43:43 <hungpv_> I'm still working on it. If there's no update from them, maybe we'll take over?
14:43:57 <ihrachys> other patches in the query are old / in conflicts.
14:45:18 <ihrachys> hungpv_, the author respinned the patch two weeks ago
14:46:23 <ihrachys> hungpv_, I think you could e.g. help them with suggestions as comments + upload your diff on top of their patch / in a parallel work-in-progress patch so that they can incorporate your changes / suggestions into their patch
14:47:34 <hungpv_> Yes, I'll be working more in this and give my update to their patch
14:48:13 <ihrachys> ok
14:48:20 <ihrachys> any other patches to discuss?
14:48:41 <lujinluo> none from me
14:49:39 <ihrachys> neither from me
14:49:55 <TuanVu> Neither am I
14:50:14 <ihrachys> I guess we can close the meeting then. thanks folks.
14:50:19 <ihrachys> #endmeeting