)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"d722ad5be636bfe0c87b039d020655e77f20c499","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Issue of router updating with subnet and fiexed-ip"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Updating router with subnet and fiexed-ip was incomplete,"},{"line_number":10,"context_line":"Logical_Switch_Port table changed but Logical_Router_Port did not, so"},{"line_number":11,"context_line":"networks in Logical_Router_Port and addresses in Logical_Switch_Port"},{"line_number":12,"context_line":"are inconsistent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a3c09a3_018e552d","line":9,"updated":"2017-01-18 12:47:02.000000000","message":"By incomplete you mean a recent fix didn\u0027t handle this scenario ? or this is something which was missing altogether ?\n\nIf it\u0027s former, can you please update the commit message with either \"Fixes:\u003cCOMMIT_ID\"\u003e  or review id which missed this scenario ?\n\nCorrect me if i am wrong.","commit_id":"671af68bcb019938825c6916bfd4d11d744ea67c"},{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"6d37e4815e0704404c562e68e1dfa04b7a5ae07e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Issue of router updating with subnet and fiexed-ip"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Updating router with subnet and fiexed-ip was incomplete,"},{"line_number":10,"context_line":"Logical_Switch_Port table changed but Logical_Router_Port did not, so"},{"line_number":11,"context_line":"networks in Logical_Router_Port and addresses in Logical_Switch_Port"},{"line_number":12,"context_line":"are inconsistent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5a3905b3_761a6670","line":9,"in_reply_to":"7a3c09a3_018e552d","updated":"2017-01-19 02:25:17.000000000","message":"@Numan\nThere is an example in bug report.\nhttps://bugs.launchpad.net/networking-ovn/+bug/1653976\nUpdating router with different networks is OK.\nUpdating router with same network but different subnets is missing.\n\nThe second scenario was not missed by another bug-fix, the issue is present since NAT function supporting. \nhttps://review.openstack.org/#/c/346646/\nWhether still need to add commit ID?\n\ne.g.\nopenstack router set --external-gateway 905d6036-4536-460d-8232-e502b43f337a 0e4d345f-6b03-4574-b560-64157fc53011 is OK\nopenstack router set --external-gateway 905d6036-4536-460d-8232-e502b43f337a --fixed-ip subnet\u003d839cef52-1415-4d2f-86ae-4c3736c1b49c,ip-address\u003d172.24.4.6 0e4d345f-6b03-4574-b560-64157fc53011, addresses in Logical_Switch_Port table changed but networks in Logical_Router_Port did not.\n\nLogical_Router_Port table\n_uuid enabled external_ids mac name networks peer\n------------------------------------ ------- ------------ ------------------- ----------------------------------------------- ----------------------------------- ----\n7a6c2010-86d2-4f6d-8a4d-8b0aab0b61fd [] {} \"fa:16:3e:c6:d4:4b\" \"lrp-c99a83cf-739e-47b9-92c5-21a950e13ab5\" [\"172.24.4.3/24\", \"2001:db8::6/64\"] []\n\nLogical_Switch_Port table\n_uuid addresses dhcpv4_options dhcpv6_options dynamic_addresses enabled external_ids\n6dbad916-ef30-480e-9551-c42cdd562bc0 [\"fa:16:3e:c6:d4:4b 172.24.4.6\"] [] [] [] true {\"neutron:port_name\"\u003d\"\"}","commit_id":"671af68bcb019938825c6916bfd4d11d744ea67c"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"67efedb9a642d4a4a8498687e053fe2eeb66f328","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dong Jun \u003cdongj@dtdream.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-01-22 02:57:52 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Issue of router updating with subnet and fiexed-ip"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Updating router with subnet and fiexed-ip was incomplete,"},{"line_number":10,"context_line":"Logical_Switch_Port table changed but Logical_Router_Port did not, so"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5a3905b3_c029b5e7","line":7,"range":{"start_line":7,"start_character":41,"end_line":7,"end_character":47},"updated":"2017-01-24 06:14:13.000000000","message":"typo - fixed-ip","commit_id":"446789443aafd94e14f8806003c7c7264d858d7e"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"67efedb9a642d4a4a8498687e053fe2eeb66f328","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Issue of router updating with subnet and fiexed-ip"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Updating router with subnet and fiexed-ip was incomplete,"},{"line_number":10,"context_line":"Logical_Switch_Port table changed but Logical_Router_Port did not, so"},{"line_number":11,"context_line":"networks in Logical_Router_Port and addresses in Logical_Switch_Port"},{"line_number":12,"context_line":"are inconsistent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5a3905b3_00201df6","line":9,"range":{"start_line":9,"start_character":32,"end_line":9,"end_character":38},"updated":"2017-01-24 06:14:13.000000000","message":"same here.","commit_id":"446789443aafd94e14f8806003c7c7264d858d7e"}],"networking_ovn/l3/l3_ovn.py":[{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"3d6fe74a4b29c16003bc2b61028818276a786d13","unresolved":false,"context_lines":[{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"},{"line_number":485,"context_line":"                    if old_snat_state !\u003d new_snat_state:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a3905b3_7646e6c0","line":482,"range":{"start_line":482,"start_character":20,"end_line":482,"end_character":21},"updated":"2017-01-19 02:52:00.000000000","message":"Note: Here is some change for NAT. When external gateway was rebuilt in if branch, NAT was also rebuilt, it was redundant to do it again, thus i moved it to else branch.\nI added another two unit tests handling the conditions *only NAT enable or disable*, but the diff file is so confused that i would open another commit.","commit_id":"671af68bcb019938825c6916bfd4d11d744ea67c"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"87a653a9729887ab45cfb11291d1233029070524","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                             gateway_new[\u0027external_fixed_ips\u0027]])):"},{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_76b52b7c","line":481,"range":{"start_line":481,"start_character":16,"end_line":481,"end_character":21},"updated":"2017-01-24 07:20:46.000000000","message":"I would propose to do it in a separate change, since this is not related to the bug.","commit_id":"3377dcbda40c81206c1b70d3ea40eaff7a183c34"},{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"9d878512f3fbbdb2dd1d19c83e058adc4d1e05c0","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                             gateway_new[\u0027external_fixed_ips\u0027]])):"},{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_8d999dc6","line":481,"range":{"start_line":481,"start_character":16,"end_line":481,"end_character":21},"in_reply_to":"5a3905b3_76b52b7c","updated":"2017-01-24 09:44:19.000000000","message":"Although the two issues are not closely related, the new test cases are. If I separate them, I should open another commit for unit tests, or I need to write the unit tests with original logic and revise them soon after. It\u0027s a dilemma choice.","commit_id":"3377dcbda40c81206c1b70d3ea40eaff7a183c34"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"b3abd9e74f4292b3943214e3e8c034c8b9825bd3","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                             gateway_new[\u0027external_fixed_ips\u0027]])):"},{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_65eac5dc","line":481,"range":{"start_line":481,"start_character":16,"end_line":481,"end_character":21},"in_reply_to":"5a3905b3_8294078b","updated":"2017-01-24 11:15:30.000000000","message":"Ok. thanks.","commit_id":"3377dcbda40c81206c1b70d3ea40eaff7a183c34"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"ee5bd2daefcea930a4e1ca065d65169d96c27679","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                             gateway_new[\u0027external_fixed_ips\u0027]])):"},{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_b3115c54","line":481,"range":{"start_line":481,"start_character":16,"end_line":481,"end_character":21},"in_reply_to":"5a3905b3_8d999dc6","updated":"2017-01-24 10:22:36.000000000","message":"I agree with Han Zhou. A separate commit makes sense. You can make one patch dependent on another one.\n\nBut, I fail to understand why the snat enable/disable update is  moved to \"else\".\n\nA user can run the commands in sequence like below and try to update both the gateway and the \"snat\" field in one request and we should allow it. But this patch seem to not allow updating both in one request.\n\n\nopenstack router set  --external-gateway 9619be8b-6343-43da-8072-2ed2f5b3ed92 --fixed-ip subnet\u003dcf7f330a-cae7-4492-8c15-043ac8bb68e5,ip-address\u003d192.168.121.31  --enable-snat r1\n\nopenstack router set  --external-gateway 9619be8b-6343-43da-8072-2ed2f5b3ed92 --fixed-ip subnet\u003dcf7f330a-cae7-4492-8c15-043ac8bb68e5,ip-address\u003d192.168.121.32  --disable-snat r1","commit_id":"3377dcbda40c81206c1b70d3ea40eaff7a183c34"},{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"89314dc6b5ce3bceb9a7299598cab3360da3d18c","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                             gateway_new[\u0027external_fixed_ips\u0027]])):"},{"line_number":479,"context_line":"                    self._delete_router_ext_gw(context, id, original_router)"},{"line_number":480,"context_line":"                    self._add_router_ext_gw(context, result)"},{"line_number":481,"context_line":"                else:"},{"line_number":482,"context_line":"                    # Check if snat has been enabled/disabled and update"},{"line_number":483,"context_line":"                    old_snat_state \u003d gateway_old.get(\u0027enable_snat\u0027, True)"},{"line_number":484,"context_line":"                    new_snat_state \u003d gateway_new.get(\u0027enable_snat\u0027, True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_8294078b","line":481,"range":{"start_line":481,"start_character":16,"end_line":481,"end_character":21},"in_reply_to":"5a3905b3_b3115c54","updated":"2017-01-24 10:52:56.000000000","message":"This condition can be covered, the new sNAT was processed in _add_router_ext_gw(context, result). So need not to deal with sNat individually if external gateway was rebuilt.\nOK, I will separate them.","commit_id":"3377dcbda40c81206c1b70d3ea40eaff7a183c34"}]}