)]}'
{"neutron/tests/common/net_helpers.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"0bd02abb58665233f20a0abe2e687ac6e29332d0","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        try:"},{"line_number":258,"context_line":"            self.port \u003d self.unallocated_ports.pop()"},{"line_number":259,"context_line":"        except KeyError:"},{"line_number":260,"context_line":"            raise ValueError(_(\u0027VethBridgeFixture supports at most 2 ports\u0027))"},{"line_number":261,"context_line":"        self.ns_ip_wrapper.add_device_to_namespace(self.port)"},{"line_number":262,"context_line":"        self.port.link.set_up()"},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a6ced46_ad7945fe","line":260,"updated":"2015-03-23 14:24:37.000000000","message":"nit: \"IOError\" might fit the message better, but this is strictly a nit and a matter of taste, so I won\u0027t hold this against you :)","commit_id":"ad605c738ff158dd71afaacf5ce6dfd8bd58e3af"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        \"\"\"Returns a NamespacePortFixture instance.\"\"\""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    @classmethod"},{"line_number":127,"context_line":"    def get(cls, bridge):"},{"line_number":128,"context_line":"        if isinstance(bridge, cls):"},{"line_number":129,"context_line":"            return bridge"},{"line_number":130,"context_line":"        return OVSBridgeFixture(bridge)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_0982b894","line":127,"updated":"2015-03-27 19:52:27.000000000","message":"This method is used only used by create_ovs_port_in_ns to work around a test that creates a bridge directly rather than using a fixture.  I don\u0027t think this kind of workaround is desirable, and I would rather see the TestSimpleInterfaceMonitor.test_has_updates use a fixture directly.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        \"\"\"Returns a NamespacePortFixture instance.\"\"\""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    @classmethod"},{"line_number":127,"context_line":"    def get(cls, bridge):"},{"line_number":128,"context_line":"        if isinstance(bridge, cls):"},{"line_number":129,"context_line":"            return bridge"},{"line_number":130,"context_line":"        return OVSBridgeFixture(bridge)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_61cbd673","line":127,"in_reply_to":"fa963961_0982b894","updated":"2015-03-27 22:43:19.000000000","message":"removed","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":153,"context_line":"    def setUp(self):"},{"line_number":154,"context_line":"        super(OVSBridgeFixture, self).setUp()"},{"line_number":155,"context_line":"        ovs \u003d ovs_lib.BaseOVS()"},{"line_number":156,"context_line":"        self.bridge \u003d base.create_resource(BR_PREFIX, ovs.add_bridge)"},{"line_number":157,"context_line":"        self.addCleanup(self.bridge.destroy)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_29c8dc54","line":156,"updated":"2015-03-27 19:52:27.000000000","message":"If we wanted to allow overriding of the bridge via init, this creation should be conditional, e.g.\n\nif not self.bridge:\n  self.bridge \u003d ...\n\nWhat is the use case for needing for the caller to supply a bridge at all, though?  Shouldn\u0027t the goal of a bridge fixture be to manage the lifecycle of a bridge?","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":153,"context_line":"    def setUp(self):"},{"line_number":154,"context_line":"        super(OVSBridgeFixture, self).setUp()"},{"line_number":155,"context_line":"        ovs \u003d ovs_lib.BaseOVS()"},{"line_number":156,"context_line":"        self.bridge \u003d base.create_resource(BR_PREFIX, ovs.add_bridge)"},{"line_number":157,"context_line":"        self.addCleanup(self.bridge.destroy)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_01a1aaa9","line":156,"in_reply_to":"fa963961_29c8dc54","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        self.bridge \u003d base.create_resource(BR_PREFIX, ovs.add_bridge)"},{"line_number":157,"context_line":"        self.addCleanup(self.bridge.destroy)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":160,"context_line":"        return OVSNamespacePortFixture(self.bridge, ns_ip_wrapper)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4c97ae90","line":159,"updated":"2015-03-27 19:52:27.000000000","message":"I don\u0027t think this fixture should be responsible for creating a port fixture.  Instead, I\u0027d like to see the port fixture take an optional bridge argument, e.g.\n\nclass PortFixture(...):\n  def __init__(self, bridge\u003dNone):\n    self.bridge \u003d bridge\n\n  def setUp(self):\n    if not self.bridge:\n       self.bridge \u003d self.useFixture(BridgeFixture(..)).bridge\n\n bridge \u003d self.useFixture(BridgeFixture()).bridge\n port \u003d self.useFixture(PortFixture(bridge)).port\n\nor\n\n port \u003d self.userFixture(PortFixture)).port\n\nI think the goal here should be that bridge creation is always via bridge fixture, but that otherwise the bridge is used directly, and that we minimize unnecessary coupling.\n\nAlso, why is it necessary to pass a ns_ip_wrapper?  I\u0027d rather see it accept a namespace argument.  IpWrapper\u0027s are cheap, after all, there is no real reason to pass them around.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        self.bridge \u003d base.create_resource(BR_PREFIX, ovs.add_bridge)"},{"line_number":157,"context_line":"        self.addCleanup(self.bridge.destroy)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":160,"context_line":"        return OVSNamespacePortFixture(self.bridge, ns_ip_wrapper)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_e1a5a6b5","line":159,"in_reply_to":"fa963961_4c97ae90","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        super(OVSNamespacePortFixture, self).setUp()"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        port_name \u003d base.create_resource(PORT_PREFIX, self.create_port)"},{"line_number":173,"context_line":"        self.addCleanup(self.bridge.delete_port, port_name)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper()"},{"line_number":176,"context_line":"        self.port \u003d ip_wrapper.device(port_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4cde8e6e","line":173,"updated":"2015-03-27 19:52:27.000000000","message":"Is port deletion required when bridge removal will automatically result in port removal?","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        super(OVSNamespacePortFixture, self).setUp()"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        port_name \u003d base.create_resource(PORT_PREFIX, self.create_port)"},{"line_number":173,"context_line":"        self.addCleanup(self.bridge.delete_port, port_name)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper()"},{"line_number":176,"context_line":"        self.port \u003d ip_wrapper.device(port_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_a7309357","line":173,"in_reply_to":"fa963961_4cde8e6e","updated":"2015-03-27 20:36:20.000000000","message":"In general, should we assume a bridge has the ability to clean its ports? I don\u0027t think so.\n\nMoreover this comment seems in contradiction with comments on create_port_in_ns_fixture: if bridge fixture is not responsible for creating ports then why it should be responsible for deleting ports?","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":192,"context_line":"    \"\"\""},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def __init__(self):"},{"line_number":195,"context_line":"        super(LinuxBridgeFixture, self).__init__()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def setUp(self):"},{"line_number":198,"context_line":"        super(LinuxBridgeFixture, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_ec17a2eb","line":195,"updated":"2015-03-27 19:52:27.000000000","message":"A method with just a call to super() is unnecessary.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":192,"context_line":"    \"\"\""},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def __init__(self):"},{"line_number":195,"context_line":"        super(LinuxBridgeFixture, self).__init__()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def setUp(self):"},{"line_number":198,"context_line":"        super(LinuxBridgeFixture, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_c78f1722","line":195,"in_reply_to":"fa963961_ec17a2eb","updated":"2015-03-27 20:36:20.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def setUp(self):"},{"line_number":198,"context_line":"        super(LinuxBridgeFixture, self).setUp()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"        self.ip_wrapper \u003d self.useFixture(NamespaceFixture()).ip_wrapper"},{"line_number":201,"context_line":"        self.bridge \u003d base.create_resource("},{"line_number":202,"context_line":"            BR_PREFIX,"},{"line_number":203,"context_line":"            bridge_lib.BridgeDevice.addbr,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_2c522a68","line":200,"updated":"2015-03-27 19:52:27.000000000","message":"I don\u0027t think it\u0027s necessary or desirable to save the ip_wrapper instance here.  It also doesn\u0027t make sense to have the corresponding fixture depend on the bridge fixture instead of the bridge.  The return value of addbr is an instance of BridgeDevice and has a namespace property that can be used to create a new IpWrapper as required.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def setUp(self):"},{"line_number":198,"context_line":"        super(LinuxBridgeFixture, self).setUp()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"        self.ip_wrapper \u003d self.useFixture(NamespaceFixture()).ip_wrapper"},{"line_number":201,"context_line":"        self.bridge \u003d base.create_resource("},{"line_number":202,"context_line":"            BR_PREFIX,"},{"line_number":203,"context_line":"            bridge_lib.BridgeDevice.addbr,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_e7469bbe","line":200,"in_reply_to":"fa963961_2c522a68","updated":"2015-03-27 20:36:20.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        self.bridge.link.set_up()"},{"line_number":207,"context_line":"        self.addCleanup(self.bridge.link.set_down)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":210,"context_line":"        return LinuxBridgeNamespacePortFixture(self, ns_ip_wrapper)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4ce8cee2","line":209,"updated":"2015-03-27 19:52:27.000000000","message":"As per a previous comment, I don\u0027t think the bridge fixture should be responsible for creating port fixtures.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        self.bridge.link.set_up()"},{"line_number":207,"context_line":"        self.addCleanup(self.bridge.link.set_down)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":210,"context_line":"        return LinuxBridgeNamespacePortFixture(self, ns_ip_wrapper)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_c1aaa285","line":209,"in_reply_to":"fa963961_4ce8cee2","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    \"\"\""},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def __init__(self, bridge_fixture, ns_ip_wrapper):"},{"line_number":223,"context_line":"        self.bridge_fixture \u003d bridge_fixture"},{"line_number":224,"context_line":"        self.ns_ip_wrapper \u003d ns_ip_wrapper"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4cdc4ee4","line":223,"updated":"2015-03-27 19:52:27.000000000","message":"As per a previous comment, this fixture should depend on the bridge rather than its fixture.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    \"\"\""},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def __init__(self, bridge_fixture, ns_ip_wrapper):"},{"line_number":223,"context_line":"        self.bridge_fixture \u003d bridge_fixture"},{"line_number":224,"context_line":"        self.ns_ip_wrapper \u003d ns_ip_wrapper"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_6192b659","line":223,"in_reply_to":"fa963961_4cdc4ee4","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        super(LinuxBridgeNamespacePortFixture, self).setUp()"},{"line_number":228,"context_line":"        self.port, self.br_port \u003d self.useFixture(VethFixture()).ports"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        self.bridge_fixture.ip_wrapper.add_device_to_namespace(self.br_port)"},{"line_number":231,"context_line":"        self.bridge_fixture.bridge.addif(self.br_port)"},{"line_number":232,"context_line":"        self.br_port.link.set_up()"},{"line_number":233,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_ece2029f","line":230,"updated":"2015-03-27 19:52:27.000000000","message":"An ip wrapper can be created with the bridge\u0027s namespace property instead of using the fixture.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        super(LinuxBridgeNamespacePortFixture, self).setUp()"},{"line_number":228,"context_line":"        self.port, self.br_port \u003d self.useFixture(VethFixture()).ports"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        self.bridge_fixture.ip_wrapper.add_device_to_namespace(self.br_port)"},{"line_number":231,"context_line":"        self.bridge_fixture.bridge.addif(self.br_port)"},{"line_number":232,"context_line":"        self.br_port.link.set_up()"},{"line_number":233,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_219cae68","line":230,"in_reply_to":"fa963961_ece2029f","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        self.veth_fixture \u003d self.useFixture(VethFixture())"},{"line_number":248,"context_line":"        self.unallocated_ports \u003d set(self.veth_fixture.ports)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":251,"context_line":"        return VethNamespacePortFixture(self.unallocated_ports, ns_ip_wrapper)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4c032e43","line":250,"updated":"2015-03-27 19:52:27.000000000","message":"As per a previous comment, I don\u0027t think the bridge fixture should be responsible for creating port fixtures.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"27d27e341ba8f9810e3be37f2882cd0678da3d20","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        self.veth_fixture \u003d self.useFixture(VethFixture())"},{"line_number":248,"context_line":"        self.unallocated_ports \u003d set(self.veth_fixture.ports)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def create_port_in_ns_fixture(self, ns_ip_wrapper):"},{"line_number":251,"context_line":"        return VethNamespacePortFixture(self.unallocated_ports, ns_ip_wrapper)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_01888a21","line":250,"in_reply_to":"fa963961_4c032e43","updated":"2015-03-27 22:43:19.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"badab78d4683b61b7795b393c689fe8cbfcf4d4b","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":121,"context_line":"class BridgeFixture(fixtures.Fixture):"},{"line_number":122,"context_line":"    \"\"\"Create a bridge."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    :ivar bridge: created bridge"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_7587b4d7","line":121,"updated":"2015-03-31 19:01:38.000000000","message":"This class looks like a no-op. Is it needed?","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"478ff4caa5913ddc987f7e394f7ca26a97dcca18","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":121,"context_line":"class BridgeFixture(fixtures.Fixture):"},{"line_number":122,"context_line":"    \"\"\"Create a bridge."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    :ivar bridge: created bridge"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_aae94f82","line":121,"in_reply_to":"da9b358b_7587b4d7","updated":"2015-03-31 19:34:04.000000000","message":"imo, we should define somewhere what we expect from a bridge fixture.\n\nIt could be described in module docstring or interface docstring (my proposal).\n\nWe could also use only duck typing but with the fake machine and peer machines fixtures (to come in follow-up change) but it seems difficult to understand.\n\nThe same remark applies to PortFixture.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"bda8c11d5989be9038504338cd9facd9f0010c3b","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":121,"context_line":"class BridgeFixture(fixtures.Fixture):"},{"line_number":122,"context_line":"    \"\"\"Create a bridge."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    :ivar bridge: created bridge"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_32fff535","line":121,"in_reply_to":"da9b358b_aae94f82","updated":"2015-03-31 23:03:44.000000000","message":"Why is it necessary to have this be an abstract class?  It\u0027s really strange to see a method with an implementation marked as abstract, and I don\u0027t think there is any value over simply having a bare implementation used as a marker / documenting base class.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"badab78d4683b61b7795b393c689fe8cbfcf4d4b","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":133,"context_line":"class PortFixture(fixtures.Fixture):"},{"line_number":134,"context_line":"    \"\"\"Create a port."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    :ivar port: created port"},{"line_number":137,"context_line":"    :type port: IPDevice"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_8448c488","line":134,"updated":"2015-03-31 19:01:38.000000000","message":"Nit: Might want to document how this fixture is used to get() a port fixture.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"478ff4caa5913ddc987f7e394f7ca26a97dcca18","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":133,"context_line":"class PortFixture(fixtures.Fixture):"},{"line_number":134,"context_line":"    \"\"\"Create a port."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    :ivar port: created port"},{"line_number":137,"context_line":"    :type port: IPDevice"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_8a868b11","line":134,"in_reply_to":"da9b358b_8448c488","updated":"2015-03-31 19:34:04.000000000","message":"Done","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"bda8c11d5989be9038504338cd9facd9f0010c3b","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    \"\"\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, bridge, namespace\u003dNone):"},{"line_number":141,"context_line":"        self.bridge \u003d bridge"},{"line_number":142,"context_line":"        self.namespace \u003d namespace"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_c3323910","line":141,"updated":"2015-03-31 23:03:44.000000000","message":"(No action required) Consider making the bridge parameter optional and having it initialized via fixture automatically if required in the concrete classes, e.g.\n\n class OvsPortFixture(...):\n  def setUp(self):\n    if not self.bridge:\n      self.bridge \u003d self.useFixture(OvsBridgeFixture(...)).bridge\n    \nThen usage could be simplified:\n\n ...\n port1_fixture \u003d self.useFixture(OvsPortFixture())\n port2_fixture \u003d self.useFixture(OvsPortFixture(port1_fixture.bridge))","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"de09c726b4bfed6bd6933de8caae0c09a7972311","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    \"\"\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, bridge, namespace\u003dNone):"},{"line_number":141,"context_line":"        self.bridge \u003d bridge"},{"line_number":142,"context_line":"        self.namespace \u003d namespace"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_f76f2829","line":141,"in_reply_to":"da9b358b_c3323910","updated":"2015-04-01 12:19:59.000000000","message":"Done","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"bda8c11d5989be9038504338cd9facd9f0010c3b","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        super(PortFixture, self).setUp()"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    @staticmethod"},{"line_number":149,"context_line":"    def get(bridge, namespace\u003dNone):"},{"line_number":150,"context_line":"        if isinstance(bridge, ovs_lib.OVSBridge):"},{"line_number":151,"context_line":"            return OVSPortFixture(bridge, namespace)"},{"line_number":152,"context_line":"        if isinstance(bridge, bridge_lib.BridgeDevice):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_d2c8c924","line":149,"updated":"2015-03-31 23:03:44.000000000","message":"YAGNI I don\u0027t see the point of treating port fixtures polymorphically at this point. Please remove and you can add it back in a patch that you think requires it.  That way reviewers can evaluate this function properly, on its merits.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"de09c726b4bfed6bd6933de8caae0c09a7972311","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        super(PortFixture, self).setUp()"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    @staticmethod"},{"line_number":149,"context_line":"    def get(bridge, namespace\u003dNone):"},{"line_number":150,"context_line":"        if isinstance(bridge, ovs_lib.OVSBridge):"},{"line_number":151,"context_line":"            return OVSPortFixture(bridge, namespace)"},{"line_number":152,"context_line":"        if isinstance(bridge, bridge_lib.BridgeDevice):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_37f11002","line":149,"in_reply_to":"da9b358b_d2c8c924","updated":"2015-04-01 12:19:59.000000000","message":"Done","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"bda8c11d5989be9038504338cd9facd9f0010c3b","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        tools.fail(\u0027Unexpected bridge type: %s\u0027 % type(bridge))"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"class OVSBridgeFixture(BridgeFixture):"},{"line_number":160,"context_line":"    \"\"\"Create an OVS bridge."},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    :ivar bridge: created bridge"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_a3673524","line":159,"updated":"2015-03-31 23:03:44.000000000","message":"(No action required) I think OvsBridgeFixture makes more sense, but I don\u0027t think we\u0027re consistent about this.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"de09c726b4bfed6bd6933de8caae0c09a7972311","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        tools.fail(\u0027Unexpected bridge type: %s\u0027 % type(bridge))"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"class OVSBridgeFixture(BridgeFixture):"},{"line_number":160,"context_line":"    \"\"\"Create an OVS bridge."},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    :ivar bridge: created bridge"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_57f014fe","line":159,"in_reply_to":"da9b358b_a3673524","updated":"2015-04-01 12:19:59.000000000","message":"i follow ovs_lib/interface naming but i agree we are inconsistent","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"badab78d4683b61b7795b393c689fe8cbfcf4d4b","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    :type namespace: str"},{"line_number":198,"context_line":"    \"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def __init__(self):"},{"line_number":201,"context_line":"        super(LinuxBridgeFixture, self).__init__()"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_84eb24b0","line":200,"updated":"2015-03-31 19:01:38.000000000","message":"Looks like this __init__ is not needed, it does not add anything.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"478ff4caa5913ddc987f7e394f7ca26a97dcca18","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    :type namespace: str"},{"line_number":198,"context_line":"    \"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def __init__(self):"},{"line_number":201,"context_line":"        super(LinuxBridgeFixture, self).__init__()"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_ca993321","line":200,"in_reply_to":"da9b358b_84eb24b0","updated":"2015-03-31 19:34:04.000000000","message":"Done","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"}],"neutron/tests/functional/agent/linux/base.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.ovs \u003d ovs_lib.BaseOVS()"},{"line_number":58,"context_line":"        self.ip \u003d ip_lib.IPWrapper()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def create_ovs_bridge(self):"},{"line_number":61,"context_line":"        return self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def create_ovs_port_in_ns(self, br, ns):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_ac687af6","line":60,"updated":"2015-03-27 19:52:27.000000000","message":"I\u0027d rather see these helper methods go away in favor of direct use of the fixtures involved here.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.ovs \u003d ovs_lib.BaseOVS()"},{"line_number":58,"context_line":"        self.ip \u003d ip_lib.IPWrapper()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def create_ovs_bridge(self):"},{"line_number":61,"context_line":"        return self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def create_ovs_port_in_ns(self, br, ns):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4242d53f","line":60,"in_reply_to":"fa963961_ac687af6","updated":"2015-03-27 20:36:20.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def create_ovs_bridge(self):"},{"line_number":61,"context_line":"        return self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def create_ovs_port_in_ns(self, br, ns):"},{"line_number":64,"context_line":"        bridge_fixture \u003d net_helpers.BridgeFixture.get(br)"},{"line_number":65,"context_line":"        return self.useFixture("},{"line_number":66,"context_line":"            bridge_fixture.create_port_in_ns_fixture(ns)).port"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_4c78cec4","line":63,"updated":"2015-03-27 19:52:27.000000000","message":"As per a previous comment, this method has only one caller and fixture use should be inlined instead.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def create_ovs_bridge(self):"},{"line_number":61,"context_line":"        return self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def create_ovs_port_in_ns(self, br, ns):"},{"line_number":64,"context_line":"        bridge_fixture \u003d net_helpers.BridgeFixture.get(br)"},{"line_number":65,"context_line":"        return self.useFixture("},{"line_number":66,"context_line":"            bridge_fixture.create_port_in_ns_fixture(ns)).port"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_a252c111","line":63,"in_reply_to":"fa963961_4c78cec4","updated":"2015-03-27 20:36:20.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ebd6763a2ec88ae0114cb9e652ce141a0651616a","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        return self.useFixture("},{"line_number":66,"context_line":"            bridge_fixture.create_port_in_ns_fixture(ns)).port"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def bind_namespace_to_cidr(self, namespace, br, ip_cidr):"},{"line_number":69,"context_line":"        return self.useFixture("},{"line_number":70,"context_line":"            net_helpers.CidrBoundNamespaceFixture(br, namespace, ip_cidr)).port"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_ac4f5a91","line":68,"updated":"2015-03-27 19:52:27.000000000","message":"This method has only a single caller - please inline usage of fixture.","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e08d88bab60542871e00885d7e89a962f2cb4239","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        return self.useFixture("},{"line_number":66,"context_line":"            bridge_fixture.create_port_in_ns_fixture(ns)).port"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def bind_namespace_to_cidr(self, namespace, br, ip_cidr):"},{"line_number":69,"context_line":"        return self.useFixture("},{"line_number":70,"context_line":"            net_helpers.CidrBoundNamespaceFixture(br, namespace, ip_cidr)).port"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_c24dc573","line":68,"in_reply_to":"fa963961_ac4f5a91","updated":"2015-03-27 20:36:20.000000000","message":"Done","commit_id":"28d91dc8f6b56e96bfaf4eb498307a4df069146f"}],"neutron/tests/functional/agent/linux/test_ovsdb_monitor.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"bda8c11d5989be9038504338cd9facd9f0010c3b","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        root_helper\u003d\" \".join([functional_base.SUDO_CMD] * 2))"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        self._check_test_requirements()"},{"line_number":50,"context_line":"        self.bridge \u003d self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _check_test_requirements(self):"},{"line_number":53,"context_line":"        self.check_command([\u0027ovsdb-client\u0027, \u0027list-dbs\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_32093532","line":50,"updated":"2015-03-31 23:03:44.000000000","message":"I don\u0027t see why this bridge needs to be created here. The only use is by the PortFixture on line 106 and could just as well be created inline in the test.","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"de09c726b4bfed6bd6933de8caae0c09a7972311","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        root_helper\u003d\" \".join([functional_base.SUDO_CMD] * 2))"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        self._check_test_requirements()"},{"line_number":50,"context_line":"        self.bridge \u003d self.useFixture(net_helpers.OVSBridgeFixture()).bridge"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _check_test_requirements(self):"},{"line_number":53,"context_line":"        self.check_command([\u0027ovsdb-client\u0027, \u0027list-dbs\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"da9b358b_7b19c887","line":50,"in_reply_to":"da9b358b_32093532","updated":"2015-04-01 12:19:59.000000000","message":"Done","commit_id":"6b7847eeea7b04744001995f683faa60492e5548"}]}