14:03:05 <ihrachys> #startmeeting neutron_upgrades
14:03:05 <openstack> Meeting started Thu Jan 18 14:03:05 2018 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:03:06 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:03:08 <openstack> The meeting name has been set to 'neutron_upgrades'
14:03:14 <ihrachys> hey!
14:03:36 <lujinluo> o/
14:03:42 <hungpv_> Hi
14:03:46 <TuanVu> hi Ihar
14:03:49 <TuanVu> hi Luo
14:04:27 <ihrachys> https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
14:04:48 <ihrachys> before we go through patches on review, I wonder why port binding one - https://review.openstack.org/#/c/407868/ - is still not in
14:05:02 <ihrachys> ok now some timeouts in grenade
14:05:11 <lujinluo> yeah..
14:05:15 <ihrachys> that's kinda concerning that we can't get it in. is there maybe some bug or smth.
14:05:41 <lujinluo> that's what worrying me. maybe i should at lease rebase it?
14:05:53 <ihrachys> ah no seems like most latest failures are known. the last timeout on grenade is new to me but it doesn't surprise me.
14:06:15 <ihrachys> there is something infra is doing these days that make landing patches impossible. restarting zuul etc.
14:06:23 <ihrachys> rebase does nothing
14:06:35 <ihrachys> because zuul merges your tree with latest master before testing it out
14:06:44 <ihrachys> and if it will fail to merge, it will say so
14:06:53 <lujinluo> i see. then maybe recheck again to see if we are blessed this time?
14:07:33 <ihrachys> so re: latest grenade failure:
14:07:34 <ihrachys> 2018-01-18 03:48:28.179626 | primary | 2018-01-18 03:48:28.179 | Running base stack.sh
14:07:34 <ihrachys> 2018-01-18 06:31:50.469578 | primary | /home/zuul/workspace/devstack-gate/functions.sh: line 976:  9348 Killed                  timeout -s 9 ${REMAINING_TIME}m bash -c "source $WORKSPACE/devstack-gate/functions.sh && $cmd"
14:07:45 <ihrachys> so it started to deploy stack and failed right there
14:07:54 <ihrachys> didn't even get to tests or upgrade
14:08:04 <ihrachys> so it definitely is not about your patch
14:08:10 <ihrachys> because it never rolled in new code
14:08:21 <ihrachys> yeah let me recheck
14:08:34 <lujinluo> understood. thanks!
14:08:46 <ihrachys> but yeah that's sad
14:09:02 <ihrachys> anyway. looking at patches up for review now.
14:09:13 <ihrachys> https://review.openstack.org/#/c/506037/ "Part II of Integrate Port OVO"
14:09:23 <ihrachys> I noticed you respinned it
14:09:30 <ihrachys> is it ready for review now?
14:10:14 <lujinluo> i just checked the failed tests. they are still related to the transition but review would be helpful atm
14:10:17 <ihrachys> there are some failures in api tests, and traces here: http://logs.openstack.org/37/506037/15/check/neutron-tempest-plugin-api/2b646cb/logs/screen-q-svc.txt.gz?level=WARNING
14:10:27 <ihrachys> ok I will eye ball it
14:10:48 <lujinluo> thanks ;)
14:11:45 <ihrachys> then we have https://review.openstack.org/#/c/507772/ "Use Network OVO in db_base_plugin"
14:12:12 <TuanVu> The good news is: “All the functional tests have been passed”
14:12:21 <TuanVu> We’re working on 12 remaining failed unit tests
14:12:41 <ihrachys> yeah, I see you reduced the number of failures a lot!
14:12:50 <TuanVu> thank you, Ihar :)
14:13:25 <TuanVu> thanks to the great support from you and Slawek
14:14:08 <ihrachys> drop me another email or ping me in irc when those are fixed
14:14:53 <ihrachys> ok, next is https://review.openstack.org/#/c/521797/ "Use Router OVO in external_net_db"
14:15:15 <TuanVu> thank you, sure
14:16:18 <hungpv_> Hi Ihar, I restored the previous logic and add a UT for it
14:16:18 <ihrachys> I think it was close to completion the last time
14:17:42 <ihrachys> I see. maybe you reverted a bit too much. for example, consolidating arguments - projects and new_project - into a single one
14:17:50 <ihrachys> I will walk through the patch again today
14:18:36 <hungpv_> The problem here that I haven't figured out an effective way to blend them
14:18:59 <hungpv_> If possible, please leave some suggestions
14:19:08 <ihrachys> hungpv_, isn't just having a single list of all of them and passing it to ~ query operator do the same as we do now?
14:19:55 <ihrachys> what query does is first look for objects with project_id not in 'projects'
14:20:08 <ihrachys> then it also filters out anything with 'new_project'
14:20:31 <ihrachys> I don't get why we can't just filter out projects + new_project in the first step and avoid the second one
14:20:55 <hoangcx_> ihrachys: AFAIK, projects is db record and new_project is uuid. How we can consolidate them in a list?
14:21:45 <ihrachys> no 'projects' are just list or set of strings
14:21:49 <ihrachys> (list)
14:22:02 <ihrachys> at least if you restore:
14:22:02 <ihrachys> tenants_with_entries = [tenants_with_entry[0]
14:22:03 <ihrachys> for tenants_with_entry
14:22:03 <ihrachys> in tenants_with_entries]
14:22:26 <ihrachys> neutron doesn't even have a model for projects in its database
14:23:18 <hoangcx_> https://review.openstack.org/#/c/521797/23/neutron/db/external_net_db.py
14:23:50 <hoangcx_> L234 in new code not return that result is not .all()
14:24:06 <hoangcx_> s/is not/if not
14:24:38 <ihrachys> hoangcx_, well, do .all() (that's same as the tenants_with_entries calculation above)
14:26:04 <hoangcx_> ihrachys: https://review.openstack.org/#/c/521797/23/neutron/objects/router.py
14:26:10 <ihrachys> I will have a look. I want to play with the UT you added to see if it triggers all the code we touch.
14:26:30 <hoangcx_> you mean L221 -L222 need a for loop above?
14:26:45 <ihrachys> hoangcx_, no you had it in PS21
14:26:48 <ihrachys> https://review.openstack.org/#/c/521797/21..23/neutron/db/external_net_db.py
14:26:55 <ihrachys> see to the left, line 236
14:27:14 <ihrachys> you were extracting project_id strings from query, then passing them as a list of strings into OVO method
14:27:19 <ihrachys> which is the right way to do
14:27:50 <hoangcx_> ihrachys: yeah
14:28:05 <ihrachys> currently you pass a query object there instead, which is not great for isolation of concerns between sqlalchemy and OVO layers. OVO layer shouldn't accept, or return, anything related to SQLAlchemy
14:28:12 <ihrachys> you had it correct in PS21
14:28:21 <hoangcx_> ihrachys: then in OVO method https://review.openstack.org/#/c/521797/23/neutron/objects/router.py
14:28:56 <hoangcx_> We need check for all item in list of consolidated projects?
14:29:05 <ihrachys> then in OVO method, you just do lines 218-222 and you don't need lines 223-225
14:29:11 <ihrachys> because new_project is already in projects
14:29:59 <hoangcx_> the problem here is that how L222 work with that?
14:30:21 <ihrachys> what's about 222
14:30:44 <ihrachys> it takes a list of project_id strings in and it filters out all db models that have any of those project_ids
14:30:48 <ihrachys> ~ means NOT
14:31:22 <hoangcx_> But in original code it is not a strings
14:31:39 <hoangcx_> You can set a break point to see that.
14:31:41 <hungpv_> So we can keep that query and just add new_project to list of projects?
14:32:18 <ihrachys> sure. because it constructs a query in a single place. but you now have two places where queries happen, and we better not pass queries across the boundary
14:32:32 <ihrachys> yes, add new_project to the list
14:32:38 <ihrachys> you already had it!
14:33:07 <ihrachys> in PS21
14:33:20 <ihrachys> anyway, I think we should wrap up and move on. I will comment on the patch later.
14:33:35 <hoangcx_> ++
14:33:51 <ihrachys> ok next is https://review.openstack.org/#/c/530182/ "Use Router OVO in l3_db"
14:34:43 <ihrachys> I walked through it yesterday, left some comments, most of them basically are - 'there are more occurrences of using db models in the module, some added as part of this patch, and we should expand the patch to remove them too'
14:35:06 <lujinluo> agree
14:35:06 <hoangcx_> I have also looked this patch today and it seems not ready in general at the moment.
14:35:14 <ihrachys> there are also some logical issues there
14:35:18 <ihrachys> yeah, it needs more work
14:35:54 <hoangcx_> The purpose of our work it to convert current direct db query to use OVO. But the patch seems not
14:36:47 <ihrachys> it did some progress but left a lot of things intact, so now it passes .db_obj around etc. Which shouldn't happen except in some specific places where we interact with existing extensions / plugins / callbacks
14:37:04 <ihrachys> and even that should eventually change
14:37:41 <ihrachys> ok moving on
14:37:55 <ihrachys> https://review.openstack.org/529551 "Use Router OVO in metering_db"
14:38:14 <ihrachys> I see this patch has a long history
14:38:20 <ihrachys> we even +W it at some point
14:38:36 <hoangcx_> This patch just need re-approval
14:39:13 <ihrachys> does it depend on some other patch?
14:39:17 <hungpv_> Yes, but nothing changed from +W
14:39:27 <hungpv_> No, it doesn't
14:39:48 <hungpv_> I made a mistake and now it's corrected
14:39:54 <ihrachys> ok you just detached it
14:40:03 <ihrachys> great
14:40:28 <hungpv_> Yes, I think it can be safely merged
14:40:39 <ihrachys> ok I pushed it in gate
14:40:41 <ihrachys> thanks!
14:41:00 <hungpv_> Thank you!
14:41:01 <hoangcx_> ++
14:41:04 <ihrachys> other patches in review queue seem to be not ready
14:42:27 <ihrachys> any other patches to look through?
14:42:38 <ihrachys> or topics to discuss?
14:42:50 <lujinluo> none from me
14:43:16 <hungpv_> No for me
14:43:38 <ihrachys> ok good.
14:43:46 <ihrachys> we can continue in gerrit
14:43:59 <ihrachys> thanks for joining, you can have your 17 minutes back. enjoy the evening!
14:44:07 <ihrachys> #endmeeting