)]}'
{"tacker/db/nfvo/vnffg_db.py":[{"author":{"_account_id":26222,"name":"Cong Phuoc Hoang","email":"hoangphuocbk2.07@gmail.com","username":"hoangphuoc"},"change_message_id":"2158a4e422a6d6f375873af2fe13d98589f7cc48","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                                template_db,"},{"line_number":387,"context_line":"                                                nfp_dict[\u0027name\u0027])"},{"line_number":388,"context_line":"                LOG.debug(\u0027chain: %s\u0027, chain)"},{"line_number":389,"context_line":"                sfc_db \u003d VnffgChain(id\u003dsfc_id,"},{"line_number":390,"context_line":"                                    tenant_id\u003dtenant_id,"},{"line_number":391,"context_line":"                                    status\u003dconstants.PENDING_CREATE,"},{"line_number":392,"context_line":"                                    symmetrical\u003dsymmetrical,"},{"line_number":393,"context_line":"                                    chain\u003dchain,"},{"line_number":394,"context_line":"                                    nfp_id\u003dnfp_id,"},{"line_number":395,"context_line":"                                    path_id\u003dnfp_dict[\u0027path_id\u0027])"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"                context.session.add(sfc_db)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_7063d01b","line":395,"range":{"start_line":389,"start_character":0,"end_line":395,"end_character":64},"updated":"2017-10-12 09:08:33.000000000","message":"It should be one VnffgChain too, move it to outside \"for\" loop, you can also store all nfp_ids in a array\neg. nfp_id \u003d [nfp1_id, nfp2_id,...]","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":25505,"name":"Dimitrios Markou","email":"dimitrios.markou@att.com","username":"mardim"},"change_message_id":"888be6df94090c7955ac3a8526221a34787111f8","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                                template_db,"},{"line_number":387,"context_line":"                                                nfp_dict[\u0027name\u0027])"},{"line_number":388,"context_line":"                LOG.debug(\u0027chain: %s\u0027, chain)"},{"line_number":389,"context_line":"                sfc_db \u003d VnffgChain(id\u003dsfc_id,"},{"line_number":390,"context_line":"                                    tenant_id\u003dtenant_id,"},{"line_number":391,"context_line":"                                    status\u003dconstants.PENDING_CREATE,"},{"line_number":392,"context_line":"                                    symmetrical\u003dsymmetrical,"},{"line_number":393,"context_line":"                                    chain\u003dchain,"},{"line_number":394,"context_line":"                                    nfp_id\u003dnfp_id,"},{"line_number":395,"context_line":"                                    path_id\u003dnfp_dict[\u0027path_id\u0027])"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"                context.session.add(sfc_db)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_c48d25d4","line":395,"range":{"start_line":389,"start_character":0,"end_line":395,"end_character":64},"in_reply_to":"5f4e5783_7063d01b","updated":"2017-10-12 09:33:35.000000000","message":"I think this comment it goes hand to hand with your previous comment so IMHO I think we should leave it inside the for loop because we have one classifier per chain right now.","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"}],"tacker/nfvo/nfvo_plugin.py":[{"author":{"_account_id":26222,"name":"Cong Phuoc Hoang","email":"hoangphuocbk2.07@gmail.com","username":"hoangphuoc"},"change_message_id":"2158a4e422a6d6f375873af2fe13d98589f7cc48","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_f021004f","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"updated":"2017-10-12 09:08:33.000000000","message":"As my knowledge, there are one chain and many flow classifiers. So you should put it out of \"for\" loop","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":25505,"name":"Dimitrios Markou","email":"dimitrios.markou@att.com","username":"mardim"},"change_message_id":"2df855832696a88076a06e2d86fdc4cd83a10ff6","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_3d864cdd","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"in_reply_to":"5f4e5783_1a227653","updated":"2017-10-12 11:22:23.000000000","message":"Currently a forwarding path consists of a classifier and a chain.\n\nYou cannot create a forwarding path which consists of only a classifier or only a chain you have to include both of them in a forwarding path (at least this is my understanding maybe I am wrong). \nSo if we want to support multiple classifiers this doesn\u0027t mean multiple forwarding paths because those things are not equal.\n\nSo if you have 5 forwarding paths that means that you want to create 5 classifiers and 5 chains respectively.\n\nThe best thing to do is to have the ability to create classifiers and chains independently I agree on that but right now we cannot do that because the notion and the structure of forwarding path doesn\u0027t allow to do so.\n\nMaybe I am wrong please tell me your opinion on this :)","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":26222,"name":"Cong Phuoc Hoang","email":"hoangphuocbk2.07@gmail.com","username":"hoangphuoc"},"change_message_id":"9862a5c79999b4c31f6126cc809f574321d150d8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_1a227653","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"in_reply_to":"5f4e5783_2456395e","updated":"2017-10-12 10:34:02.000000000","message":"Example you have 5 forwarding paths, if you put function create_chain() in for loop, you will create 5 port chains too. I think you should put it outside for loop, to create 1 port chains and 5 flow classifiers.\n\nYou can look up this file: https://github.com/openstack/tacker/blob/master/tacker/nfvo/drivers/vim/openstack_driver.py \nand change this: port_chain[\u0027flow_classifiers\u0027] \u003d [fc_id_list]\n\nSo Tacker will support multiple flow classifiers after your patch :)","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":26222,"name":"Cong Phuoc Hoang","email":"hoangphuocbk2.07@gmail.com","username":"hoangphuoc"},"change_message_id":"047ac58bea9a386b4bb19388a030248a12befac4","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_b84fea32","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"in_reply_to":"5f4e5783_3d864cdd","updated":"2017-10-12 11:45:15.000000000","message":"I got your point. I misunderstood about it. Current forwarding path definition only supports one flow classifier per chain, so we should redefine it to apply multiple flow classifiers per chain. Keep going ahead :)","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":25505,"name":"Dimitrios Markou","email":"dimitrios.markou@att.com","username":"mardim"},"change_message_id":"9d926af50f847e035423c6156518c9c7b34b66c8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_187efe18","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"in_reply_to":"5f4e5783_b84fea32","updated":"2017-10-12 11:54:07.000000000","message":"Thanks a lot for reviewing this closely :). \n\nActually right now (in another patch) I am trying to add support for multiple classifier per chain but I am not sure what exactly I should change. I think the best thing is to redefine Forwarding path (as you said ) to support multiclassifiers per chain. But I think to do so we need to change a bit tosca templates.\n\nWe can discuss a bit this in IRC channel my nickname is mardim.\n\nIf you guys can guide me on this a bit that would be great because I am new in tacker :)","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"},{"author":{"_account_id":25505,"name":"Dimitrios Markou","email":"dimitrios.markou@att.com","username":"mardim"},"change_message_id":"888be6df94090c7955ac3a8526221a34787111f8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027],"},{"line_number":324,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027])"},{"line_number":325,"context_line":"                # Check if the chain is already created by networking sfc"},{"line_number":326,"context_line":"                sfc_id \u003d self._vim_drivers. \\"},{"line_number":327,"context_line":"                    invoke(driver_type,"},{"line_number":328,"context_line":"                           \u0027create_chain\u0027,"},{"line_number":329,"context_line":"                           name\u003dvnffg_dict[\u0027name\u0027],"},{"line_number":330,"context_line":"                           vnfs\u003dsfc[\u0027chain\u0027], fc_id\u003dfc_id,"},{"line_number":331,"context_line":"                           symmetrical\u003dsfc[\u0027symmetrical\u0027],"},{"line_number":332,"context_line":"                           auth_attr\u003dvim_obj[\u0027auth_cred\u0027])"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":335,"context_line":"                    self.delete_vnffg(context, vnffg_id\u003dvnffg_dict[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f4e5783_2456395e","line":332,"range":{"start_line":326,"start_character":0,"end_line":332,"end_character":58},"in_reply_to":"5f4e5783_f021004f","updated":"2017-10-12 09:33:35.000000000","message":"IF I am not mistaken tacker doesn\u0027t support assigning multiple flow classifiers to one chain [0] (which is something that we should fix).\n\nSo for now we have one classifier per chain. So IMHO we can leave it inside the for loop.\n\n[0] https://bugs.launchpad.net/tacker/+bug/1719839","commit_id":"a42aa98f17fac6d1d0f628a498dffbb38b3c0047"}]}