17:00:17 #startmeeting service_chaining 17:00:17 Meeting started Thu Oct 8 17:00:17 2015 UTC and is due to finish in 60 minutes. The chair is cathy_. Information about MeetBot at http://wiki.debian.org/MeetBot. 17:00:18 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 17:00:20 The meeting name has been set to 'service_chaining' 17:00:25 o/ 17:00:26 ji 17:00:27 hi everyone 17:00:27 hi 17:00:28 hi 17:00:42 hi 17:01:39 hello 17:01:47 OK let's start 17:02:03 #topic Separate flow classifier API and DB code patch out of the SFC API and DB code patch 17:02:27 hi 17:02:34 hi 17:02:34 amotoki: hi 17:02:46 any thought on this? 17:03:28 cathy_: do we want to instead post the patch to Neutron? 17:03:29 cathy_ : can you explain the background for this change ? 17:03:58 cathy_: are you thinking about the common classifier? 17:03:59 If we separate the classifier code patch, then it will be easier to move this code to Neutron core if needed and for othere modules to leverage this code later 17:04:00 cathy_: or we separate in case that we are going to move that from networkiing-sfc to Neutron in the future? (but it would remain in networking-sfc for now) 17:04:22 s3wong: We can't get the same code in neutron.. 17:04:46 LouisF: potential for that in the future 17:05:02 current proposal is to introduce it as a core extension and not as a service plug-in 17:05:10 vikram: really? how so? 17:05:29 I think the current proposed code has a separate flow classifier extension and db code, so I think we can easily migrate to a new one once general flow classifier feature lands in neutron. 17:05:30 https://review.openstack.org/#/c/190463/ 17:06:01 amotoki, cathy_: I agree we can port most of the changes 17:06:01 amotoki: yes, agree. 17:06:26 amotoki, cathy_: but need some changes as well 17:07:06 vikram: I was under the impression that armax has commented on that flow classifier vs flow classifier in networking-sfc in an email to ML? 17:07:06 amotoki: vikram so for that easy porting, it seems better to separate the code patch out for review now 17:07:40 vikram: yes, may need some changes or extension, but most of the meaty part will be the same 17:07:42 s3wong: do you have a pointer to the thread? 17:07:54 amotoki, cathy_: but need some changes as well? 17:07:59 s3wong: any link? 17:08:04 vikram: yes 17:08:26 amotoki, vikram: looking... 17:08:27 cathy_: +1 for moving flow_classifier changes out 17:08:37 vikram: OK, good 17:09:33 how other's think about this change? 17:09:50 i think what we need to do when flow classifer from neutron core is available are to remove the extension from SFC service plugin supported_exnteison_list and consume methods from common flow classifier. 17:09:57 s3wong: If I remember correctly, the comment is about the putting the BP proposed by Yuji in "abandoned" state for that time. 17:10:16 I am not sure which is better. I think we still need time to land the common classifier discussion. 17:10:38 amotoki: agree. but the code developed here can be leveraged 17:10:42 cathy_: IIRC yuji resumed his one. 17:11:01 amotoki: but a separate patch will be helpful to port the code later 17:11:32 cathy_: yes, I remember that as well, but still searching for the email... 17:11:34 vikram: cathy_: I see. I got your point. you are suggesting separate gerrit reviews. 17:11:43 OK, so separate it out will be helpful, but how to do the common classifier is another topic which needs more discussion 17:11:45 amotoki: +1 17:12:04 cathy_: +1 17:12:38 +1 17:13:06 s3wong: mohankumar johnsom pcarver_ : OK with you? 17:13:16 yes, +1 17:13:20 vikram: for now can you create a separate patch for the flow classifier? 17:13:23 yes +1 17:13:23 cathy_: +1 17:13:26 +1 17:13:33 LouisF: Sure! 17:13:35 LouisF: I can do this 17:13:37 cathy_: do you want to split the patches? 17:13:41 cathy_ 17:13:45 i can help vikram 17:13:50 cathy_: go ahead 17:14:25 #agreed Separate flow classifier API and DB code patch out of the SFC API and DB code patch 17:14:29 amotoki, vikram, cathy_: found it: http://lists.openstack.org/pipermail/openstack-dev/2015-July/070849.html] 17:14:43 #action Cathy Separate flow classifier API and DB code patch out of the SFC API and DB code patch 17:15:24 #Patches Ready for merge discussion 17:15:35 #topic Patches Ready for merge discussion 17:15:43 s3wong: yes its about sfc fc vs. common classifier 17:16:11 s3wong: I think the discussion was about packet forwarding ... Yuji had a separate patch for tah 17:16:23 *that.. not same as flow classifer 17:17:39 it seems the following patches are ready for merge: Realizing SFC (1b/5), Realizing SFC (2/5), Refactored documents. 17:17:54 #link https://review.openstack.org/#/q/status:open+project:openstack/networking-sfc+branch:master+topic:networking-sfc,n,z 17:18:15 pcarver_: do still have a problem with https://review.openstack.org/#/c/229777/ 17:18:29 pcarver_: do you? 17:18:42 LouisF: I did as of last time I tried it. Has there been a new PS? 17:19:06 I found that if I deleted the pylint line from test-requirements.txt I get a successful tox run 17:19:08 pcarver_: vikram retested the pep8 and it was ok 17:19:16 LouisF: pcarver_ I see email from Vikram saying he does not have problem 17:19:17 pcarver_: I tried before the meeting.. It worked without any issue 17:19:47 I'll try it again, but I tried it on a freshly built Ubuntu 14.04 VM twice and got the same issue consistently 17:20:17 pcarver_: It would be great if you can post your error and details 17:20:27 can you check if the pylint line is in your test-requirements.txt? 17:21:15 the file isn't part of the patch, it's part of a freshly cloned networking-sfc repo 17:21:17 pcarver_: it's there 17:21:29 pylint==1.4.4 # GNU GPL v2 17:21:34 line no 22 17:22:00 I'll rerun and post the full output to a pastebin 17:22:10 pcarver_: let's take this offline.. ;) 17:22:16 ok 17:22:47 For this patch, let's wait for this issue to be resolved before merging it. 17:23:28 cathy_: +1.. let's figure out hwy pcarver_ facing issue.. 17:23:28 I'd be ok with merging it if other folks are getting a successful tox run on a clean clone and fetch 17:23:51 How about the other two patches? I have gone through them and they look good to me. OK for merge? Any -1? 17:24:16 anyway's i re-triggered CI 17:26:28 If anyone has an issue with the other two patches, please shout here :-) , otherwise I will approve them for merge. 17:26:35 https://review.openstack.org/#/c/227074/ looks good 17:26:52 cathy_: we do have to wait for 1b to merge before merging 2 though, right? 17:27:05 cathy_: do you have the link handy? 17:27:10 https://review.openstack.org/#/c/229765/ also 17:27:34 both looks good to me 17:27:51 vikram: I posted the link before. Here it is https://review.openstack.org/#/q/status:open+project:openstack/networking-sfc+branch:master+topic:networking-sfc,n,z 17:28:08 amotoki: LouisF OK, thanks for the input 17:28:11 cathy_: thanks.. 17:28:29 s3wong: let me check the dependency 17:28:37 cathy_, LouisF: yeah, the migration patch looks good 17:29:52 I'm ok with the migration patch, although this whole Alembic business seems questionable to me. But if it's conssistent with the rest of OpenStack I guess we have to live with it. 17:30:20 I just don't like the idea of database schemas being defined in two places and relying on code review to visually check consistency 17:30:22 pcarver_: a can of worms you don't want to open :-) 17:30:53 s3wong: well, not right now anyway. I don't mind opening cans of worms at the appropriate time. 17:30:56 pcarver_: we have check-migration in pep8 target. 17:31:06 it is done in most neutron related projects. 17:31:37 we can improve it. 17:31:53 s3wong: so you are OK with merging the two patches, right? 17:32:19 regarding 229777, I created a fresh pep8 env now. "tox -r -e pep8" and then run ".tox/pep8/bin/pylint --rcfile=.pylintrc --output-format=colorized networking_sfc/**/*.py". It succeeded. 17:32:21 cathy_: migration I am OK... 1b we have to wait for pcarver_'s OK (that it works installing on ubuntu) 17:32:44 amotoki: great 17:32:59 s3wong: I am referring to Realizing SFC (2/5), Refactored documents. 17:33:19 I'm probably going to +2 the 1b patch later today. I just got a clean tox run on a bare metal Ubuntu box. I want to run it once more on a VM and see if I can identify why I was getting failures. 17:33:20 cathy_: I think we can merge 1b/5 as well 17:33:44 pcarver_: great 17:33:48 vikram: +1 17:34:00 vikram: OK 17:34:16 cathy_, pcarver_: I think it might be an env issue rather than code flaw ;) 17:34:18 the DependsOn for migration patch needs to be updated 17:34:36 on the other hand "tox -e pylint" failed. I think we can tackle this later. 17:34:47 s3wong: you mean the [OUTDATED]? 17:35:04 vikram: probably, but I'd like to identify what it is specifically because I built two fresh Ubuntu 14.04 VMs yesterday and repeated the issue on both. 17:35:22 pcarver_: ok 17:35:26 LouisF: yes 17:35:27 Simply deleting the one pylint line from test-requirements.txt resolved the error, but I don't know why 17:35:40 LouisF: (that's why I did 17:35:47 didn't +A) 17:36:10 vikram: could you update the dependency per s3wong's suggestion 17:36:43 s3wong: No need to worry.. 17:36:56 s3wong: Once the patch gets merged we can rebase 17:37:21 vikram: ok 17:37:36 vikram: sure 17:37:56 s3wong: no conflict .. so nothing to worry 17:38:43 #agreed merge the three patches: Realizing SFC (1b/5), Realizing SFC (2/5), Refactored documents. 17:39:14 I would suggest to remove "pylint" from deps in tox.ini rather than deleting pylint from test-reqs.txt 17:39:15 #topic review for the other SFC patches 17:39:30 will comment in the review. 17:40:14 amotoki: sure 17:40:27 amotoki: OK 17:40:52 amotoki: sure, thanks for the comment, vikram I will wait for you to address that and then approve it for merge. OK with you? 17:40:56 just commented. vikram, can I update it? 17:41:15 amotki: sure ;) 17:41:21 amotoki: sure ;) 17:41:39 I succeeded to run 'tox -e pylint' now. 17:41:49 once others confirm the success, we can go. 17:42:04 pcarver_: can you check also? 17:42:12 amotoki: That sounds like it'll resolve what I was seeing. I'll check it. 17:42:51 The error I was getting was about a duplicate of pylint. The error pointed to test-requirements.txt, but obviously a duplicate can be resolved by removing either of the duplications. 17:43:00 pcarver_: I uploaded the new patch set. try it! 17:43:34 amotoki: thanks! 17:46:07 I see vikram and mohankumar have posted new comments for the other patches 3/5, 4/5, 5/5. LouisF could you address them and upload new patches? 17:46:14 pcarver_: will do 17:46:27 amotoki: could you also review these patches? 17:46:57 I am looking at 3/5. 17:47:04 LouisF: Let me know if any help is needed 17:47:10 after that, I will visit later ones. 17:47:14 amotoki: thanks 17:47:17 amotoki: great. thanks. 17:47:23 vikram: ok 17:47:39 pcarver_: s3wong could you review these patches ? 17:47:57 cathy_: certainly 17:48:01 cathy_: I will. I'm swamped, but will make time to get through all of them. 17:48:20 great, thanks folks! 17:49:03 Let's try to squeeze time and give these good review so that we can move forward faster. 17:49:22 any other topic you would like to discuss? we have 11 minutes left 17:49:36 cathy_: how about the agent code 17:49:49 cathy_: when it could be ready 17:49:50 vikram: yes, I am going to post that code soon. 17:49:57 cathy_: ok.. 17:50:25 vikram: I can post them either this Friday or next Monday. 17:50:37 cathy_: ok.. 17:51:08 vikram: Still working on the UT codes for the agent to make them more comprehensive. 17:51:17 cathy_: You are going to separate the patch 3/5? 17:51:23 vikram: yes 17:51:26 cathy_: ok 17:51:34 cathy_: great 17:51:55 horizon changes are also coming great 17:52:16 mohan will post them soon 17:53:51 Horizon patch will up for review tomorrow without UT , as of now create , delete , list for all resources are tested with client ,sever code . Doing few update checks 17:54:01 vikram: great. So mohan will post an updated patch, right? 17:54:16 cathy_: yes 17:54:18 mohankumar: great. thanks. 17:54:26 mohankumar: very good! 17:54:41 mohankumar: great! 17:55:01 thanks :) 17:57:11 We have a good discussion. Thanks everyone! If nothing else, I will end the meeting. 17:57:12 3 minutes, is the meeting coming to a close? 17:57:20 s3wong: yes 17:57:21 bye 17:57:26 bye. Thanks, everyone! 17:57:29 bye everyone 17:57:34 bye 17:57:43 bye 17:57:54 bye 17:58:06 #endmeeting