14:00:43 #startmeeting neutron_upgrades 14:00:44 Meeting started Thu Aug 24 14:00:43 2017 UTC and is due to finish in 60 minutes. The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:00:45 lujinluo, hi 14:00:45 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 14:00:47 The meeting name has been set to 'neutron_upgrades' 14:00:53 manjeets, is off today 14:00:55 ihrachys: o/ 14:00:58 Hi everybody 14:01:01 i see 14:01:05 TuanVu, hey! 14:01:38 we have no action items from prev meeting. I think we can just run through patches up for review. 14:01:39 Hi 14:01:43 annp_, hey! 14:01:49 Hi ihrachys 14:02:00 Hi ihrachys. :) 14:02:32 #topic OVO patches 14:02:48 https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db 14:03:08 Floating IP: https://review.openstack.org/#/c/396351/ 14:03:24 I have a question regarding floating ip 14:03:31 i met check policy failure 14:03:40 because of the absence of tenant_id 14:03:56 where is the error? 14:04:12 e.g http://logs.openstack.org/51/396351/28/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/0e31319/console.html#_2017-08-24_09_10_47_777599 14:04:39 It ha in API layer, as I observed 14:04:44 It is* 14:05:27 lujinluo, we probably return a dict from plugin to api layer that doesn't have tenant_id in it 14:05:55 yes, but how can we mitigate that? i mean we switched from tenant_id to project_id in ovo 14:06:15 there is no longer tenant_id in floating ip ovo 14:06:21 I think we still return tenant_id (in addition to project_id) in dicts generated with .to_dict() 14:06:30 i see. 14:06:42 i will add tenant_id to .to_dict() then 14:07:08 I think it should already be there 14:07:15 sec 14:07:32 it is possible that i deleted it, not 100% sure though 14:08:04 this is generic implementation for tenant_id field: https://github.com/openstack/neutron/blob/master/neutron/objects/base.py#L244-L247 14:08:39 and this, I think, should add tenant_id to the dict: https://github.com/openstack/neutron/blob/master/neutron/objects/base.py#L121-L125 14:09:33 i did not modify base.py, then they are not working as expected..? 14:10:41 for what I understand, the request that fails is to get floatingip 14:10:49 which should be get_floatingip in plugin code 14:11:41 this: https://github.com/openstack/neutron/blob/master/neutron/db/l3_db.py#L1390-L1393 14:12:03 in your patch, you changed the type of result of _get_floatingip to an OVO object 14:12:13 (before the patch, it was a sqlalchemy model) 14:12:31 Ahh, i see. https://review.openstack.org/#/c/396351/28/neutron/db/l3_db.py@1037 i should change this line as well 14:12:36 that being said, _make_floatingip_dict should have correctly extract tenant_id: https://github.com/openstack/neutron/blob/master/neutron/db/l3_db.py#L1036 14:13:11 lujinluo, yeah, I am not sure we implement __getitem__ for tenant_id 14:13:29 it should be floatingip['project_id']. thanks, i will double check tmr 14:13:34 cool 14:13:52 next is https://review.openstack.org/#/c/495810/ "Use Agent OVO in agents_db and test_agents_db" 14:14:17 it's pretty red 14:14:25 hi, we're still working on it 14:14:34 I and Tuan just working on it 14:14:46 we have issue with _get_dict 14:14:57 ack, we'll look closer when it's ready 14:15:10 next is https://review.openstack.org/#/c/370452/ "OVO for NetworkDhcpAgentBinding" 14:15:19 hi Ihrachys 14:15:25 could you please wait 14:15:43 TuanVu_, eh, sure. what's up? 14:15:54 we have one concern and hopefully you could help 14:16:13 yes, regarding to _get_dict 14:16:49 TuanVu_, which patch do we talk about, agent one? 14:16:55 https://review.openstack.org/#/c/495810/5/neutron/db/agents_db.py 14:16:59 yes 14:17:10 line 178 14:17:42 for this function 14:17:44 conf = jsonutils.loads(json_value) 14:18:03 we cannot load json_value 14:18:21 it looks like there's problem with unicode string 14:18:41 have you checked that json_value is a legit JSON string? 14:18:45 therefore, there are several tests failed and we still couldn't figure out how to fix it 14:19:37 is is because in OVO, "configuraton" is DictOfMiscValuesField? 14:20:00 hmm, we've tried to dump it, and it looks legit 14:20:16 could you please clarify more, Ihra and Luo ? 14:20:21 TuanVu_, I think lujinluo is onto smth here. maybe the attribute you extract is already a dict 14:20:30 so you may not need to do .loads() 14:20:47 have you checked its type? 14:20:59 what do you think, An-san? 14:21:03 https://review.openstack.org/#/c/411830/ this patch declares the type pf "configurations" 14:21:19 TuanVu_, f.e see https://review.openstack.org/#/c/495810/5/neutron/objects/agent.py@75 14:21:36 sorry, i just lost connection. :) 14:21:53 let me see 14:22:16 thank you, Luo and Ihra, I'm also checking 14:23:01 I will try with ihrachys's suggestion tomorrow, 14:23:09 Please go ahead 14:23:13 ok, thanks 14:23:21 next is https://review.openstack.org/#/c/370452/ "OVO for NetworkDhcpAgentBinding" 14:23:34 I already +2d it, we need 2nd opinion to merge 14:24:11 moving on. next is https://review.openstack.org/#/c/361443/, "OVO for L3HARouter" 14:24:42 I need to repeat review for the patch, I see TuanVu_ respinned it yesterday 14:24:51 yes, I did 14:25:16 thanks for your insightful review, Ihrachys 14:25:30 I see some discussion here: https://review.openstack.org/#/c/361443/46/neutron/objects/l3_hamode.py@53 give me a second to read it 14:26:23 ok 14:26:52 ok I see what's going on there. I think it's fine to leave it as-is. I would suggest we leave a TODO here to get back to it, but it's not a must. 14:27:15 I think it's the right thing not to try to unravel the whole stack of interdependencies in one go\ 14:27:32 ok then 14:27:51 next is https://review.openstack.org/#/c/429829/, "Allow Agent object to be queryable by dict's key field" 14:28:08 TuanVu_, you respinned it 14:28:20 I wonder whether it's still needed 14:28:39 the patch is quite old, not sure it applies now. do we have some code that would benefit from it? 14:28:54 i went through the unittest of agent, and it seems we do not need it anymore 14:29:27 yeah, I've checked that too 14:29:47 ok, then let's abandon? we can always revive if needed. 14:29:51 but since being a newbie, I'm not sure if we still need it 14:30:17 ok, no problem 14:30:45 I'll abandon this patch 14:30:49 thanks! 14:31:08 there are no more patches in the review queue that have new developments 14:31:36 any patch from those I haven't mentioned that you would like to discuss? 14:32:08 ihrachys: thanks for changing the topic of Port patch! I always forgot to do it ;) 14:32:14 Hi 14:32:20 I have one question 14:32:23 TuanVu_, shoot 14:32:52 about the strategy for updating patches for Agent (eg: the order of files to be updated) 14:34:39 TuanVu_, what's the question? 14:36:01 have we lost TuanVu_ ? 14:36:13 after working on "agents_db", I realize that when continue working on other files (eg: l3agentscheduler), there are lots of changes need to be done on agents_db 14:36:40 sorry, I'm trying to explain my concern in a simple way 14:36:54 TuanVu_, np, I thought that's connectivity issues 14:37:12 TuanVu_, so in general, I think it's fine to split patches as much as to simplify review / reduce their size 14:37:17 it's easier to land small patches 14:37:30 yes, I totally agree with that idea 14:37:46 there is no requirement that you switch all uses of a model in one go 14:37:51 but regarding to the order of file to be updated 14:38:33 how does the order play out in this particular case? 14:38:54 especially for Agent OVO which is pretty big and complicated, do you have any suggestion for "the order of files to be updated" 14:39:07 eg: which one first, then what is the next one 14:39:27 yes, the order plays a pretty big deal 14:39:42 there are two files at play, agents_db and l3agentscheduler? or there are more? 14:40:35 well, there are lots of files need to be updated for Agent OVO (about 10, I think) 14:40:58 https://review.openstack.org/#/c/495810/ 14:41:32 after above patch, I'm about to move on to l3agentscheduler 14:41:49 and when working with l3agentscheduler, I'll need to update agents_db again 14:42:04 so that's the problem I would like to mention here 14:42:13 ok. I don't have specifics to give very specific answer, but what I would do is I would try to take the smallest bit you can tackle from one of those files [f.e. a single method], try to migrate it to OVO and see where it leads you (and do the minimal necessary change in other files to adopt to the change and pass CI without touching other code) 14:42:38 TuanVu_, oh, you mean you want to base your scheduler work on that other patch? 14:42:40 I mean, looks like it would be better to update agents_db at the end 14:42:51 if so, you can stack them in git 14:43:51 thank you, Ihar 14:43:59 I got your idea :) 14:44:17 TuanVu_, in the agents_db patch, if the only remaining issue is with json.loads() and it will be ready to merge, I think that's what we should concentrate on 14:44:17 "I would try to take the smallest bit you can tackle from one of those files [f.e. a single method], try to migrate it to OVO and see where it leads you (and do the minimal necessary change in other files to adopt to the change and pass CI without touching other code)" 14:44:33 but in the meantime, you can work on scheduler patch, just make sure you base it on the agents_db one 14:45:02 yes, thanks for your advice 14:45:08 I really appreciate it :) 14:45:16 you do it with: git review -d 495810 ; ; git commit -a -m "blablabla"; git review 14:45:41 the first command (git review -d ...) will download the state of git tree with the agents_db patch on top 14:45:51 yeah, I got your idea 14:46:10 so when you execute the last command (git review), it will send both patches, but the first is the same, so it is not affected 14:46:15 ok, cool 14:46:32 I guess that's all we had for today unless someone has anything more to discuss 14:46:43 none from me 14:46:51 me too 14:46:57 same here 14:47:01 TuanVu_, annp_, lujinluo thanks for all the effort 14:47:10 #endmeeting