)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"c0d9feb5f9bd470971221415bcd301490c437474","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     shihanzhang \u003cshihanzhang@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-08-05 16:44:10 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset to security group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add ipset to security group for improving the performance of the security"},{"line_number":10,"context_line":"group."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_03221454","line":7,"updated":"2014-08-06 04:07:34.000000000","message":"Add ipset support for Security Groups","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"71a6493f404937216bb83238eb1f136867dc554c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     shihanzhang \u003cshihanzhang@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-08-05 16:44:10 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset to security group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add ipset to security group for improving the performance of the security"},{"line_number":10,"context_line":"group."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_8301c409","line":7,"in_reply_to":"baada198_03221454","updated":"2014-08-06 04:49:21.000000000","message":"thanks for your review","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"0ac494e9b24e43976846160b5066f38cf306dd4a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset to security group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add ipset to security group for improving the performance of the security"},{"line_number":10,"context_line":"group."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I6ff0ac519d0b9034d3bb5270885ed3cc1805674d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_49f42dd0","line":9,"updated":"2014-08-06 05:52:01.000000000","message":"Can you explain a little more that this patch does? Whats different between these two modes, etc.","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"0ee1cefe84480a88fe5c56d1f432d83dd861fce2","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset to security group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add ipset to security group for improving the performance of the security"},{"line_number":10,"context_line":"group."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I6ff0ac519d0b9034d3bb5270885ed3cc1805674d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_6f2f2e8a","line":9,"in_reply_to":"baada198_49f42dd0","updated":"2014-08-06 08:21:11.000000000","message":"thanks for your review, I will modify it as soon as possilbe","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     shihanzhang \u003cshihanzhang@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-09-09 16:59:12 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset support for Security Groups"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Iptables chain is linear storage and filtering, when iptables rules are"},{"line_number":10,"context_line":"large, the load of l2 agent is heavy, this patch will introduces ipset to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"fa98f980_99b65f89","line":7,"updated":"2014-09-09 12:59:48.000000000","message":"Adds instead of Add","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     shihanzhang \u003cshihanzhang@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-09-09 16:59:12 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ipset support for Security Groups"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Iptables chain is linear storage and filtering, when iptables rules are"},{"line_number":10,"context_line":"large, the load of l2 agent is heavy, this patch will introduces ipset to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"fa98f980_e7da5de8","line":7,"in_reply_to":"fa98f980_99b65f89","updated":"2014-09-09 14:24:18.000000000","message":"Done","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add ipset support for Security Groups"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Iptables chain is linear storage and filtering, when iptables rules are"},{"line_number":10,"context_line":"large, the load of l2 agent is heavy, this patch will introduces ipset to"},{"line_number":11,"context_line":"security group for improving the security group performance."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I6ff0ac519d0b9034d3bb5270885ed3cc1805674d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"fa98f980_19934ffb","line":10,"updated":"2014-09-09 12:59:48.000000000","message":"introduces instead of will introduces","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add ipset support for Security Groups"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Iptables chain is linear storage and filtering, when iptables rules are"},{"line_number":10,"context_line":"large, the load of l2 agent is heavy, this patch will introduces ipset to"},{"line_number":11,"context_line":"security group for improving the security group performance."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I6ff0ac519d0b9034d3bb5270885ed3cc1805674d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"fa98f980_67ee6d8d","line":10,"in_reply_to":"fa98f980_19934ffb","updated":"2014-09-09 14:24:18.000000000","message":"agree","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"}],"etc/neutron/plugins/linuxbridge/linuxbridge_conf.ini":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"c0d9feb5f9bd470971221415bcd301490c437474","unresolved":false,"context_lines":[{"line_number":77,"context_line":"# It should be false when you use nova security group."},{"line_number":78,"context_line":"# enable_security_group \u003d True"},{"line_number":79,"context_line":"# enable_enhanced_rpc \u003d False"},{"line_number":80,"context_line":"# Requires L2 agent install ipset and enable_enhanced_rpc is True"},{"line_number":81,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":4,"id":"baada198_e32c404c","line":80,"updated":"2014-08-06 04:07:34.000000000","message":"rephrase as: Enabling ipset support requires that ipset is installed on L2 agent node and enable_enhanced_rpc is set to True","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"71a6493f404937216bb83238eb1f136867dc554c","unresolved":false,"context_lines":[{"line_number":77,"context_line":"# It should be false when you use nova security group."},{"line_number":78,"context_line":"# enable_security_group \u003d True"},{"line_number":79,"context_line":"# enable_enhanced_rpc \u003d False"},{"line_number":80,"context_line":"# Requires L2 agent install ipset and enable_enhanced_rpc is True"},{"line_number":81,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":4,"id":"baada198_4e0ef334","line":80,"in_reply_to":"baada198_e32c404c","updated":"2014-08-06 04:49:21.000000000","message":"ok","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"7498c31ec4cba8a676f1fa24b36d33a6e9efce02","unresolved":false,"context_lines":[{"line_number":83,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":84,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":85,"context_line":"# enable_ipset \u003d False"},{"line_number":86,"context_line":"\u003e\u003e\u003e\u003e\u003e\u003e\u003e 5acf40d... Add ipset support for Security Groups"}],"source_content_type":"text/x-properties","patch_set":5,"id":"1abeadc6_8891a29c","line":86,"updated":"2014-08-27 09:08:45.000000000","message":"That was a merge conflict, just remove \u003c\u003c\u003cHEAD, \u003d\u003d\u003d\u003d \u003e\u003e\u003e\u003e\u003e lines,\n\nand keep the enable_enhanced_rpc \u003d True","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":80,"context_line":"# reduce the message size and optimize performance. This is recommended"},{"line_number":81,"context_line":"# in general, and specially for high density clouds."},{"line_number":82,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":83,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":84,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":85,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_e17a29f0","line":83,"updated":"2014-09-02 14:48:49.000000000","message":"please use blank line to separate options for better readability","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":82,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":83,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":84,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":85,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_eab976ea","line":85,"updated":"2014-09-01 08:12:22.000000000","message":"Enable by default (as per conversation on the last meeting about high priority issues)","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":82,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":83,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":84,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":85,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_db3ac635","line":85,"in_reply_to":"fa98f980_eab976ea","updated":"2014-09-01 09:05:33.000000000","message":"agree","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_4c2b7a94","updated":"2014-09-08 13:49:56.000000000","message":"linuxbridge plugin is deprecated and will be deleted in Juno, so I think no need to update this config","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fa66b82df87a855a2e4d11f4a2d80eba0c6e176e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_b4734cb3","in_reply_to":"fa98f980_4c2b7a94","updated":"2014-09-08 14:18:34.000000000","message":"What will use the lb agent?, even if the plugin is deprecated, the agent still will be the linuxbridge agent, right?","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_28737bb2","in_reply_to":"fa98f980_947d68b4","updated":"2014-09-08 15:57:26.000000000","message":"ok, I got it, I will remove this in next patch","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0cae561af98ecf554f9eae56dafd43d8d3c14a7e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_947d68b4","in_reply_to":"fa98f980_b4734cb3","updated":"2014-09-08 14:44:25.000000000","message":"Yes, the agent is the same, but the config will be from ml2 plugin","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"}],"etc/neutron/plugins/ml2/ml2_conf.ini":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":64,"context_line":"# reduce the message size and optimize performance. This is recommended"},{"line_number":65,"context_line":"# in general, and specially for high density clouds."},{"line_number":66,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":67,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":68,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":69,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_616e3931","line":67,"updated":"2014-09-02 14:48:49.000000000","message":"ditto","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":66,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":67,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":68,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":69,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_aaa68ebf","line":69,"updated":"2014-09-01 08:12:22.000000000","message":"Enable by default (as per conversation on the last meeting about high priority issues)","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":66,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":67,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":68,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":69,"context_line":"# enable_ipset \u003d False"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_1b3c4e2f","line":69,"in_reply_to":"fa98f980_aaa68ebf","updated":"2014-09-01 09:05:33.000000000","message":"Done","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":70,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":71,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":22,"id":"fa98f980_a9454e44","line":70,"updated":"2014-09-11 09:47:10.000000000","message":"there is no enable_enhanced_rpc now, right? Also would be good to add why enabling ipset may be needed (just like in help message in option definition)","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":70,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":71,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":22,"id":"fa98f980_5d973c90","line":70,"in_reply_to":"fa98f980_a9454e44","updated":"2014-09-11 11:10:10.000000000","message":"Correct,","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":70,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":71,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":22,"id":"fa98f980_7f7eb8b0","line":70,"in_reply_to":"fa98f980_a9454e44","updated":"2014-09-11 11:13:23.000000000","message":"yes, enable_enhanced_rpc has be removed, I agree with your suggestion","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"f13ec29975a20ec980e6866ba87e3f3d4652fa62","unresolved":false,"context_lines":[{"line_number":65,"context_line":"# Controls if neutron security group is enabled or not."},{"line_number":66,"context_line":"# It should be false when you use nova security group."},{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node."},{"line_number":70,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":23,"id":"fa98f980_7daec02f","line":68,"updated":"2014-09-11 11:32:22.000000000","message":"# Use ipset to speedup the iptables based security groups.\n ...","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"4d9995e3a86b6a27a425539dbdb6edfd939ec228","unresolved":false,"context_lines":[{"line_number":65,"context_line":"# Controls if neutron security group is enabled or not."},{"line_number":66,"context_line":"# It should be false when you use nova security group."},{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node."},{"line_number":70,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":23,"id":"fa98f980_93d8b31f","line":68,"in_reply_to":"fa98f980_7daec02f","updated":"2014-09-11 12:28:29.000000000","message":"Done","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"1a2ed8179d610ccc22989baa5784b9d4082edd95","unresolved":false,"context_lines":[{"line_number":65,"context_line":"# Controls if neutron security group is enabled or not."},{"line_number":66,"context_line":"# It should be false when you use nova security group."},{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node."},{"line_number":70,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":23,"id":"fa98f980_93c19331","line":68,"in_reply_to":"fa98f980_7daec02f","updated":"2014-09-11 12:25:23.000000000","message":"agree","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9790c1ba8bf1aa1880219fd141f4abcd964869c6","unresolved":false,"context_lines":[{"line_number":66,"context_line":"# It should be false when you use nova security group."},{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Use ipset to speedup the iptables security groups. Enabling ipset support"},{"line_number":70,"context_line":"# requires that ipset is installed on L2 agent node."},{"line_number":71,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":24,"id":"fa98f980_e448422f","line":69,"updated":"2014-09-11 22:00:58.000000000","message":"nit: s/speedup/speed-up/","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9cdbb93e8c8e8d62ca6542f8c624cb0e328b5ae8","unresolved":false,"context_lines":[{"line_number":66,"context_line":"# It should be false when you use nova security group."},{"line_number":67,"context_line":"# enable_security_group \u003d True"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"# Use ipset to speedup the iptables security groups. Enabling ipset support"},{"line_number":70,"context_line":"# requires that ipset is installed on L2 agent node."},{"line_number":71,"context_line":"# enable_ipset \u003d True"}],"source_content_type":"text/x-properties","patch_set":24,"id":"fa98f980_a7f55cee","line":69,"in_reply_to":"fa98f980_e448422f","updated":"2014-09-11 22:24:06.000000000","message":"Done","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"}],"etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"c0d9feb5f9bd470971221415bcd301490c437474","unresolved":false,"context_lines":[{"line_number":158,"context_line":"# It should be false when you use nova security group."},{"line_number":159,"context_line":"# enable_security_group \u003d True"},{"line_number":160,"context_line":"# enable_enhanced_rpc \u003d False"},{"line_number":161,"context_line":"# Requires L2 agent install ipset and enable_enhanced_rpc is True"},{"line_number":162,"context_line":"# enable_ipset \u003d False"},{"line_number":163,"context_line":"#-----------------------------------------------------------------------------"},{"line_number":164,"context_line":"# Sample Configurations."}],"source_content_type":"text/x-properties","patch_set":4,"id":"baada198_a3fb28bd","line":161,"updated":"2014-08-06 04:07:34.000000000","message":"rephrase as:\nEnabling ipset support requires that ipset is installed on L2 agent node and enable_enhanced_rpc is set to True","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5a04409524660920f653bed9e18f35351881d6b6","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"# Controls if neutron security group is enabled or not."},{"line_number":158,"context_line":"# It should be false when you use nova security group."},{"line_number":159,"context_line":"# enable_security_group \u003d True"},{"line_number":160,"context_line":"\u003c\u003c\u003c\u003c\u003c\u003c\u003c HEAD"},{"line_number":161,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":162,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-properties","patch_set":5,"id":"baada198_d9388b3d","line":159,"updated":"2014-08-09 18:25:52.000000000","message":"\u003c\u003c\u003c\u003c\u003c\u003c\u003c HEAD is quite a funny looking ini option :)","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":162,"context_line":"# reduce the message size and optimize performance. This is recommended"},{"line_number":163,"context_line":"# in general, and specially for high density clouds."},{"line_number":164,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":165,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":166,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":167,"context_line":"# enable_ipset \u003d False"},{"line_number":168,"context_line":"#-----------------------------------------------------------------------------"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_81695d27","line":165,"updated":"2014-09-02 14:48:49.000000000","message":"ditto","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":164,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":165,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":166,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":167,"context_line":"# enable_ipset \u003d False"},{"line_number":168,"context_line":"#-----------------------------------------------------------------------------"},{"line_number":169,"context_line":"# Sample Configurations."},{"line_number":170,"context_line":"#-----------------------------------------------------------------------------"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_caabd287","line":167,"updated":"2014-09-01 08:12:22.000000000","message":"Enable by default (as per conversation on the last meeting about high priority issues)","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":164,"context_line":"# enable_enhanced_rpc \u003d True"},{"line_number":165,"context_line":"# Enabling ipset support requires that ipset is installed on L2 agent node"},{"line_number":166,"context_line":"# and enable_enhanced_rpc is set to True"},{"line_number":167,"context_line":"# enable_ipset \u003d False"},{"line_number":168,"context_line":"#-----------------------------------------------------------------------------"},{"line_number":169,"context_line":"# Sample Configurations."},{"line_number":170,"context_line":"#-----------------------------------------------------------------------------"}],"source_content_type":"text/x-properties","patch_set":9,"id":"fa98f980_9b175ea3","line":167,"in_reply_to":"fa98f980_caabd287","updated":"2014-09-01 09:05:33.000000000","message":"Done","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_cc168a55","updated":"2014-09-08 13:49:56.000000000","message":"ditto, openvswitch plugin is deprecated in favor of ml2","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_a85e8b10","in_reply_to":"fa98f980_34675cec","updated":"2014-09-08 15:57:26.000000000","message":"Done","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0cae561af98ecf554f9eae56dafd43d8d3c14a7e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_545a2051","in_reply_to":"fa98f980_34675cec","updated":"2014-09-08 14:44:25.000000000","message":"it will use ml2 conf","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_c89aefc4","in_reply_to":"fa98f980_545a2051","updated":"2014-09-08 15:57:26.000000000","message":"got it","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fa66b82df87a855a2e4d11f4a2d80eba0c6e176e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"fa98f980_34675cec","in_reply_to":"fa98f980_cc168a55","updated":"2014-09-08 14:18:34.000000000","message":"won\u0027t the OVS agent still need the file?","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"}],"neutron/agent/linux/ipset_manager.py":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"c0d9feb5f9bd470971221415bcd301490c437474","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 Locaweb."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_436dec99","line":1,"updated":"2014-08-06 04:07:34.000000000","message":"incorrect copyright","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"71a6493f404937216bb83238eb1f136867dc554c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 Locaweb."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_8e180b78","line":1,"in_reply_to":"baada198_436dec99","updated":"2014-08-06 04:49:21.000000000","message":"Done","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"7498c31ec4cba8a676f1fa24b36d33a6e9efce02","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 Locaweb."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_48130a0d","line":1,"updated":"2014-08-27 09:08:45.000000000","message":"2012? :) Locaweb? :)","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"971d9667b128217c6ee0142b65365e70a556deda","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 Locaweb."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_a97460cf","line":1,"in_reply_to":"1abeadc6_48130a0d","updated":"2014-08-29 07:07:27.000000000","message":"it is not correct, I will modify it in next patch","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":7987,"name":"Ivar Lazzaro","email":"ivarlazzaro@gmail.com","username":"Ivar"},"change_message_id":"9cf48cd05e4e8f853f24b3262decb9e4c6f9e0dd","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    limitations under the License."},{"line_number":15,"context_line":"#"},{"line_number":16,"context_line":"# @author: shihanzhang."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from neutron.agent.linux import utils as linux_utils"},{"line_number":20,"context_line":"from neutron.common import utils"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_d87796d2","line":17,"updated":"2014-08-29 01:03:27.000000000","message":"nit: leave just one blank line","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"7498c31ec4cba8a676f1fa24b36d33a6e9efce02","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":65,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":66,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":69,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_a8be5efd","line":66,"updated":"2014-08-27 09:08:45.000000000","message":"Is there any way to add several members at once?\n\nThat would save lots of system calls, for example if add_member_to_ipset could receive a list of ip addresses.","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"971d9667b128217c6ee0142b65365e70a556deda","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":65,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":66,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":69,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_24007700","line":66,"in_reply_to":"1abeadc6_a8be5efd","updated":"2014-08-29 07:07:27.000000000","message":"I am not sure that ipset can do that, but I will look up carefully!","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"7498c31ec4cba8a676f1fa24b36d33a6e9efce02","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":69,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"},{"line_number":70,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def destroy_ipset_chain_by_name(self, chain_name):"},{"line_number":73,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027destroy\u0027, chain_name]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_28596efe","line":70,"updated":"2014-08-27 09:08:45.000000000","message":"same here, so if we receive several ips to remove we can do all at once. May be this case will happen less or , is it less likely to exist?","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"971d9667b128217c6ee0142b65365e70a556deda","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":69,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"},{"line_number":70,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def destroy_ipset_chain_by_name(self, chain_name):"},{"line_number":73,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027destroy\u0027, chain_name]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_4405730f","line":70,"in_reply_to":"1abeadc6_28596efe","updated":"2014-08-29 07:07:27.000000000","message":"Done","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":24,"context_line":"class IpsetManager(object):"},{"line_number":25,"context_line":"    \"\"\"Wrapper for ipset."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone, use_ipv6\u003dFalse):"},{"line_number":30,"context_line":"        if _execute:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_213d11df","line":27,"updated":"2014-09-02 14:48:49.000000000","message":"please either add more descriptopn or make it one-liner docstring","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"01cc816930dd7d74b4a80d305f03e400394c6490","unresolved":false,"context_lines":[{"line_number":24,"context_line":"class IpsetManager(object):"},{"line_number":25,"context_line":"    \"\"\"Wrapper for ipset."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone, use_ipv6\u003dFalse):"},{"line_number":30,"context_line":"        if _execute:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_3141caa9","line":27,"in_reply_to":"fa98f980_213d11df","updated":"2014-09-03 01:54:46.000000000","message":"ok","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone, use_ipv6\u003dFalse):"},{"line_number":30,"context_line":"        if _execute:"},{"line_number":31,"context_line":"            self.execute \u003d _execute"},{"line_number":32,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_81087d2e","line":29,"updated":"2014-09-02 14:48:49.000000000","message":"_execute: why starting with underscore?","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone, use_ipv6\u003dFalse):"},{"line_number":30,"context_line":"        if _execute:"},{"line_number":31,"context_line":"            self.execute \u003d _execute"},{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            self.execute \u003d linux_utils.execute"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_fc03b20a","line":30,"updated":"2014-09-02 14:48:49.000000000","message":"self.execute \u003d _execute or linux_utils.execute","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"01cc816930dd7d74b4a80d305f03e400394c6490","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone, use_ipv6\u003dFalse):"},{"line_number":30,"context_line":"        if _execute:"},{"line_number":31,"context_line":"            self.execute \u003d _execute"},{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            self.execute \u003d linux_utils.execute"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_7147d2bc","line":30,"in_reply_to":"fa98f980_fc03b20a","updated":"2014-09-03 01:54:46.000000000","message":"got it","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"20298ac1844e30b6d9f349c4096b59c9877ee3a0","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def _apply(self, cmd, root_helper):"},{"line_number":39,"context_line":"        lock_name \u003d \u0027ipset\u0027"},{"line_number":40,"context_line":"        try:"},{"line_number":41,"context_line":"            with lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX, True):"},{"line_number":42,"context_line":"                LOG.debug(_(\u0027Got semaphore / lock \"%s\"\u0027), lock_name)"},{"line_number":43,"context_line":"                self.execute(cmd, root_helper)"},{"line_number":44,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_7c47c21d","line":41,"updated":"2014-09-02 14:48:49.000000000","message":"why not using existing decorator?\n @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"01cc816930dd7d74b4a80d305f03e400394c6490","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def _apply(self, cmd, root_helper):"},{"line_number":39,"context_line":"        lock_name \u003d \u0027ipset\u0027"},{"line_number":40,"context_line":"        try:"},{"line_number":41,"context_line":"            with lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX, True):"},{"line_number":42,"context_line":"                LOG.debug(_(\u0027Got semaphore / lock \"%s\"\u0027), lock_name)"},{"line_number":43,"context_line":"                self.execute(cmd, root_helper)"},{"line_number":44,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_d1807ec2","line":41,"in_reply_to":"fa98f980_7c47c21d","updated":"2014-09-03 01:54:46.000000000","message":"I reference the method in iptables_manager.py:_apply, I want to know which is more better","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3c39234d2013107aaa9e6fb048f962f3a1effa1","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def _apply(self, cmd, root_helper):"},{"line_number":39,"context_line":"        lock_name \u003d \u0027ipset\u0027"},{"line_number":40,"context_line":"        try:"},{"line_number":41,"context_line":"            with lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX, True):"},{"line_number":42,"context_line":"                LOG.debug(_(\u0027Got semaphore / lock \"%s\"\u0027), lock_name)"},{"line_number":43,"context_line":"                self.execute(cmd, root_helper)"},{"line_number":44,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_58e90ae5","line":41,"in_reply_to":"fa98f980_d1807ec2","updated":"2014-09-03 07:20:44.000000000","message":"I guess that \u0027synchronized\u0027 decorator was extracted when iptables_manager code was already in. I think it\u0027s better to use decorator now","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":56,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":57,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":60,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_0a895aca","line":57,"updated":"2014-09-01 08:12:22.000000000","message":"We must try to switch into save/restore (and not sure if we may need swap) to avoid excessive system calls.","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":56,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":57,"context_line":"        self._apply(cmd, self.root_helper)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def del_ipset_chain_member(self, chain_name, member_ip):"},{"line_number":60,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027del\u0027, chain_name, member_ip]"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_3b775204","line":57,"in_reply_to":"fa98f980_0a895aca","updated":"2014-09-01 09:05:33.000000000","message":"I am also think about use save/restore, but there is other problem, I will communicate with you on IRC","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"4648b16aebb8cd4157aa2be160b0272005c74627","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(_(\u0027Got semaphore / lock ipset\u0027))"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_3bf09990","line":33,"updated":"2014-09-04 05:51:13.000000000","message":"We don\u0027t need to translate debug messages.","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"09bd0d050e42b402512e36476ad7c46a037ccba9","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(_(\u0027Got semaphore / lock ipset\u0027))"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_01228251","line":33,"in_reply_to":"fa98f980_3bf09990","updated":"2014-09-04 07:13:33.000000000","message":"I forgot this, thanks for your review","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"4648b16aebb8cd4157aa2be160b0272005c74627","unresolved":false,"context_lines":[{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"},{"line_number":38,"context_line":"                             process_input\u003d\u0027\\n\u0027.join(input))"},{"line_number":39,"context_line":"        finally:"},{"line_number":40,"context_line":"            LOG.debug(_(\u0027Semaphore / lock released ipset\u0027))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_5bed5db6","line":40,"updated":"2014-09-04 05:51:13.000000000","message":"ditto, etc in patch set.\n\nThere is already a log statement that is provided via utils.synchronized() that tells us this.","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"09bd0d050e42b402512e36476ad7c46a037ccba9","unresolved":false,"context_lines":[{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"},{"line_number":38,"context_line":"                             process_input\u003d\u0027\\n\u0027.join(input))"},{"line_number":39,"context_line":"        finally:"},{"line_number":40,"context_line":"            LOG.debug(_(\u0027Semaphore / lock released ipset\u0027))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_41180a1e","line":40,"in_reply_to":"fa98f980_5bed5db6","updated":"2014-09-04 07:13:33.000000000","message":"got it","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"4648b16aebb8cd4157aa2be160b0272005c74627","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"},{"line_number":44,"context_line":"        if ethertype \u003d\u003d \u0027IPv4\u0027:"},{"line_number":45,"context_line":"            cmd.append(\u0027inet\u0027)"},{"line_number":46,"context_line":"        else:"},{"line_number":47,"context_line":"            cmd.append(\u0027inet6\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_db86adda","line":44,"updated":"2014-09-04 05:51:13.000000000","message":"nit: I think we should pull lines 44-47 out to a helper method as we basically duplicate this on 58-62.","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"09bd0d050e42b402512e36476ad7c46a037ccba9","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"},{"line_number":44,"context_line":"        if ethertype \u003d\u003d \u0027IPv4\u0027:"},{"line_number":45,"context_line":"            cmd.append(\u0027inet\u0027)"},{"line_number":46,"context_line":"        else:"},{"line_number":47,"context_line":"            cmd.append(\u0027inet6\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_81067284","line":44,"in_reply_to":"fa98f980_db86adda","updated":"2014-09-04 07:13:33.000000000","message":"agree","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(\u0027Got semaphore / lock ipset\u0027)"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_8e19d77b","line":33,"updated":"2014-09-08 13:49:56.000000000","message":"Why additional logging here? Please check existing logging in utils.synchronized","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(\u0027Got semaphore / lock ipset\u0027)"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_a8edabf1","line":33,"in_reply_to":"fa98f980_8e19d77b","updated":"2014-09-08 15:57:26.000000000","message":"ok","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(\u0027Got semaphore / lock ipset\u0027)"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"},{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_eef26305","line":34,"updated":"2014-09-08 13:49:56.000000000","message":"Consider: \n input \u003d \u0027\\n\u0027.join(input) if input else None\n self.execute(cmd, root_helper\u003droot_helper, process_input\u003dinput)","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            LOG.debug(\u0027Got semaphore / lock ipset\u0027)"},{"line_number":34,"context_line":"            if input is None:"},{"line_number":35,"context_line":"                self.execute(cmd, root_helper\u003droot_helper)"},{"line_number":36,"context_line":"            else:"},{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_08b4f7f0","line":34,"in_reply_to":"fa98f980_eef26305","updated":"2014-09-08 15:57:26.000000000","message":"good suggestion","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"},{"line_number":38,"context_line":"                             process_input\u003d\u0027\\n\u0027.join(input))"},{"line_number":39,"context_line":"        finally:"},{"line_number":40,"context_line":"            LOG.debug(\u0027Semaphore / lock released ipset\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_4ed9af82","line":40,"updated":"2014-09-08 13:49:56.000000000","message":"ditto on logging","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":37,"context_line":"                self.execute(cmd, root_helper\u003droot_helper,"},{"line_number":38,"context_line":"                             process_input\u003d\u0027\\n\u0027.join(input))"},{"line_number":39,"context_line":"        finally:"},{"line_number":40,"context_line":"            LOG.debug(\u0027Semaphore / lock released ipset\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":43,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_c8a02fa8","line":40,"in_reply_to":"fa98f980_4ed9af82","updated":"2014-09-08 15:57:26.000000000","message":"ok","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"645c6f54b2ef1c45bd7b2182d66a52ac0e9b3f9f","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from neutron.common import utils"},{"line_number":16,"context_line":"from neutron.openstack.common import log as logging"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class IpsetManager(object):"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_e307917f","line":18,"updated":"2014-09-10 13:09:10.000000000","message":"comment by jlibosva via IRC: remove this. LOG not used.","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from neutron.common import utils"},{"line_number":16,"context_line":"from neutron.openstack.common import log as logging"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class IpsetManager(object):"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_262447ae","line":18,"in_reply_to":"fa98f980_e307917f","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from neutron.common import utils"},{"line_number":16,"context_line":"from neutron.openstack.common import log as logging"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class IpsetManager(object):"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_a32ae98d","line":18,"in_reply_to":"fa98f980_e307917f","updated":"2014-09-10 13:26:40.000000000","message":"yes, it does not used, can be removed","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":18,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_90e52daa","line":21,"updated":"2014-09-10 12:35:25.000000000","message":"I know we\u0027re short on time but could you add unittests for this?","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":18,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_237279ad","line":21,"in_reply_to":"fa98f980_90e52daa","updated":"2014-09-10 12:54:55.000000000","message":"Where do you believe unitests add value to current (follow up) functional tests?","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"},{"line_number":25,"context_line":"        self.execute \u003d _execute or linux_utils.execute"},{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_d02b35e9","line":24,"updated":"2014-09-10 12:35:25.000000000","message":"Why _execute and not execute?","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"},{"line_number":25,"context_line":"        self.execute \u003d _execute or linux_utils.execute"},{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_a6b4b735","line":24,"in_reply_to":"fa98f980_43af7de3","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"},{"line_number":25,"context_line":"        self.execute \u003d _execute or linux_utils.execute"},{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_060ae320","line":24,"in_reply_to":"fa98f980_43af7de3","updated":"2014-09-10 13:26:40.000000000","message":"agree","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":21,"context_line":"class IpsetManager(object):"},{"line_number":22,"context_line":"    \"\"\"Wrapper for ipset.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self, _execute\u003dNone, root_helper\u003dNone):"},{"line_number":25,"context_line":"        self.execute \u003d _execute or linux_utils.execute"},{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_43af7de3","line":24,"in_reply_to":"fa98f980_d02b35e9","updated":"2014-09-10 12:54:55.000000000","message":"+1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a7ece3cf34da10e71144b414860d82db71dcd73e","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":29,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":30,"context_line":"        input \u003d \u0027\\n\u0027.join(input) if input else None"},{"line_number":31,"context_line":"        self.execute(cmd, root_helper\u003droot_helper, process_input\u003dinput)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_d0bc156f","line":29,"updated":"2014-09-10 12:38:19.000000000","message":"Why not using self.root_helper instead of having a parameter? You\u0027re always passing self.root_helper","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":29,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":30,"context_line":"        input \u003d \u0027\\n\u0027.join(input) if input else None"},{"line_number":31,"context_line":"        self.execute(cmd, root_helper\u003droot_helper, process_input\u003dinput)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_c6829b51","line":29,"in_reply_to":"fa98f980_63aac1d3","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":29,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":30,"context_line":"        input \u003d \u0027\\n\u0027.join(input) if input else None"},{"line_number":31,"context_line":"        self.execute(cmd, root_helper\u003droot_helper, process_input\u003dinput)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_660e4f34","line":29,"in_reply_to":"fa98f980_63aac1d3","updated":"2014-09-10 13:26:40.000000000","message":"agree","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    @utils.synchronized(\u0027ipset\u0027, external\u003dTrue)"},{"line_number":29,"context_line":"    def _apply(self, cmd, root_helper, input\u003dNone):"},{"line_number":30,"context_line":"        input \u003d \u0027\\n\u0027.join(input) if input else None"},{"line_number":31,"context_line":"        self.execute(cmd, root_helper\u003droot_helper, process_input\u003dinput)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_63aac1d3","line":29,"in_reply_to":"fa98f980_d0bc156f","updated":"2014-09-10 12:54:55.000000000","message":"+1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":43,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":44,"context_line":"        args \u003d []"},{"line_number":45,"context_line":"        add_rules \u003d []"},{"line_number":46,"context_line":"        ethertype \u003d self._get_ipset_chain_tpye(chain_name)"},{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_301001a8","line":44,"updated":"2014-09-10 12:35:25.000000000","message":"Not needed","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":42,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":43,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":44,"context_line":"        args \u003d []"},{"line_number":45,"context_line":"        add_rules \u003d []"},{"line_number":46,"context_line":"        ethertype \u003d self._get_ipset_chain_tpye(chain_name)"},{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_b072310f","line":45,"updated":"2014-09-10 12:35:25.000000000","message":"Not needed","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        args \u003d []"},{"line_number":45,"context_line":"        add_rules \u003d []"},{"line_number":46,"context_line":"        ethertype \u003d self._get_ipset_chain_tpye(chain_name)"},{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_d01255b3","line":47,"updated":"2014-09-10 12:35:25.000000000","message":"args \u003d [\u0027create %s hash:ip family %s\u0027 % (new_chain_name, ethertype))]","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        args \u003d []"},{"line_number":45,"context_line":"        add_rules \u003d []"},{"line_number":46,"context_line":"        ethertype \u003d self._get_ipset_chain_tpye(chain_name)"},{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_c6261bba","line":47,"in_reply_to":"fa98f980_83b96530","updated":"2014-09-10 13:26:40.000000000","message":"agree","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        args \u003d []"},{"line_number":45,"context_line":"        add_rules \u003d []"},{"line_number":46,"context_line":"        ethertype \u003d self._get_ipset_chain_tpye(chain_name)"},{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_83b96530","line":47,"in_reply_to":"fa98f980_d01255b3","updated":"2014-09-10 12:54:55.000000000","message":"+1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"},{"line_number":51,"context_line":"        args.extend(add_rules)"},{"line_number":52,"context_line":"        self._restore_ipset_chain_by_name(args)"},{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_9077ed01","line":50,"updated":"2014-09-10 12:35:25.000000000","message":"Add it directly to args\n\n args.append(\u0027add %s %s\u0027 % (new_chain_name, ip))","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"},{"line_number":51,"context_line":"        args.extend(add_rules)"},{"line_number":52,"context_line":"        self._restore_ipset_chain_by_name(args)"},{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_0621039f","line":50,"in_reply_to":"fa98f980_037d756e","updated":"2014-09-10 13:26:40.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        args.append(\u0027 \u0027.join("},{"line_number":48,"context_line":"            [\u0027create\u0027, new_chain_name, \u0027hash:ip family\u0027, ethertype]))"},{"line_number":49,"context_line":"        for ip in member_ips:"},{"line_number":50,"context_line":"            add_rules.append(\u0027 \u0027.join([\u0027add\u0027, new_chain_name, ip]))"},{"line_number":51,"context_line":"        args.extend(add_rules)"},{"line_number":52,"context_line":"        self._restore_ipset_chain_by_name(args)"},{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_037d756e","line":50,"in_reply_to":"fa98f980_9077ed01","updated":"2014-09-10 12:54:55.000000000","message":"more readable, right. +1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39fd43bfdb9ae5f143153b9d2b0eab3b94a9826b","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"},{"line_number":54,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _get_ipset_chain_tpye(self, ethertype):"},{"line_number":57,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":58,"context_line":"        if ethertype.startswith(\u0027IPv6\u0027):"},{"line_number":59,"context_line":"            chain_type \u003d \u0027inet6\u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_909fed5e","line":56,"updated":"2014-09-10 12:35:25.000000000","message":"Typo: tpye -\u003e type","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"},{"line_number":54,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _get_ipset_chain_tpye(self, ethertype):"},{"line_number":57,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":58,"context_line":"        if ethertype.startswith(\u0027IPv6\u0027):"},{"line_number":59,"context_line":"            chain_type \u003d \u0027inet6\u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_e6295f8a","line":56,"in_reply_to":"fa98f980_83908596","updated":"2014-09-10 13:26:40.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self._swap_ipset_chains(new_chain_name, chain_name)"},{"line_number":54,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _get_ipset_chain_tpye(self, ethertype):"},{"line_number":57,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":58,"context_line":"        if ethertype.startswith(\u0027IPv6\u0027):"},{"line_number":59,"context_line":"            chain_type \u003d \u0027inet6\u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_83908596","line":56,"in_reply_to":"fa98f980_909fed5e","updated":"2014-09-10 12:54:55.000000000","message":"+1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c1f04f0f872fb92421cc02ca5ad4674d7ff1117e","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":31,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"},{"line_number":32,"context_line":"        cmd.append(self._get_ipset_chain_type(ethertype))"},{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa98f980_fc92d249","line":32,"updated":"2014-09-10 14:39:45.000000000","message":"I somehow missed this in previous patchset, sorry :) suggestion:\n\n chain_type \u003d self._get_ipset_chain_type(ethertype)\n cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027, chain_type]","commit_id":"0385c727041bdd4159ccf4a8b355c0442a006ed5"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"923e06f4304980a3fe89a6dd4043951e4b47de5f","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":31,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027]"},{"line_number":32,"context_line":"        cmd.append(self._get_ipset_chain_type(ethertype))"},{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa98f980_5c891ec9","line":32,"in_reply_to":"fa98f980_fc92d249","updated":"2014-09-10 15:41:54.000000000","message":"Your suggestion will require an extra line (\u003e80 linebreak) :D ;)\n\nor yet ...\n\ncmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027,\n       self._get_ipset_chain_type(ethertype)]","commit_id":"0385c727041bdd4159ccf4a8b355c0442a006ed5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        ethertype \u003d self._get_ipset_chain_type(chain_name)"},{"line_number":42,"context_line":"        process_input \u003d [\"create %s hash:ip family %s\" % (new_chain_name,"},{"line_number":43,"context_line":"                                                          ethertype)]"},{"line_number":44,"context_line":"        for ip in member_ips:"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_295b1ef7","line":41,"updated":"2014-09-11 09:47:10.000000000","message":"looks really confusing comparing to #32: accepting ethertype and returning family first time and here accepting chain_name and returning ethertype. Please consider renaming parameters","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        ethertype \u003d self._get_ipset_chain_type(chain_name)"},{"line_number":42,"context_line":"        process_input \u003d [\"create %s hash:ip family %s\" % (new_chain_name,"},{"line_number":43,"context_line":"                                                          ethertype)]"},{"line_number":44,"context_line":"        for ip in member_ips:"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_bdb82812","line":41,"in_reply_to":"fa98f980_295b1ef7","updated":"2014-09-11 11:10:10.000000000","message":"Hmm, yes, it\u0027s assuming the internal implementation of the top module, which\nprepends IPv4/6 to the chain_name, may be we should just provide the ethertype parameter, I\u0027ll check if that\u0027s possible.","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        ethertype \u003d self._get_ipset_chain_type(chain_name)"},{"line_number":42,"context_line":"        process_input \u003d [\"create %s hash:ip family %s\" % (new_chain_name,"},{"line_number":43,"context_line":"                                                          ethertype)]"},{"line_number":44,"context_line":"        for ip in member_ips:"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_c2642724","line":41,"in_reply_to":"fa98f980_295b1ef7","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"4d9995e3a86b6a27a425539dbdb6edfd939ec228","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        ethertype \u003d \u0027IPv6\u0027 if chain_name.startswith(\u0027IPv6\u0027) else \u0027IPv4\u0027"},{"line_number":42,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_d39cfbc2","line":39,"updated":"2014-09-11 12:28:29.000000000","message":"I believe we better add \"ethertype\" as another parameter. We are using knowledge about the top-module implementation (iptables_firewall.py) which will prepend the ethertype in front.\n\nThis way the module will be more independent and reusable.","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"f13ec29975a20ec980e6866ba87e3f3d4652fa62","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _get_ipset_chain_type(self, ethertype):"},{"line_number":53,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":54,"context_line":"        if ethertype \u003d\u003d \u0027IPv6\u0027:"},{"line_number":55,"context_line":"            chain_type \u003d \u0027inet6\u0027"},{"line_number":56,"context_line":"        return chain_type"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_1d3934f2","line":53,"updated":"2014-09-11 11:32:22.000000000","message":"nit: return \u0027inet6\u0027 if ethertype \u003d\u003d \u0027IPv6\u0027 else \u0027inet\u0027","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"4d9995e3a86b6a27a425539dbdb6edfd939ec228","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _get_ipset_chain_type(self, ethertype):"},{"line_number":53,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":54,"context_line":"        if ethertype \u003d\u003d \u0027IPv6\u0027:"},{"line_number":55,"context_line":"            chain_type \u003d \u0027inet6\u0027"},{"line_number":56,"context_line":"        return chain_type"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_73942f8b","line":53,"in_reply_to":"fa98f980_1d3934f2","updated":"2014-09-11 12:28:29.000000000","message":"Done","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"1a2ed8179d610ccc22989baa5784b9d4082edd95","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self.destroy_ipset_chain_by_name(new_chain_name)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _get_ipset_chain_type(self, ethertype):"},{"line_number":53,"context_line":"        chain_type \u003d \u0027inet\u0027"},{"line_number":54,"context_line":"        if ethertype \u003d\u003d \u0027IPv6\u0027:"},{"line_number":55,"context_line":"            chain_type \u003d \u0027inet6\u0027"},{"line_number":56,"context_line":"        return chain_type"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_d3debb10","line":53,"in_reply_to":"fa98f980_1d3934f2","updated":"2014-09-11 12:25:23.000000000","message":"got it","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9790c1ba8bf1aa1880219fd141f4abcd964869c6","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        self.execute(cmd, root_helper\u003dself.root_helper, process_input\u003dinput)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":31,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027,"},{"line_number":32,"context_line":"               self._get_ipset_chain_type(ethertype)]"},{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_e1e4b40e","line":31,"updated":"2014-09-11 22:00:58.000000000","message":"Why are you using -N here but \u0027create\u0027 below?  And \u0027iphash\u0027 vs \u0027hash:ip\u0027?  Should be consistent.\n\nAlso, hash:ip can\u0027t be used for 0.0.0.0 - is it possible to have \"add\" called with this value?","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9cdbb93e8c8e8d62ca6542f8c624cb0e328b5ae8","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        self.execute(cmd, root_helper\u003dself.root_helper, process_input\u003dinput)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def create_ipset_chain(self, chain_name, ethertype):"},{"line_number":31,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-N\u0027, \u0027-exist\u0027, chain_name, \u0027iphash\u0027, \u0027family\u0027,"},{"line_number":32,"context_line":"               self._get_ipset_chain_type(ethertype)]"},{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_c73a60cc","line":31,"in_reply_to":"fa98f980_e1e4b40e","updated":"2014-09-11 22:24:06.000000000","message":"Changing into create to have it standarized. Done.\n\nIt won\u0027t be called with 0.0.0.0, that would mean that any port on a SG has IP 0.0.0.0 which is an invalid address.","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9790c1ba8bf1aa1880219fd141f4abcd964869c6","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_01ee18ed","line":36,"updated":"2014-09-11 22:00:58.000000000","message":"And -A instead of \u0027add\u0027","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9cdbb93e8c8e8d62ca6542f8c624cb0e328b5ae8","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self._apply(cmd)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def add_member_to_ipset_chain(self, chain_name, member_ip):"},{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027-A\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_0731c8ec","line":36,"in_reply_to":"fa98f980_01ee18ed","updated":"2014-09-11 22:24:06.000000000","message":"Done","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9790c1ba8bf1aa1880219fd141f4abcd964869c6","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"},{"line_number":42,"context_line":"        process_input \u003d [\"create %s hash:ip family %s\" % (new_chain_name,"},{"line_number":43,"context_line":"                                                          chain_type)]"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_414bc086","line":40,"updated":"2014-09-11 22:00:58.000000000","message":"Is it possible to have two threads in this code at once?  If so, wouldn\u0027t they possibly interfere with each other since the only lock is held during \"apply\", but you could have:\n\nthread1 - restore\nthread2 - restore\nthread1 - swap (just swapped wrong ipset)\n\nI could be wrong.","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9cdbb93e8c8e8d62ca6542f8c624cb0e328b5ae8","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"},{"line_number":40,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":41,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"},{"line_number":42,"context_line":"        process_input \u003d [\"create %s hash:ip family %s\" % (new_chain_name,"},{"line_number":43,"context_line":"                                                          chain_type)]"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_c768a0bc","line":40,"in_reply_to":"fa98f980_414bc086","updated":"2014-09-11 22:24:06.000000000","message":"Not in the current implementation of the agent, but I will add a @synchronized to avoid problems if the agent implementation ever went multi-threaded.\n\nGood catch btw :)","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"65b3502cfb2de7717b1e5c3932b2b2010e5ef5cd","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027add\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.synchronized(\u0027ipset-refresh\u0027, external\u003dTrue)"},{"line_number":40,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"},{"line_number":41,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":42,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa98f980_334fee35","line":39,"updated":"2014-09-12 01:26:41.000000000","message":"Not to pick at this some more, but now you\u0027ve got two locks, and since they\u0027re \"external\", that means two flock() calls in this code path.\n\nI\u0027m not sure you can use the same lock name, since the second call might block, but I\u0027m thinking the best thing would be to add the decorator to all the methods that call _apply(), just not the _ ones since they should only be called from in this class.\n\nDoes that make sense?\n\nUnless you can remove the external\u003dTrue from one or both - will there be two processes/daemons fighting over this?","commit_id":"720974ec13ad6c3ba1c4d82f65dc25a87a5961c6"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9881a88a26a06946ffac669a27f9b71a4960f7ea","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027add\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.synchronized(\u0027ipset-refresh\u0027, external\u003dTrue)"},{"line_number":40,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"},{"line_number":41,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":42,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa98f980_d302223d","line":39,"in_reply_to":"fa98f980_334fee35","updated":"2014-09-12 01:59:35.000000000","message":"in the current implementation of the agent, there is only one process to call it. I think your suggestion that adding the decorator to all the methods that call _apply() make sense, because in patch:120806, we will move ipset_chains from iptables_firewall.py to ipset_manager.py","commit_id":"720974ec13ad6c3ba1c4d82f65dc25a87a5961c6"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f404c92140f2663b8beddff910a854761e9e91a4","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        cmd \u003d [\u0027ipset\u0027, \u0027add\u0027, \u0027-exist\u0027, chain_name, member_ip]"},{"line_number":37,"context_line":"        self._apply(cmd)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.synchronized(\u0027ipset-refresh\u0027, external\u003dTrue)"},{"line_number":40,"context_line":"    def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype):"},{"line_number":41,"context_line":"        new_chain_name \u003d chain_name + \u0027-new\u0027"},{"line_number":42,"context_line":"        chain_type \u003d self._get_ipset_chain_type(ethertype)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa98f980_1e610e39","line":39,"in_reply_to":"fa98f980_d302223d","updated":"2014-09-12 06:24:14.000000000","message":"It makes sense to me actually. We get the same effect if we synch on all the publicly exposed methods and remove the apply one.\n\nThank you  Brian.","commit_id":"720974ec13ad6c3ba1c4d82f65dc25a87a5961c6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bce5158bafd137444e9492aca791c569b9a1474a","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    def destroy_ipset_chain_by_name(self, chain_name):"},{"line_number":56,"context_line":"        self._destroy_ipset_chain(chain_name)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _apply(self, cmd, input\u003dNone):"},{"line_number":59,"context_line":"        input \u003d \u0027\\n\u0027.join(input) if input else None"},{"line_number":60,"context_line":"        self.execute(cmd, root_helper\u003dself.root_helper, process_input\u003dinput)"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"fa98f980_0642983d","line":58,"updated":"2014-09-12 13:30:54.000000000","message":"I think this looks better, thanks Miguel.  My only nit is to put a comment here mentioning it\u0027s the responsibility of the caller to hold the ipset \"lock\".  I wouldn\u0027t hold the review for that though.","commit_id":"2562a9271c828e982a74593e8fd07be13b0cfc4a"}],"neutron/agent/linux/iptables_firewall.py":[{"author":{"_account_id":7987,"name":"Ivar Lazzaro","email":"ivarlazzaro@gmail.com","username":"Ivar"},"change_message_id":"9cf48cd05e4e8f853f24b3262decb9e4c6f9e0dd","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                                     ipv4_iptables_rule,"},{"line_number":338,"context_line":"                                     ipv6_iptables_rule)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def _get_cur_sg_member_ips(self, sg_id, ethertype):"},{"line_number":341,"context_line":"        if (sg_id in self.sg_members"},{"line_number":342,"context_line":"            and ethertype in self.sg_members[sg_id]):"},{"line_number":343,"context_line":"            cur_member_ips \u003d self.sg_members[sg_id][ethertype]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_9b799884","line":340,"updated":"2014-08-29 01:03:27.000000000","message":"nit, could be:\n\nreturn self.sg_members.get(sg_id, {}).get(ethertype, [])","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"971d9667b128217c6ee0142b65365e70a556deda","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                                     ipv4_iptables_rule,"},{"line_number":338,"context_line":"                                     ipv6_iptables_rule)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def _get_cur_sg_member_ips(self, sg_id, ethertype):"},{"line_number":341,"context_line":"        if (sg_id in self.sg_members"},{"line_number":342,"context_line":"            and ethertype in self.sg_members[sg_id]):"},{"line_number":343,"context_line":"            cur_member_ips \u003d self.sg_members[sg_id][ethertype]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_c81b3671","line":340,"in_reply_to":"1abeadc6_9b799884","updated":"2014-08-29 07:07:27.000000000","message":"thanks for your review and suggestion","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":7987,"name":"Ivar Lazzaro","email":"ivarlazzaro@gmail.com","username":"Ivar"},"change_message_id":"9cf48cd05e4e8f853f24b3262decb9e4c6f9e0dd","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            cur_member_ips \u003d []"},{"line_number":346,"context_line":"        return cur_member_ips"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def _get_pre_sg_member_ips(self, sg_id, ethertype):"},{"line_number":349,"context_line":"        if (sg_id in self.pre_sg_members"},{"line_number":350,"context_line":"            and ethertype in self.pre_sg_members[sg_id]):"},{"line_number":351,"context_line":"            pre_member_ips \u003d self.pre_sg_members[sg_id][ethertype]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_5beaa00f","line":348,"updated":"2014-08-29 01:03:27.000000000","message":"same as above","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":7987,"name":"Ivar Lazzaro","email":"ivarlazzaro@gmail.com","username":"Ivar"},"change_message_id":"9cf48cd05e4e8f853f24b3262decb9e4c6f9e0dd","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                    else:"},{"line_number":430,"context_line":"                        continue"},{"line_number":431,"context_line":"                    args +\u003d [\u0027-j RETURN\u0027]"},{"line_number":432,"context_line":"                    iptables_rules +\u003d [\u0027 \u0027.join(args)]"},{"line_number":433,"context_line":"                    continue"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"            args \u003d self._ip_prefix_arg(\u0027s\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_9bda7849","line":432,"updated":"2014-08-29 01:03:27.000000000","message":"L431:432 could be moved under the if statement in L425 and L429:430 removed","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"971d9667b128217c6ee0142b65365e70a556deda","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                    else:"},{"line_number":430,"context_line":"                        continue"},{"line_number":431,"context_line":"                    args +\u003d [\u0027-j RETURN\u0027]"},{"line_number":432,"context_line":"                    iptables_rules +\u003d [\u0027 \u0027.join(args)]"},{"line_number":433,"context_line":"                    continue"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"            args \u003d self._ip_prefix_arg(\u0027s\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_e8e0f227","line":432,"in_reply_to":"1abeadc6_9bda7849","updated":"2014-08-29 07:07:27.000000000","message":"agree","commit_id":"065de47df79399257a649e0ed9162f2e8466837c"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                   EGRESS_DIRECTION: \u0027dst\u0027}"},{"line_number":39,"context_line":"LINUX_DEV_LEN \u003d 14"},{"line_number":40,"context_line":"IPSET_CHAIN_LEN \u003d 20"},{"line_number":41,"context_line":"IPSET_CHAIN_MEMBER_CHANGE \u003d 10"},{"line_number":42,"context_line":"IPSET_CHAIN_MEMBER_ADD \u003d 5"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_6415403f","line":41,"updated":"2014-09-03 11:11:25.000000000","message":"may be more clear for the constant intent:\n\n    IPSET_CHANGE_BULK_THRESHOLD \u003d 10\n    IPSET_ADD_BULK_THRESHOLD\u003d 5","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                   EGRESS_DIRECTION: \u0027dst\u0027}"},{"line_number":39,"context_line":"LINUX_DEV_LEN \u003d 14"},{"line_number":40,"context_line":"IPSET_CHAIN_LEN \u003d 20"},{"line_number":41,"context_line":"IPSET_CHAIN_MEMBER_CHANGE \u003d 10"},{"line_number":42,"context_line":"IPSET_CHAIN_MEMBER_ADD \u003d 5"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_5a565407","line":41,"in_reply_to":"fa98f980_6415403f","updated":"2014-09-03 13:28:48.000000000","message":"agree","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        chain_name \u003d self._port_chain_name(port, direction)"},{"line_number":309,"context_line":"        # select rules for current direction"},{"line_number":310,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":313,"context_line":"            port, direction)"},{"line_number":314,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_b97f55aa","line":311,"updated":"2014-09-03 11:11:25.000000000","message":"nit: unneeded whitespace","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        chain_name \u003d self._port_chain_name(port, direction)"},{"line_number":309,"context_line":"        # select rules for current direction"},{"line_number":310,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":313,"context_line":"            port, direction)"},{"line_number":314,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_5a25745a","line":311,"in_reply_to":"fa98f980_b97f55aa","updated":"2014-09-03 13:28:48.000000000","message":"Done","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":313,"context_line":"            port, direction)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":316,"context_line":"            remote_sg_ids \u003d port.get(\u0027security_group_source_groups\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_f94c7da0","line":313,"updated":"2014-09-03 11:11:25.000000000","message":"nit: unneeded break into two lines.","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":313,"context_line":"            port, direction)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":316,"context_line":"            remote_sg_ids \u003d port.get(\u0027security_group_source_groups\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_3a343092","line":313,"in_reply_to":"fa98f980_f94c7da0","updated":"2014-09-03 13:28:48.000000000","message":"Done","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                else:"},{"line_number":382,"context_line":"                    if (len(add_ips) + len(del_ips)"},{"line_number":383,"context_line":"                            \u003c IPSET_CHAIN_MEMBER_CHANGE):"},{"line_number":384,"context_line":"                        for add_ip in add_ips:"},{"line_number":385,"context_line":"                            if add_ip not in self.ipset_chains[chain_name]:"},{"line_number":386,"context_line":"                                self.ipset.add_member_to_ipset_chain("},{"line_number":387,"context_line":"                                    chain_name, add_ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_44a91cde","line":384,"updated":"2014-09-03 11:11:25.000000000","message":"I see replication with line 372\n\nI would refactor the loop into a function.\n\nself._add_ips_to_ipset_chain(add_ips, chain_name)","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                else:"},{"line_number":382,"context_line":"                    if (len(add_ips) + len(del_ips)"},{"line_number":383,"context_line":"                            \u003c IPSET_CHAIN_MEMBER_CHANGE):"},{"line_number":384,"context_line":"                        for add_ip in add_ips:"},{"line_number":385,"context_line":"                            if add_ip not in self.ipset_chains[chain_name]:"},{"line_number":386,"context_line":"                                self.ipset.add_member_to_ipset_chain("},{"line_number":387,"context_line":"                                    chain_name, add_ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_9a4d1c41","line":384,"in_reply_to":"fa98f980_44a91cde","updated":"2014-09-03 13:28:48.000000000","message":"it is good to refactor these to a function!","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                self.ipset.add_member_to_ipset_chain("},{"line_number":387,"context_line":"                                    chain_name, add_ip)"},{"line_number":388,"context_line":"                                self.ipset_chains[chain_name].append(add_ip)"},{"line_number":389,"context_line":"                        for del_ip in del_ips:"},{"line_number":390,"context_line":"                            if (chain_name in self.ipset_chains"},{"line_number":391,"context_line":"                                and del_ip in self.ipset_chains[chain_name]):"},{"line_number":392,"context_line":"                                self.ipset.del_ipset_chain_member("}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_247b387c","line":389,"updated":"2014-09-03 11:11:25.000000000","message":"same for self._del_ips_from_ipset_chain(del_ips, chain_name)","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                self.ipset.add_member_to_ipset_chain("},{"line_number":387,"context_line":"                                    chain_name, add_ip)"},{"line_number":388,"context_line":"                                self.ipset_chains[chain_name].append(add_ip)"},{"line_number":389,"context_line":"                        for del_ip in del_ips:"},{"line_number":390,"context_line":"                            if (chain_name in self.ipset_chains"},{"line_number":391,"context_line":"                                and del_ip in self.ipset_chains[chain_name]):"},{"line_number":392,"context_line":"                                self.ipset.del_ipset_chain_member("}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_1a612ca9","line":389,"in_reply_to":"fa98f980_247b387c","updated":"2014-09-03 13:28:48.000000000","message":"agree","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d1dfb17d2d80d5be2cfa832067aaacb6a548b9e5","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                    chain_name, del_ip)"},{"line_number":394,"context_line":"                                self.ipset_chains[chain_name].remove(del_ip)"},{"line_number":395,"context_line":"                    else:"},{"line_number":396,"context_line":"                        self.ipset.refresh_ipset_chain_by_name("},{"line_number":397,"context_line":"                            chain_name, cur_member_ips)"},{"line_number":398,"context_line":"                        self.ipset_chains[chain_name] \u003d cur_member_ips"},{"line_number":399,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_64746068","line":396,"updated":"2014-09-03 11:11:25.000000000","message":"To match the two above sugestions, may be:\n\nself._bulk_set_ips_into_chain(ip_addresses, chain_name) ?","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"aecd1feb4204f07a9059373b670de9c2ab5065c2","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                    chain_name, del_ip)"},{"line_number":394,"context_line":"                                self.ipset_chains[chain_name].remove(del_ip)"},{"line_number":395,"context_line":"                    else:"},{"line_number":396,"context_line":"                        self.ipset.refresh_ipset_chain_by_name("},{"line_number":397,"context_line":"                            chain_name, cur_member_ips)"},{"line_number":398,"context_line":"                        self.ipset_chains[chain_name] \u003d cur_member_ips"},{"line_number":399,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa98f980_5a5734ca","line":396,"in_reply_to":"fa98f980_64746068","updated":"2014-09-03 13:28:48.000000000","message":"agree","commit_id":"4be1a7ea72748c3a4a4dedd2cc7c6dec8774fdfc"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"53eff3b978e11ca07bbe8be6d0d326fbaab601b7","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.iptables \u003d iptables_manager.IptablesManager("},{"line_number":53,"context_line":"            root_helper\u003dself.root_helper,"},{"line_number":54,"context_line":"            use_ipv6\u003dipv6_utils.is_enabled())"},{"line_number":55,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":56,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":57,"context_line":"        # list of port which has security group"},{"line_number":58,"context_line":"        self.filtered_ports \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_d451b00f","line":55,"updated":"2014-09-08 14:17:50.000000000","message":"Please create a new class inheriting IptablesFirewallDriver that extends functionality of ipset. This way you will get rid of repeating \"if cfg.CONF.SECURITYGROUP.enable_ipset\" in lots of methods. The if can be called just once when choosing which driver should be used.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.iptables \u003d iptables_manager.IptablesManager("},{"line_number":53,"context_line":"            root_helper\u003dself.root_helper,"},{"line_number":54,"context_line":"            use_ipv6\u003dipv6_utils.is_enabled())"},{"line_number":55,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":56,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":57,"context_line":"        # list of port which has security group"},{"line_number":58,"context_line":"        self.filtered_ports \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_08f3170b","line":55,"in_reply_to":"fa98f980_5d9243a0","updated":"2014-09-08 15:57:26.000000000","message":"thanks, I agree to refactor in Kilo","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"2bed7480191e86a4bed8b160d60c3eef0f721f54","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.iptables \u003d iptables_manager.IptablesManager("},{"line_number":53,"context_line":"            root_helper\u003dself.root_helper,"},{"line_number":54,"context_line":"            use_ipv6\u003dipv6_utils.is_enabled())"},{"line_number":55,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":56,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":57,"context_line":"        # list of port which has security group"},{"line_number":58,"context_line":"        self.filtered_ports \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_5d9243a0","line":55,"in_reply_to":"fa98f980_948c8881","updated":"2014-09-08 15:25:30.000000000","message":"Given the timeframe (FFEs need to merge by 8-12-2014), I think allowing this to merge as-is and then refactoring early in Kilo would be an ok option.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"5e83cfa643593b0c0c7010f05bfc94d38c94e62f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.iptables \u003d iptables_manager.IptablesManager("},{"line_number":53,"context_line":"            root_helper\u003dself.root_helper,"},{"line_number":54,"context_line":"            use_ipv6\u003dipv6_utils.is_enabled())"},{"line_number":55,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":56,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":57,"context_line":"        # list of port which has security group"},{"line_number":58,"context_line":"        self.filtered_ports \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_948c8881","line":55,"in_reply_to":"fa98f980_d451b00f","updated":"2014-09-08 15:21:45.000000000","message":"+1, Hmm.... but I\u0027m unsure how should we develop \u0026 roll this out to devstack/CI. May be it\u0027s worth asking about it on the list.\n\nI mean, I suppose this needs a change on devstack to set the new driver during testing. But that can\u0027t be set until we have a working version of the driver.\n\nPossible algorithm for this:\n\nMay be it\u0027s ok to develop with the setting, merge, then refactor into driver, and try to make the change into devstack, if devstack tests pass, then we\u0027re good, else fix.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c3e86703423e5163ef076f041ac3d59916d45a73","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.sg_members \u003d {}"},{"line_number":67,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":68,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":69,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":70,"context_line":"            self.ipset.force_flush_all_ipset_chains()"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_ce991f21","line":69,"updated":"2014-09-08 13:49:56.000000000","message":"nit: consider initialize once like:\n self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset\nand use shortened version in code","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fa66b82df87a855a2e4d11f4a2d80eba0c6e176e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.sg_members \u003d {}"},{"line_number":67,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":68,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":69,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":70,"context_line":"            self.ipset.force_flush_all_ipset_chains()"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_f16c060b","line":69,"in_reply_to":"fa98f980_ce991f21","updated":"2014-09-08 14:18:34.000000000","message":"I was talking with jlibosva about factoring this out into another driver (which inherits fro IptablesFirewallDriver) like \"IpsetIptablesFirewallDriver\" and \"OVSHybrid..\"\n\nSo it composes over the iptables one, and we remove all the if ipset_, if ipset_ everywhere.\n\nWhat do you think about this? Shihanzhang, I think I could help on that, doesn\u0027t look very dificult.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0cae561af98ecf554f9eae56dafd43d8d3c14a7e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.sg_members \u003d {}"},{"line_number":67,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":68,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":69,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":70,"context_line":"            self.ipset.force_flush_all_ipset_chains()"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_f7f23605","line":69,"in_reply_to":"fa98f980_f16c060b","updated":"2014-09-08 14:44:25.000000000","message":"+1","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.sg_members \u003d {}"},{"line_number":67,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":68,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":69,"context_line":"        if cfg.CONF.SECURITYGROUP.enable_ipset:"},{"line_number":70,"context_line":"            self.ipset.force_flush_all_ipset_chains()"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_28461b24","line":69,"in_reply_to":"fa98f980_f16c060b","updated":"2014-09-08 15:57:26.000000000","message":"I agree you to modify this, now I can\u0027t commit patch in my home, can you help me modify it now?","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fa66b82df87a855a2e4d11f4a2d80eba0c6e176e","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":388,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":390,"context_line":"                cur_member_ips \u003d self.sg_members[sg_id][ethertype]"},{"line_number":391,"context_line":"                chain_name \u003d ethertype + sg_id[:IPSET_CHAIN_LEN]"},{"line_number":392,"context_line":"                if chain_name not in self.ipset_chains:"},{"line_number":393,"context_line":"                    if ethertype in self.sg_members[sg_id]:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_b117de53","line":390,"updated":"2014-09-08 14:18:34.000000000","message":"please use:\ncur_member_ips \u003d self._get_cur_sg_member_ips(sg_id, ethertype)\n\n\nhttp://logs.openstack.org/77/111877/14/check/check-tempest-dsvm-neutron-full/e99a993/logs/screen-q-agt.txt.gz#_2014-09-08_03_35_19_738\n\nOtherwise it fails sometimes.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":388,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":390,"context_line":"                cur_member_ips \u003d self.sg_members[sg_id][ethertype]"},{"line_number":391,"context_line":"                chain_name \u003d ethertype + sg_id[:IPSET_CHAIN_LEN]"},{"line_number":392,"context_line":"                if chain_name not in self.ipset_chains:"},{"line_number":393,"context_line":"                    if ethertype in self.sg_members[sg_id]:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_686e6398","line":390,"in_reply_to":"fa98f980_b117de53","updated":"2014-09-08 15:57:26.000000000","message":"agree","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fa66b82df87a855a2e4d11f4a2d80eba0c6e176e","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":390,"context_line":"                cur_member_ips \u003d self.sg_members[sg_id][ethertype]"},{"line_number":391,"context_line":"                chain_name \u003d ethertype + sg_id[:IPSET_CHAIN_LEN]"},{"line_number":392,"context_line":"                if chain_name not in self.ipset_chains:"},{"line_number":393,"context_line":"                    if ethertype in self.sg_members[sg_id]:"},{"line_number":394,"context_line":"                        if cur_member_ips:"},{"line_number":395,"context_line":"                            self.ipset_chains[chain_name] \u003d []"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_34391cd9","line":392,"updated":"2014-09-08 14:18:34.000000000","message":"I believe we could just use \"bulk\" always the first time, after agent restart, or always?, and this way we can avoid the \"flush all\" problem at init.","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":390,"context_line":"                cur_member_ips \u003d self.sg_members[sg_id][ethertype]"},{"line_number":391,"context_line":"                chain_name \u003d ethertype + sg_id[:IPSET_CHAIN_LEN]"},{"line_number":392,"context_line":"                if chain_name not in self.ipset_chains:"},{"line_number":393,"context_line":"                    if ethertype in self.sg_members[sg_id]:"},{"line_number":394,"context_line":"                        if cur_member_ips:"},{"line_number":395,"context_line":"                            self.ipset_chains[chain_name] \u003d []"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_8802a7b2","line":392,"in_reply_to":"fa98f980_34391cd9","updated":"2014-09-08 15:57:26.000000000","message":"agree","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":1923,"name":"Mohammad Banikazemi","email":"mbanikazemi@gmail.com","username":"mb-s"},"change_message_id":"9fe16432ff2300f350f64206991c71b8e59843be","unresolved":false,"context_lines":[{"line_number":434,"context_line":"                    direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":435,"context_line":"                    ethertype \u003d rule.get(\u0027ethertype\u0027)"},{"line_number":436,"context_line":"                    # the length of ipset chain name require less than 31"},{"line_number":437,"context_line":"                    # charactors"},{"line_number":438,"context_line":"                    ipset_chain_name \u003d ("},{"line_number":439,"context_line":"                        ethertype + remote_gid[:IPSET_CHAIN_LEN])"},{"line_number":440,"context_line":"                    if ipset_chain_name in self.ipset_chains:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_b4498c90","line":437,"updated":"2014-09-08 14:38:38.000000000","message":"typo: characters","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"773944262d3cca184c2748e7c6d6a82f6b61ec6e","unresolved":false,"context_lines":[{"line_number":434,"context_line":"                    direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":435,"context_line":"                    ethertype \u003d rule.get(\u0027ethertype\u0027)"},{"line_number":436,"context_line":"                    # the length of ipset chain name require less than 31"},{"line_number":437,"context_line":"                    # charactors"},{"line_number":438,"context_line":"                    ipset_chain_name \u003d ("},{"line_number":439,"context_line":"                        ethertype + remote_gid[:IPSET_CHAIN_LEN])"},{"line_number":440,"context_line":"                    if ipset_chain_name in self.ipset_chains:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_282b7b31","line":437,"in_reply_to":"fa98f980_b4498c90","updated":"2014-09-08 15:57:26.000000000","message":"Done","commit_id":"d893c74ed2caf5c85b42453ff4204f7c6ccfb5e9"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":307,"context_line":"    def _get_remote_sg_ids(self, port, direction):"},{"line_number":308,"context_line":"        sg_ids \u003d port.get(\u0027security_groups\u0027, [])"},{"line_number":309,"context_line":"        remote_sg_ids \u003d []"},{"line_number":310,"context_line":"        for sg_id in sg_ids:"},{"line_number":311,"context_line":"            for rule in self.sg_rules.get(sg_id, []):"},{"line_number":312,"context_line":"                if rule[\u0027direction\u0027] \u003d\u003d direction:"},{"line_number":313,"context_line":"                    remote_group_id \u003d rule.get(\u0027remote_group_id\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_c4cd5ab9","line":310,"updated":"2014-09-09 12:59:48.000000000","message":"this is quite the chain. how about:\n\nremote_sg_ids.extend([rule[\u0027remote_group_id\u0027] \n                      for rule in self.sg_rules.get(sg_id, []) if\n                      rule[\u0027direction\u0027] \u003d\u003d direction and \n                      rule.get(\u0027remote_group_id\u0027)])\n\ni think it looks more readable like this.","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":307,"context_line":"    def _get_remote_sg_ids(self, port, direction):"},{"line_number":308,"context_line":"        sg_ids \u003d port.get(\u0027security_groups\u0027, [])"},{"line_number":309,"context_line":"        remote_sg_ids \u003d []"},{"line_number":310,"context_line":"        for sg_id in sg_ids:"},{"line_number":311,"context_line":"            for rule in self.sg_rules.get(sg_id, []):"},{"line_number":312,"context_line":"                if rule[\u0027direction\u0027] \u003d\u003d direction:"},{"line_number":313,"context_line":"                    remote_group_id \u003d rule.get(\u0027remote_group_id\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_471b29ca","line":310,"in_reply_to":"fa98f980_c4cd5ab9","updated":"2014-09-09 14:24:18.000000000","message":"thanks for your review and good suggestion, I agree","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":376,"context_line":"    def _del_ips_from_ipset_chain(self, chain_name, del_ips):"},{"line_number":377,"context_line":"        for del_ip in del_ips:"},{"line_number":378,"context_line":"            if (chain_name in self.ipset_chains"},{"line_number":379,"context_line":"                and del_ip in self.ipset_chains[chain_name]):"},{"line_number":380,"context_line":"                self.ipset.del_ipset_chain_member("},{"line_number":381,"context_line":"                    chain_name, del_ip)"},{"line_number":382,"context_line":"                self.ipset_chains[chain_name].remove(del_ip)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_8415129d","line":379,"updated":"2014-09-09 12:59:48.000000000","message":"hmm, I don\u0027t think you should always check if chain_name in self.ipset_chains. I don\u0027t think self.ipset_chains will have new chain names during this method. \n\nIf instead, new chains are added to self.ipset_chains (another thread) while this method executes, you might have to consider synchronization.\n\nAnyways, in a thread-safe method, you might want to have instead:\n\n    if chain_name in self.ipset_chains:\n        for del_ip in del_ips:\t377\n            if del_ip in self.ipset_chains[chain_name]:","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":376,"context_line":"    def _del_ips_from_ipset_chain(self, chain_name, del_ips):"},{"line_number":377,"context_line":"        for del_ip in del_ips:"},{"line_number":378,"context_line":"            if (chain_name in self.ipset_chains"},{"line_number":379,"context_line":"                and del_ip in self.ipset_chains[chain_name]):"},{"line_number":380,"context_line":"                self.ipset.del_ipset_chain_member("},{"line_number":381,"context_line":"                    chain_name, del_ip)"},{"line_number":382,"context_line":"                self.ipset_chains[chain_name].remove(del_ip)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_c75d597b","line":379,"in_reply_to":"fa98f980_8415129d","updated":"2014-09-09 14:24:18.000000000","message":"agree","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":438,"context_line":"                                 IPSET_DIRECTION[direction]]"},{"line_number":439,"context_line":"                        args +\u003d [\u0027-j RETURN\u0027]"},{"line_number":440,"context_line":"                        iptables_rules +\u003d [\u0027 \u0027.join(args)]"},{"line_number":441,"context_line":"                    continue"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"            args \u003d self._ip_prefix_arg(\u0027s\u0027,"},{"line_number":444,"context_line":"                                       rule.get(\u0027source_ip_prefix\u0027))"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_1fc40f3f","line":441,"updated":"2014-09-09 12:59:48.000000000","message":"I think this if block could be a separate method. This way, it will be easier to test.","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":438,"context_line":"                                 IPSET_DIRECTION[direction]]"},{"line_number":439,"context_line":"                        args +\u003d [\u0027-j RETURN\u0027]"},{"line_number":440,"context_line":"                        iptables_rules +\u003d [\u0027 \u0027.join(args)]"},{"line_number":441,"context_line":"                    continue"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"            args \u003d self._ip_prefix_arg(\u0027s\u0027,"},{"line_number":444,"context_line":"                                       rule.get(\u0027source_ip_prefix\u0027))"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_87ca7118","line":441,"in_reply_to":"fa98f980_1fc40f3f","updated":"2014-09-09 14:24:18.000000000","message":"got it","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"cb7d150b5426eab75f663c1f28c6924a7ba8158f","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        # TODO(majopela, shihanzhang): refactor out ipset to a separate"},{"line_number":56,"context_line":"        # driver composed over this one"},{"line_number":57,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":58,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":59,"context_line":"        # list of port which has security group"},{"line_number":60,"context_line":"        self.filtered_ports \u003d {}"},{"line_number":61,"context_line":"        self._add_fallback_chain_v4v6()"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_6f5e2d47","line":58,"updated":"2014-09-10 10:30:31.000000000","message":"nit: no space between the parameter name and actual parameter","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        # TODO(majopela, shihanzhang): refactor out ipset to a separate"},{"line_number":56,"context_line":"        # driver composed over this one"},{"line_number":57,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":58,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":59,"context_line":"        # list of port which has security group"},{"line_number":60,"context_line":"        self.filtered_ports \u003d {}"},{"line_number":61,"context_line":"        self._add_fallback_chain_v4v6()"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_4626ab17","line":58,"in_reply_to":"fa98f980_43ddbd6f","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        # TODO(majopela, shihanzhang): refactor out ipset to a separate"},{"line_number":56,"context_line":"        # driver composed over this one"},{"line_number":57,"context_line":"        self.ipset \u003d ipset_manager.IpsetManager("},{"line_number":58,"context_line":"            root_helper \u003d self.root_helper)"},{"line_number":59,"context_line":"        # list of port which has security group"},{"line_number":60,"context_line":"        self.filtered_ports \u003d {}"},{"line_number":61,"context_line":"        self._add_fallback_chain_v4v6()"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_43ddbd6f","line":58,"in_reply_to":"fa98f980_6f5e2d47","updated":"2014-09-10 12:54:55.000000000","message":"nits nits ;D\n\nI thought pep8 complained on that...","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"cb7d150b5426eab75f663c1f28c6924a7ba8158f","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        if security_group_ids is None:"},{"line_number":384,"context_line":"            return"},{"line_number":385,"context_line":"        for sg_id in security_group_ids:"},{"line_number":386,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":387,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":388,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                cur_member_ips \u003d self._get_cur_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_00bbaee5","line":386,"updated":"2014-09-10 10:30:31.000000000","message":"The following block can be flattened using \"if-elif-else\", instead of \"if-else if-else else\".","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        if security_group_ids is None:"},{"line_number":384,"context_line":"            return"},{"line_number":385,"context_line":"        for sg_id in security_group_ids:"},{"line_number":386,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":387,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":388,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                cur_member_ips \u003d self._get_cur_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_03f495f0","line":386,"in_reply_to":"fa98f980_00bbaee5","updated":"2014-09-10 12:54:55.000000000","message":"+1","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        if security_group_ids is None:"},{"line_number":384,"context_line":"            return"},{"line_number":385,"context_line":"        for sg_id in security_group_ids:"},{"line_number":386,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":387,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":388,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                cur_member_ips \u003d self._get_cur_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_866253d6","line":386,"in_reply_to":"fa98f980_03f495f0","updated":"2014-09-10 13:26:40.000000000","message":"agree","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        if security_group_ids is None:"},{"line_number":384,"context_line":"            return"},{"line_number":385,"context_line":"        for sg_id in security_group_ids:"},{"line_number":386,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":387,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":388,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"},{"line_number":389,"context_line":"                cur_member_ips \u003d self._get_cur_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_6621effb","line":386,"in_reply_to":"fa98f980_866253d6","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_eca124f9","line":69,"updated":"2014-09-11 09:47:10.000000000","message":"I\u0027d place this dict in ipset manager and sync it there as it\u0027s more safe. What do you think?","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_7d116051","line":69,"in_reply_to":"fa98f980_eca124f9","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        # select rules for current direction"},{"line_number":319,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":320,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":321,"context_line":"            port, direction)"},{"line_number":322,"context_line":"        if self.enable_ipset:"},{"line_number":323,"context_line":"            remote_sg_ids \u003d self._get_remote_sg_ids(port, direction)"},{"line_number":324,"context_line":"            # update the corresponding ipset chain member"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_290f9e89","line":321,"updated":"2014-09-11 09:47:10.000000000","message":"nit: unneeded change","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        # select rules for current direction"},{"line_number":319,"context_line":"        security_group_rules \u003d self._select_sgr_by_direction(port, direction)"},{"line_number":320,"context_line":"        security_group_rules +\u003d self._select_sg_rules_for_port("},{"line_number":321,"context_line":"            port, direction)"},{"line_number":322,"context_line":"        if self.enable_ipset:"},{"line_number":323,"context_line":"            remote_sg_ids \u003d self._get_remote_sg_ids(port, direction)"},{"line_number":324,"context_line":"            # update the corresponding ipset chain member"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_22aed398","line":321,"in_reply_to":"fa98f980_290f9e89","updated":"2014-09-11 11:13:23.000000000","message":"Done","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    def _get_pre_sg_member_ips(self, sg_id, ethertype):"},{"line_number":352,"context_line":"        return self.pre_sg_members.get(sg_id, {}).get(ethertype, [])"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _get_add_sg_member_ips(self, sg_id, ethertype):"},{"line_number":355,"context_line":"        add_member_ips \u003d (set(self._get_cur_sg_member_ips(sg_id, ethertype)) -"},{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_29d4febb","line":354,"updated":"2014-09-11 09:47:10.000000000","message":"get_add is confusing, may consider get_new...","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    def _get_pre_sg_member_ips(self, sg_id, ethertype):"},{"line_number":352,"context_line":"        return self.pre_sg_members.get(sg_id, {}).get(ethertype, [])"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _get_add_sg_member_ips(self, sg_id, ethertype):"},{"line_number":355,"context_line":"        add_member_ips \u003d (set(self._get_cur_sg_member_ips(sg_id, ethertype)) -"},{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_3d44381a","line":354,"in_reply_to":"fa98f980_29d4febb","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    def _get_pre_sg_member_ips(self, sg_id, ethertype):"},{"line_number":352,"context_line":"        return self.pre_sg_members.get(sg_id, {}).get(ethertype, [])"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _get_add_sg_member_ips(self, sg_id, ethertype):"},{"line_number":355,"context_line":"        add_member_ips \u003d (set(self._get_cur_sg_member_ips(sg_id, ethertype)) -"},{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_a2c2c350","line":354,"in_reply_to":"fa98f980_29d4febb","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    def _get_del_sg_member_ips(self, sg_id, ethertype):"},{"line_number":360,"context_line":"        del_member_ips \u003d (set(self._get_pre_sg_member_ips(sg_id, ethertype)) -"},{"line_number":361,"context_line":"                          set(self._get_cur_sg_member_ips(sg_id, ethertype)))"},{"line_number":362,"context_line":"        return list(del_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_e9f4b65c","line":359,"updated":"2014-09-11 09:47:10.000000000","message":"prefer get_deleted to make code more readable","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    def _get_del_sg_member_ips(self, sg_id, ethertype):"},{"line_number":360,"context_line":"        del_member_ips \u003d (set(self._get_pre_sg_member_ips(sg_id, ethertype)) -"},{"line_number":361,"context_line":"                          set(self._get_cur_sg_member_ips(sg_id, ethertype)))"},{"line_number":362,"context_line":"        return list(del_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_dd5face3","line":359,"in_reply_to":"fa98f980_e9f4b65c","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                          set(self._get_pre_sg_member_ips(sg_id, ethertype)))"},{"line_number":357,"context_line":"        return list(add_member_ips)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    def _get_del_sg_member_ips(self, sg_id, ethertype):"},{"line_number":360,"context_line":"        del_member_ips \u003d (set(self._get_pre_sg_member_ips(sg_id, ethertype)) -"},{"line_number":361,"context_line":"                          set(self._get_cur_sg_member_ips(sg_id, ethertype)))"},{"line_number":362,"context_line":"        return list(del_member_ips)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_62e53b08","line":359,"in_reply_to":"fa98f980_e9f4b65c","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":381,"context_line":"    def _update_ipset_chain_member(self, security_group_ids):"},{"line_number":382,"context_line":"        if security_group_ids is None:"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        for sg_id in security_group_ids:"},{"line_number":385,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":386,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":387,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_acb8fc11","line":384,"updated":"2014-09-11 09:47:10.000000000","message":"nit: for sg_id in security_group_ids or []: and remove 382-383","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":381,"context_line":"    def _update_ipset_chain_member(self, security_group_ids):"},{"line_number":382,"context_line":"        if security_group_ids is None:"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        for sg_id in security_group_ids:"},{"line_number":385,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":386,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":387,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_1d5694ce","line":384,"in_reply_to":"fa98f980_acb8fc11","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":381,"context_line":"    def _update_ipset_chain_member(self, security_group_ids):"},{"line_number":382,"context_line":"        if security_group_ids is None:"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        for sg_id in security_group_ids:"},{"line_number":385,"context_line":"            for ethertype in [\u0027IPv4\u0027, \u0027IPv6\u0027]:"},{"line_number":386,"context_line":"                add_ips \u003d self._get_add_sg_member_ips(sg_id, ethertype)"},{"line_number":387,"context_line":"                del_ips \u003d self._get_del_sg_member_ips(sg_id, ethertype)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_c25fc7e3","line":384,"in_reply_to":"fa98f980_acb8fc11","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":434,"context_line":"            # display them: source/dest, protocol, sport, dport, target"},{"line_number":435,"context_line":"            # Otherwise the iptables_manager code won\u0027t be able to find"},{"line_number":436,"context_line":"            # them to preserve their [packet:byte] counts."},{"line_number":437,"context_line":"            if self.enable_ipset:"},{"line_number":438,"context_line":"                remote_gid \u003d rule.get(\u0027remote_group_id\u0027)"},{"line_number":439,"context_line":"                if remote_gid:"},{"line_number":440,"context_line":"                    iptables_rules.extend("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_cc05407b","line":437,"updated":"2014-09-11 09:47:10.000000000","message":"this block should be placed before the comments above","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":434,"context_line":"            # display them: source/dest, protocol, sport, dport, target"},{"line_number":435,"context_line":"            # Otherwise the iptables_manager code won\u0027t be able to find"},{"line_number":436,"context_line":"            # them to preserve their [packet:byte] counts."},{"line_number":437,"context_line":"            if self.enable_ipset:"},{"line_number":438,"context_line":"                remote_gid \u003d rule.get(\u0027remote_group_id\u0027)"},{"line_number":439,"context_line":"                if remote_gid:"},{"line_number":440,"context_line":"                    iptables_rules.extend("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_c2d18758","line":437,"in_reply_to":"fa98f980_cc05407b","updated":"2014-09-11 11:13:23.000000000","message":"agree","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":434,"context_line":"            # display them: source/dest, protocol, sport, dport, target"},{"line_number":435,"context_line":"            # Otherwise the iptables_manager code won\u0027t be able to find"},{"line_number":436,"context_line":"            # them to preserve their [packet:byte] counts."},{"line_number":437,"context_line":"            if self.enable_ipset:"},{"line_number":438,"context_line":"                remote_gid \u003d rule.get(\u0027remote_group_id\u0027)"},{"line_number":439,"context_line":"                if remote_gid:"},{"line_number":440,"context_line":"                    iptables_rules.extend("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_9d69a48e","line":437,"in_reply_to":"fa98f980_cc05407b","updated":"2014-09-11 11:10:10.000000000","message":"correct","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"f13ec29975a20ec980e6866ba87e3f3d4652fa62","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_fd9b9015","line":69,"updated":"2014-09-11 11:32:22.000000000","message":"-1: didn\u0027t you agree to move it to ipset manager?","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"1a2ed8179d610ccc22989baa5784b9d4082edd95","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_53cbab52","line":69,"in_reply_to":"fa98f980_fd9b9015","updated":"2014-09-11 12:25:23.000000000","message":"I couldn\u0027t agree with you more, but if move it to ipset manager in this patch, there are many unit tests to be modified, what do you think do it later with the refactor into a new driver during K?","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"4d9995e3a86b6a27a425539dbdb6edfd939ec228","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_b343f7ad","line":69,"in_reply_to":"fa98f980_fd9b9015","updated":"2014-09-11 12:28:29.000000000","message":"Still undone, looking into refactoring it now. We want to check how to do it, shihanzhang realized that it would affect a lot of the UT, so may be it\u0027s worth postponing to K, when we refactor this into two drivers. (see line 55 above)\n\nAnyway we\u0027re looking into it right now, if it\u0027s reasonable within the deadline we can do it for J.","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e9e1ec8e17c958ee176bb51337cd816a7e64fba4","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        # List of security group member ips for ports residing on this host"},{"line_number":67,"context_line":"        self.sg_members \u003d {}"},{"line_number":68,"context_line":"        self.pre_sg_members \u003d None"},{"line_number":69,"context_line":"        self.ipset_chains \u003d {}"},{"line_number":70,"context_line":"        self.enable_ipset \u003d cfg.CONF.SECURITYGROUP.enable_ipset"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_c4142ceb","line":69,"updated":"2014-09-11 15:22:21.000000000","message":"For the discussion about moving this to ipset_manager, check : https://review.openstack.org/#/c/120806/ it\u0027s still WIP and untested.","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"e32abc1ff3864763a1664fadab47b245c2b3d57e","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                        port_rules.append(ip_rule)"},{"line_number":304,"context_line":"        return port_rules"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def _get_remote_sg_ids(self, port, direction):"},{"line_number":307,"context_line":"        sg_ids \u003d port.get(\u0027security_groups\u0027, [])"},{"line_number":308,"context_line":"        remote_sg_ids \u003d []"},{"line_number":309,"context_line":"        for sg_id in sg_ids:"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa98f980_3406f1ce","line":306,"updated":"2014-09-12 17:06:30.000000000","message":"The method name \"_get_remote_sg_id_for_port\" looks bette.r","commit_id":"2562a9271c828e982a74593e8fd07be13b0cfc4a"}],"neutron/agent/securitygroups_rpc.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"0ac494e9b24e43976846160b5066f38cf306dd4a","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        default\u003dFalse,"},{"line_number":40,"context_line":"        help\u003d_(\u0027Use the new rpc call to enhance security group\u0027)),"},{"line_number":41,"context_line":"    cfg.StrOpt("},{"line_number":42,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":43,"context_line":"        default\u003dFalse,"},{"line_number":44,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":45,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_a964e9b3","line":42,"updated":"2014-08-06 05:52:01.000000000","message":"Is there a reason why someone wouldn\u0027t want to use enable_ipset? If not I think we should only provide one options this way we don\u0027t need to maintain two.","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"0ee1cefe84480a88fe5c56d1f432d83dd861fce2","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        default\u003dFalse,"},{"line_number":40,"context_line":"        help\u003d_(\u0027Use the new rpc call to enhance security group\u0027)),"},{"line_number":41,"context_line":"    cfg.StrOpt("},{"line_number":42,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":43,"context_line":"        default\u003dFalse,"},{"line_number":44,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":45,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_6fe2ce31","line":42,"in_reply_to":"baada198_a964e9b3","updated":"2014-08-06 08:21:11.000000000","message":"Considerd some deployment do not provide ipset, so I make enable_ipset is optional","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":48,"context_line":"               \u0027for high density clouds.\u0027)),"},{"line_number":49,"context_line":"    cfg.BoolOpt("},{"line_number":50,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":51,"context_line":"        default\u003dFalse,"},{"line_number":52,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":53,"context_line":"]"},{"line_number":54,"context_line":"cfg.CONF.register_opts(security_group_opts, \u0027SECURITYGROUP\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_203e3db9","line":51,"updated":"2014-09-01 08:12:22.000000000","message":"True by default,\nso we get this tested by a wider audience, and also by tempest.","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":48,"context_line":"               \u0027for high density clouds.\u0027)),"},{"line_number":49,"context_line":"    cfg.BoolOpt("},{"line_number":50,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":51,"context_line":"        default\u003dFalse,"},{"line_number":52,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":53,"context_line":"]"},{"line_number":54,"context_line":"cfg.CONF.register_opts(security_group_opts, \u0027SECURITYGROUP\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_1b51ae62","line":51,"in_reply_to":"fa98f980_203e3db9","updated":"2014-09-01 09:05:33.000000000","message":"I got it","commit_id":"61e8a9ec28818acc1869953d94b6a47a9b6f139a"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"4648b16aebb8cd4157aa2be160b0272005c74627","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    cfg.BoolOpt("},{"line_number":42,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":43,"context_line":"        default\u003dTrue,"},{"line_number":44,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":"cfg.CONF.register_opts(security_group_opts, \u0027SECURITYGROUP\u0027)"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_fb8a91bd","line":44,"updated":"2014-09-04 05:51:13.000000000","message":"nit: I think we should improve this help message to why it enhances or maybe just say: (\"Use Ipset\")","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"09bd0d050e42b402512e36476ad7c46a037ccba9","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    cfg.BoolOpt("},{"line_number":42,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":43,"context_line":"        default\u003dTrue,"},{"line_number":44,"context_line":"        help\u003d_(\u0027Use Ipset to enhance security group\u0027))"},{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":"cfg.CONF.register_opts(security_group_opts, \u0027SECURITYGROUP\u0027)"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_e101fe6a","line":44,"in_reply_to":"fa98f980_fb8a91bd","updated":"2014-09-04 07:13:33.000000000","message":"Done","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9790c1ba8bf1aa1880219fd141f4abcd964869c6","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    cfg.BoolOpt("},{"line_number":42,"context_line":"        \u0027enable_ipset\u0027,"},{"line_number":43,"context_line":"        default\u003dTrue,"},{"line_number":44,"context_line":"        help\u003d_(\u0027Use ipset to speedup the iptables based security groups.\u0027))"},{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":"cfg.CONF.register_opts(security_group_opts, \u0027SECURITYGROUP\u0027)"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_6488d271","line":44,"updated":"2014-09-11 22:00:58.000000000","message":"nit: s/speedup/speed-up/\n      s/iptables based/iptables-based/","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"}],"neutron/db/securitygroups_rpc_base.py":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"57eb7cd6bf9d35546b41e9b9d7e7412204cf33cf","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        ports[\u0027sg_member_ips\u0027] \u003d remote_security_group_info"},{"line_number":196,"context_line":"        # the provider rules do not belong to any security group, so these"},{"line_number":197,"context_line":"        # rules still reside in ports[\u0027devices\u0027] [port_id]"},{"line_number":198,"context_line":"        self._apply_provider_rule(context, ports[\u0027devices\u0027])"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"        return self._get_security_group_member_ips(context, ports)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_aad84e54","side":"PARENT","line":197,"updated":"2014-09-01 08:12:22.000000000","message":"No need for the comment change ^","commit_id":"c03e7ae8363a0ef6935671bd1291619661097857"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"9dd6d2890e5ecf844b29a300507dcfaeb36f643b","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        ports[\u0027sg_member_ips\u0027] \u003d remote_security_group_info"},{"line_number":196,"context_line":"        # the provider rules do not belong to any security group, so these"},{"line_number":197,"context_line":"        # rules still reside in ports[\u0027devices\u0027] [port_id]"},{"line_number":198,"context_line":"        self._apply_provider_rule(context, ports[\u0027devices\u0027])"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"        return self._get_security_group_member_ips(context, ports)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa98f980_db172650","side":"PARENT","line":197,"in_reply_to":"fa98f980_aad84e54","updated":"2014-09-01 09:05:33.000000000","message":"Done","commit_id":"c03e7ae8363a0ef6935671bd1291619661097857"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"4648b16aebb8cd4157aa2be160b0272005c74627","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # if there are allowed_address_pairs then add them"},{"line_number":234,"context_line":"            if getattr(port, \u0027allowed_address_pairs\u0027, None):"},{"line_number":235,"context_line":"                for address_pair in port.allowed_address_pairs:"},{"line_number":236,"context_line":"                    ips.append(address_pair[\u0027ip_address\u0027])"},{"line_number":237,"context_line":"        return ips"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def _select_rules_for_ports(self, context, ports):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_7bbea166","line":236,"updated":"2014-09-04 05:51:13.000000000","message":"allowed_address_pairs are mac and ip bindings. This looks like it\u0027s ignoring the mac value?","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"1b6788182d767a141b22e687d67b832368a123d7","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # if there are allowed_address_pairs then add them"},{"line_number":234,"context_line":"            if getattr(port, \u0027allowed_address_pairs\u0027, None):"},{"line_number":235,"context_line":"                for address_pair in port.allowed_address_pairs:"},{"line_number":236,"context_line":"                    ips.append(address_pair[\u0027ip_address\u0027])"},{"line_number":237,"context_line":"        return ips"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def _select_rules_for_ports(self, context, ports):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_ef20abb2","line":236,"in_reply_to":"fa98f980_41472abc","updated":"2014-09-08 07:55:20.000000000","message":"I believe in the iptables SG implementation only the ip addresses are used with iptables (no mac match), ast the IPs on the SG could be on a different L2 segment.","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"09bd0d050e42b402512e36476ad7c46a037ccba9","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            # if there are allowed_address_pairs then add them"},{"line_number":234,"context_line":"            if getattr(port, \u0027allowed_address_pairs\u0027, None):"},{"line_number":235,"context_line":"                for address_pair in port.allowed_address_pairs:"},{"line_number":236,"context_line":"                    ips.append(address_pair[\u0027ip_address\u0027])"},{"line_number":237,"context_line":"        return ips"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def _select_rules_for_ports(self, context, ports):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa98f980_41472abc","line":236,"in_reply_to":"fa98f980_7bbea166","updated":"2014-09-04 07:13:33.000000000","message":"I think it does not conccern mac value","commit_id":"386812f56b88ebc3307b642c68870c8e637d92e2"}],"neutron/tests/unit/test_iptables_firewall.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"    def setUp(self):"},{"line_number":1230,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1231,"context_line":"        self.ipset_cls_p \u003d mock.patch("},{"line_number":1232,"context_line":"            \u0027neutron.agent.linux.ipset_manager.IpsetManager\u0027)"},{"line_number":1233,"context_line":"        self.firewall.ipset \u003d mock.Mock()"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"    def _fake_port(self):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_9ff39f09","line":1232,"updated":"2014-09-09 12:59:48.000000000","message":"this patch is never started. Is that ok?","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"a658debfad333894cf9083c43009f53fbba695aa","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"    def setUp(self):"},{"line_number":1230,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1231,"context_line":"        self.ipset_cls_p \u003d mock.patch("},{"line_number":1232,"context_line":"            \u0027neutron.agent.linux.ipset_manager.IpsetManager\u0027)"},{"line_number":1233,"context_line":"        self.firewall.ipset \u003d mock.Mock()"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"    def _fake_port(self):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_4aa5f045","line":1232,"in_reply_to":"fa98f980_279385fb","updated":"2014-09-09 14:51:04.000000000","message":"it is missing:\n\nself.ipset_cls_p.start()\nself.addCleanup(self.ipset_cls_p.stop)\n\notherwise, it has no effect.\n\nhttps://docs.python.org/3/library/unittest.mock.html#patch-methods-start-and-stop","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"    def setUp(self):"},{"line_number":1230,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1231,"context_line":"        self.ipset_cls_p \u003d mock.patch("},{"line_number":1232,"context_line":"            \u0027neutron.agent.linux.ipset_manager.IpsetManager\u0027)"},{"line_number":1233,"context_line":"        self.firewall.ipset \u003d mock.Mock()"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"    def _fake_port(self):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_279385fb","line":1232,"in_reply_to":"fa98f980_9ff39f09","updated":"2014-09-09 14:24:18.000000000","message":"what\u0027s your meaning?","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"cb7d150b5426eab75f663c1f28c6924a7ba8158f","unresolved":false,"context_lines":[{"line_number":1265,"context_line":"    def test_prepare_port_filter_with_add_members_beyond_4(self):"},{"line_number":1266,"context_line":"        self.firewall.sg_rules \u003d self._fake_sg_rule()"},{"line_number":1267,"context_line":"        self.firewall.sg_members \u003d {\u0027fake_sgid\u0027: {"},{"line_number":1268,"context_line":"            \u0027IPv4\u0027: ["},{"line_number":1269,"context_line":"                \u002710.0.0.1\u0027, \u002710.0.0.2,\u0027, \u002710.0.0.3\u0027, \u002710.0.0.4\u0027, \u002710.0.0.5\u0027],"},{"line_number":1270,"context_line":"            \u0027IPv6\u0027: [\u0027fe80::1\u0027]}}"},{"line_number":1271,"context_line":"        self.firewall.pre_sg_members \u003d {}"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_40277664","line":1268,"updated":"2014-09-10 10:30:31.000000000","message":"All the code for \u002710.0.0.1\u0027, ..., \u002710.0.0.5\u0027 is very repetitive. I would strongly look into writing a range() function for ip addresses, or otherwise put this list in a global variable someplace and reuse it.","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac591dbc129c376cc00e9bff1f57e207681075c","unresolved":false,"context_lines":[{"line_number":1265,"context_line":"    def test_prepare_port_filter_with_add_members_beyond_4(self):"},{"line_number":1266,"context_line":"        self.firewall.sg_rules \u003d self._fake_sg_rule()"},{"line_number":1267,"context_line":"        self.firewall.sg_members \u003d {\u0027fake_sgid\u0027: {"},{"line_number":1268,"context_line":"            \u0027IPv4\u0027: ["},{"line_number":1269,"context_line":"                \u002710.0.0.1\u0027, \u002710.0.0.2,\u0027, \u002710.0.0.3\u0027, \u002710.0.0.4\u0027, \u002710.0.0.5\u0027],"},{"line_number":1270,"context_line":"            \u0027IPv6\u0027: [\u0027fe80::1\u0027]}}"},{"line_number":1271,"context_line":"        self.firewall.pre_sg_members \u003d {}"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_a3b90917","line":1268,"in_reply_to":"fa98f980_40277664","updated":"2014-09-10 12:54:55.000000000","message":"I agree.","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"8184b7b5331bdf0c285004ca03665c2f29f2d4a8","unresolved":false,"context_lines":[{"line_number":1265,"context_line":"    def test_prepare_port_filter_with_add_members_beyond_4(self):"},{"line_number":1266,"context_line":"        self.firewall.sg_rules \u003d self._fake_sg_rule()"},{"line_number":1267,"context_line":"        self.firewall.sg_members \u003d {\u0027fake_sgid\u0027: {"},{"line_number":1268,"context_line":"            \u0027IPv4\u0027: ["},{"line_number":1269,"context_line":"                \u002710.0.0.1\u0027, \u002710.0.0.2,\u0027, \u002710.0.0.3\u0027, \u002710.0.0.4\u0027, \u002710.0.0.5\u0027],"},{"line_number":1270,"context_line":"            \u0027IPv6\u0027: [\u0027fe80::1\u0027]}}"},{"line_number":1271,"context_line":"        self.firewall.pre_sg_members \u003d {}"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_81fddd3d","line":1268,"in_reply_to":"fa98f980_66aacfd3","updated":"2014-09-10 14:31:38.000000000","message":"Done","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"077bf5efdbd262d7cfc8199c4a788a8cd66d66bc","unresolved":false,"context_lines":[{"line_number":1265,"context_line":"    def test_prepare_port_filter_with_add_members_beyond_4(self):"},{"line_number":1266,"context_line":"        self.firewall.sg_rules \u003d self._fake_sg_rule()"},{"line_number":1267,"context_line":"        self.firewall.sg_members \u003d {\u0027fake_sgid\u0027: {"},{"line_number":1268,"context_line":"            \u0027IPv4\u0027: ["},{"line_number":1269,"context_line":"                \u002710.0.0.1\u0027, \u002710.0.0.2,\u0027, \u002710.0.0.3\u0027, \u002710.0.0.4\u0027, \u002710.0.0.5\u0027],"},{"line_number":1270,"context_line":"            \u0027IPv6\u0027: [\u0027fe80::1\u0027]}}"},{"line_number":1271,"context_line":"        self.firewall.pre_sg_members \u003d {}"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa98f980_66aacfd3","line":1268,"in_reply_to":"fa98f980_a3b90917","updated":"2014-09-10 13:26:40.000000000","message":"agree","commit_id":"dd9b1a60a44894acf7fa3f44be235e72ddc893ce"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_9f451c45","line":1232,"updated":"2014-09-11 09:47:10.000000000","message":"this will run all tests from IptablesFirewallTestCase again. Consider refactor to have common base class","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6e7b0fd122348051c65bf6bec56efe1def5b17d6","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_1d8134c3","line":1232,"in_reply_to":"fa98f980_02970f3b","updated":"2014-09-11 11:17:04.000000000","message":"this should be really trivial..","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_3db7d8f6","line":1232,"in_reply_to":"fa98f980_9f451c45","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_02970f3b","line":1232,"in_reply_to":"fa98f980_9f451c45","updated":"2014-09-11 11:13:23.000000000","message":"can I refactor this after this patch commit?","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("},{"line_number":1236,"context_line":"            \u0027neutron.agent.linux.ipset_manager.IpsetManager\u0027)"},{"line_number":1237,"context_line":"        self.ipset_cls_p.start()"},{"line_number":1238,"context_line":"        self.addCleanup(self.ipset_cls_p.stop)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_3fc5108e","line":1235,"updated":"2014-09-11 09:47:10.000000000","message":"seems mocking not needed as you explicitly mock it at #1239. Additionally firewall driver was already instantiated in super class init so it is too late to mock here also :)","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.ipset_cls_p \u003d mock.patch("},{"line_number":1236,"context_line":"            \u0027neutron.agent.linux.ipset_manager.IpsetManager\u0027)"},{"line_number":1237,"context_line":"        self.ipset_cls_p.start()"},{"line_number":1238,"context_line":"        self.addCleanup(self.ipset_cls_p.stop)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_5dcf3c7f","line":1235,"in_reply_to":"fa98f980_3fc5108e","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"f13ec29975a20ec980e6866ba87e3f3d4652fa62","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.firewall.ipset \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_b85cb61a","line":1232,"updated":"2014-09-11 11:32:22.000000000","message":"Consider:\n\nclass IptablesFirewallTestCaseBase(base.BaseTestCase):\n  def setUp():\n  ...\n\nclass IptablesFirewallTestCase(class IptablesFirewallTestCaseBase):\n  def test_...\n  ...\n\nIptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCaseBase):\n  def test_...","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"4d9995e3a86b6a27a425539dbdb6edfd939ec228","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.firewall.ipset \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_13e5a3db","line":1232,"in_reply_to":"fa98f980_b85cb61a","updated":"2014-09-11 12:28:29.000000000","message":"Done","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"1a2ed8179d610ccc22989baa5784b9d4082edd95","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"        self.v4filter_inst.assert_has_calls(calls)"},{"line_number":1230,"context_line":""},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"class IptablesFirewallEnhancedIpsetTestCase(IptablesFirewallTestCase):"},{"line_number":1233,"context_line":"    def setUp(self):"},{"line_number":1234,"context_line":"        super(IptablesFirewallEnhancedIpsetTestCase, self).setUp()"},{"line_number":1235,"context_line":"        self.firewall.ipset \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa98f980_d305db7a","line":1232,"in_reply_to":"fa98f980_b85cb61a","updated":"2014-09-11 12:25:23.000000000","message":"ajo is doing it","commit_id":"5706b892e1860436b7827f9d57b70396153cb968"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f11b28ddd1b1eb8e9c1ff3da9753ecaad74fb527","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.iptables_inst.ipv6 \u003d {\u0027filter\u0027: self.v6filter_inst}"},{"line_number":56,"context_line":"        iptables_cls.return_value \u003d self.iptables_inst"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        self.firewall \u003d iptables_firewall.IptablesFirewallDriver()"},{"line_number":59,"context_line":"        self.firewall.iptables \u003d self.iptables_inst"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_b0468abe","line":58,"updated":"2014-09-11 13:30:01.000000000","message":"move this down to the IptablesFirewallTestCase, the Ipset mocks it.","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"a7818d4071a0fe2ab4c78db02eb14d2c2a4a887f","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.iptables_inst.ipv6 \u003d {\u0027filter\u0027: self.v6filter_inst}"},{"line_number":56,"context_line":"        iptables_cls.return_value \u003d self.iptables_inst"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        self.firewall \u003d iptables_firewall.IptablesFirewallDriver()"},{"line_number":59,"context_line":"        self.firewall.iptables \u003d self.iptables_inst"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa98f980_7055024f","line":58,"in_reply_to":"fa98f980_b0468abe","updated":"2014-09-11 13:31:08.000000000","message":"ah, no, wrong, it\u0027s correct here :-)","commit_id":"65fe9cd4bd3940e8274f368dc5d434a0d771786a"}],"neutron/tests/unit/test_security_groups_rpc.py":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"c0d9feb5f9bd470971221415bcd301490c437474","unresolved":false,"context_lines":[{"line_number":1889,"context_line":"            group\u003d\u0027SECURITYGROUP\u0027)"},{"line_number":1890,"context_line":""},{"line_number":1891,"context_line":"        set_enhanced_rpc_enable(False)"},{"line_number":1892,"context_line":"        set_ipset_enable(False)"},{"line_number":1893,"context_line":"        self.agent \u003d sg_rpc.SecurityGroupAgentRpcMixin()"},{"line_number":1894,"context_line":"        self.agent.context \u003d None"},{"line_number":1895,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_83bf04ab","line":1892,"updated":"2014-08-06 04:07:34.000000000","message":"So how is this being tested if this is always set to False?","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"71a6493f404937216bb83238eb1f136867dc554c","unresolved":false,"context_lines":[{"line_number":1889,"context_line":"            group\u003d\u0027SECURITYGROUP\u0027)"},{"line_number":1890,"context_line":""},{"line_number":1891,"context_line":"        set_enhanced_rpc_enable(False)"},{"line_number":1892,"context_line":"        set_ipset_enable(False)"},{"line_number":1893,"context_line":"        self.agent \u003d sg_rpc.SecurityGroupAgentRpcMixin()"},{"line_number":1894,"context_line":"        self.agent.context \u003d None"},{"line_number":1895,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_0e6a3bb5","line":1892,"in_reply_to":"baada198_83bf04ab","updated":"2014-08-06 04:49:21.000000000","message":"Now ipset does not install in jenkins, so set false here, when I modify that, I will remove it","commit_id":"75d932aeee3a7c60a641d681857a32b8e641ad63"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e97b7e2a4ff4203dccfabe9d0e7a7da0c78fa0f7","unresolved":false,"context_lines":[{"line_number":2533,"context_line":""},{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"class TestSecurityGroupAgentEnhancedIpsetWithIptables("},{"line_number":2536,"context_line":"    TestSecurityGroupAgentEnhancedRpcWithIptables):"},{"line_number":2537,"context_line":"    def setUp(self, defer_refresh_firewall\u003dFalse):"},{"line_number":2538,"context_line":"        super(TestSecurityGroupAgentEnhancedIpsetWithIptables, self).setUp("},{"line_number":2539,"context_line":"            defer_refresh_firewall)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_ffa90bde","line":2536,"updated":"2014-09-09 12:59:48.000000000","message":"could you indent this one time? So it is distinguishable from the method below.","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"5386161eefdcb6196926af224d8ec3e3be15f1ed","unresolved":false,"context_lines":[{"line_number":2533,"context_line":""},{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"class TestSecurityGroupAgentEnhancedIpsetWithIptables("},{"line_number":2536,"context_line":"    TestSecurityGroupAgentEnhancedRpcWithIptables):"},{"line_number":2537,"context_line":"    def setUp(self, defer_refresh_firewall\u003dFalse):"},{"line_number":2538,"context_line":"        super(TestSecurityGroupAgentEnhancedIpsetWithIptables, self).setUp("},{"line_number":2539,"context_line":"            defer_refresh_firewall)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa98f980_8766f127","line":2536,"in_reply_to":"fa98f980_ffa90bde","updated":"2014-09-09 14:24:18.000000000","message":"agree","commit_id":"6bac83182fb8b98f0cf0f7e1938c79a72eb0db11"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":2031,"context_line":""},{"line_number":2032,"context_line":"        self.agent.init_firewall("},{"line_number":2033,"context_line":"            defer_refresh_firewall\u003ddefer_refresh_firewall)"},{"line_number":2034,"context_line":""},{"line_number":2035,"context_line":"        self.iptables \u003d self.agent.firewall.iptables"},{"line_number":2036,"context_line":"        # TODO(jlibosva) Get rid of mocking iptables execute and mock out"},{"line_number":2037,"context_line":"        # firewall instead"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_dfe324d7","side":"PARENT","line":2034,"updated":"2014-09-11 09:47:10.000000000","message":"please avoid unrelated changes","commit_id":"f2c7ee79420fd60b6323b086e316e5747b9e9757"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ebebd1966833aa99dba968b3c98f15c40ea242f8","unresolved":false,"context_lines":[{"line_number":2031,"context_line":""},{"line_number":2032,"context_line":"        self.agent.init_firewall("},{"line_number":2033,"context_line":"            defer_refresh_firewall\u003ddefer_refresh_firewall)"},{"line_number":2034,"context_line":""},{"line_number":2035,"context_line":"        self.iptables \u003d self.agent.firewall.iptables"},{"line_number":2036,"context_line":"        # TODO(jlibosva) Get rid of mocking iptables execute and mock out"},{"line_number":2037,"context_line":"        # firewall instead"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_fda710bb","side":"PARENT","line":2034,"in_reply_to":"fa98f980_dfe324d7","updated":"2014-09-11 11:10:10.000000000","message":"+1","commit_id":"f2c7ee79420fd60b6323b086e316e5747b9e9757"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e34ba6725d435f160a6e1a54be1d535bbf1bb02d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                     \u0027SecurityGroupRpcTestPlugin\u0027)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def set_ipset_enable(enable):"},{"line_number":55,"context_line":"    cfg.CONF.set_override(\u0027enable_ipset\u0027, enable,"},{"line_number":56,"context_line":"                          group\u003d\u0027SECURITYGROUP\u0027)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_1f868cdb","line":54,"updated":"2014-09-11 09:47:10.000000000","message":"why need func if used only once?","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"3b5eecb9db46afb59933d0e56a9d931734e21179","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                     \u0027SecurityGroupRpcTestPlugin\u0027)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def set_ipset_enable(enable):"},{"line_number":55,"context_line":"    cfg.CONF.set_override(\u0027enable_ipset\u0027, enable,"},{"line_number":56,"context_line":"                          group\u003d\u0027SECURITYGROUP\u0027)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_a2be43ac","line":54,"in_reply_to":"fa98f980_1f868cdb","updated":"2014-09-11 11:13:23.000000000","message":"agree to remove this func","commit_id":"1c621eb27209175de7ee04b95d33babe6d05eefc"}]}