)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f7c0a6cf85daaa8dc4df6c901c7fe6626f9cba7a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The current method of checking openvswitch and kernel versions for"},{"line_number":10,"context_line":"specific feature support is brittle, distro-specific and unsupportable."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch removes the runtime version checks and implements and test"},{"line_number":13,"context_line":"script which allows testing specific neutron features or can test"},{"line_number":14,"context_line":"that all features required by a specific configuration are available."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1ae5cdf2_af7818f3","line":12,"updated":"2014-06-03 00:42:13.000000000","message":"and test -\u003e a test","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"}],"bin/neutron-check":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4da7dfc0468069e4bcc075b8a58299cf7adb32df","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# Copyright (c) 2014 OpenStack Foundation."}],"source_content_type":"application/octet-stream","patch_set":5,"id":"1ae5cdf2_3152b413","line":1,"updated":"2014-06-02 19:21:30.000000000","message":"the idea of having bin files was abandoned a while back, and functionality should be provided as entry points. See neutron-nsx-check for an example.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"}],"bin/neutron-ovs-check":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"681ed1d97106bca47cdc22f020b650a431140b0e","unresolved":false,"context_lines":[{"line_number":41,"context_line":"# useful info on failure"},{"line_number":42,"context_line":"def test_vxlan():"},{"line_number":43,"context_line":"    name \u003d \"vxlantest-\" + str(random.randint(1, 0x7fffffff))"},{"line_number":44,"context_line":"    with ovs_lib.OVSBridge(name, \u0027sudo\u0027) as br:"},{"line_number":45,"context_line":"        port \u003d br.add_tunnel_port(\u0027192.0.2.1\u0027, \u0027192.0.2.2\u0027, const.TYPE_VXLAN)"},{"line_number":46,"context_line":"        if port \u003d\u003d -1:"},{"line_number":47,"context_line":"            LOG.error(_(\u0027Check for Open vSwitch VXLAN support failed. \u0027"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"3ae8d1ca_1014c64c","line":44,"updated":"2014-05-29 17:48:38.000000000","message":"Just noticed I left this hardcoded to sudo. Need to use root_helper if it is configured.","commit_id":"077418f3eeb29fba479968af5f1832cd470fd4c5"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"4de3395eb69c2fdd6d6e0b6969c389bb6b39fd3b","unresolved":false,"context_lines":[{"line_number":42,"context_line":"def test_vxlan():"},{"line_number":43,"context_line":"    name \u003d \"vxlantest-\" + str(random.randint(1, 0x7fffffff))"},{"line_number":44,"context_line":"    with ovs_lib.OVSBridge(name, \u0027sudo\u0027) as br:"},{"line_number":45,"context_line":"        port \u003d br.add_tunnel_port(\u0027192.0.2.1\u0027, \u0027192.0.2.2\u0027, const.TYPE_VXLAN)"},{"line_number":46,"context_line":"        if port \u003d\u003d -1:"},{"line_number":47,"context_line":"            LOG.error(_(\u0027Check for Open vSwitch VXLAN support failed. \u0027"},{"line_number":48,"context_line":"                   \u0027Please ensure that the version of openvswitch being used \u0027"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"3ae8d1ca_13113005","line":45,"updated":"2014-05-29 17:58:32.000000000","message":"I would suggest using much more obscure IP addresses than these.","commit_id":"077418f3eeb29fba479968af5f1832cd470fd4c5"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"319a2b10bd78c7dd36ec94712df3cb6f556bc47e","unresolved":false,"context_lines":[{"line_number":42,"context_line":"def test_vxlan():"},{"line_number":43,"context_line":"    name \u003d \"vxlantest-\" + str(random.randint(1, 0x7fffffff))"},{"line_number":44,"context_line":"    with ovs_lib.OVSBridge(name, cfg.CONF.AGENT.root_helper) as br:"},{"line_number":45,"context_line":"        port \u003d br.add_tunnel_port(\u0027192.0.2.1\u0027, \u0027192.0.2.2\u0027, const.TYPE_VXLAN)"},{"line_number":46,"context_line":"        port_int \u003d -1"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            port_int \u003d int(port)"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"1ae5cdf2_3e8d8532","line":45,"updated":"2014-05-29 21:21:47.000000000","message":"I\u0027m less concerned about this IP range being used. If I was being incredibly pedant, I would argue we should provide a way for the user to pass in both src and dst IP addresses, but I think what is here is fine.","commit_id":"b171bc3c5acd90fa8d1c873de2c8433e395ce6d3"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"f7b240420237681eb380edd93d53348f403513c5","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# Copyright (c) 2014 OpenStack Foundation."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_026a1a1a","line":1,"updated":"2014-05-30 12:42:26.000000000","message":"The script is never tested by neutron UT or Tempest tests ... if the behavior of OVSBridge changes, no test will highlight it.\n\nIMO, test_vxlan should in ovs_lib and tested.","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"066edf612f14aad147044a5cbb2b3af513e8b75d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# Copyright (c) 2014 OpenStack Foundation."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_25f63bd8","line":1,"in_reply_to":"1ae5cdf2_026a1a1a","updated":"2014-05-30 15:46:21.000000000","message":"I went back and forth on that very issue and went with the simplicity of having everything in one place and thinking that a test could easily be added to tempest to cover this. I can also see the benefit to just moving the whole thing to ovs_lib and using entry points. Or, like you mention just move the testing functions into ovs_lib, then document in this script that that is the proper place for them. I\u0027m open to suggestion. :)","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d9198ff5091021abea93455cc36cadcc953f4f69","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# Copyright (c) 2014 OpenStack Foundation."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_e2ee0422","line":1,"in_reply_to":"1ae5cdf2_25f63bd8","updated":"2014-05-30 16:15:11.000000000","message":"I would just put test_vxlan in ovs_lib and script \"wrap\" there.","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"f7b240420237681eb380edd93d53348f403513c5","unresolved":false,"context_lines":[{"line_number":41,"context_line":"# useful info on failure"},{"line_number":42,"context_line":"def test_vxlan():"},{"line_number":43,"context_line":"    name \u003d \"vxlantest-\" + str(random.randint(1, 0x7fffffff))"},{"line_number":44,"context_line":"    with ovs_lib.OVSBridge(name, cfg.CONF.AGENT.root_helper) as br:"},{"line_number":45,"context_line":"        port \u003d br.add_tunnel_port(\u0027192.0.2.1\u0027, \u0027192.0.2.2\u0027, const.TYPE_VXLAN)"},{"line_number":46,"context_line":"        port_int \u003d -1"},{"line_number":47,"context_line":"        try:"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_874f1c92","line":44,"updated":"2014-05-30 12:42:26.000000000","message":"Is it compliant with rootwrap-daemon-mode[1] ? Perhaps we should also allow script users to specify root_helper in the command line ?\n\n[1] https://review.openstack.org/#/q/topic:bp/rootwrap-daemon-mode,n,z","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"066edf612f14aad147044a5cbb2b3af513e8b75d","unresolved":false,"context_lines":[{"line_number":41,"context_line":"# useful info on failure"},{"line_number":42,"context_line":"def test_vxlan():"},{"line_number":43,"context_line":"    name \u003d \"vxlantest-\" + str(random.randint(1, 0x7fffffff))"},{"line_number":44,"context_line":"    with ovs_lib.OVSBridge(name, cfg.CONF.AGENT.root_helper) as br:"},{"line_number":45,"context_line":"        port \u003d br.add_tunnel_port(\u0027192.0.2.1\u0027, \u0027192.0.2.2\u0027, const.TYPE_VXLAN)"},{"line_number":46,"context_line":"        port_int \u003d -1"},{"line_number":47,"context_line":"        try:"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_cad124cf","line":44,"in_reply_to":"1ae5cdf2_874f1c92","updated":"2014-05-30 15:46:21.000000000","message":"There\u0027s no reason I can think of that it shouldn\u0027t work with daemon mode. The default is \u0027sudo\u0027, so if no config file is passed in that\u0027s what it should be (actually, that isn\u0027t the case until I move the registering of the AGENT opts inside the conditional for config-file below...which I\u0027ll do). If root_helper is set up, this should be able to use it just like anything else.","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"f7b240420237681eb380edd93d53348f403513c5","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# Define CLI opts to test specific features, with a calback for the test"},{"line_number":61,"context_line":"OPTS \u003d ["},{"line_number":62,"context_line":"    BoolOptCallback(\u0027vxlan\u0027, test_vxlan, default\u003dFalse,"},{"line_number":63,"context_line":"                    help\u003d_(\u0027Check for vxlan support\u0027)),"},{"line_number":64,"context_line":"]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_c22592ac","line":63,"updated":"2014-05-30 12:42:26.000000000","message":"We should also provide \"Check for patch (port) support\" because patch ports are required when using tunnels.\n\nIf you want, i can provide the associated implementation in a daughter change.","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d9198ff5091021abea93455cc36cadcc953f4f69","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# Define CLI opts to test specific features, with a calback for the test"},{"line_number":61,"context_line":"OPTS \u003d ["},{"line_number":62,"context_line":"    BoolOptCallback(\u0027vxlan\u0027, test_vxlan, default\u003dFalse,"},{"line_number":63,"context_line":"                    help\u003d_(\u0027Check for vxlan support\u0027)),"},{"line_number":64,"context_line":"]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_df363f03","line":63,"in_reply_to":"1ae5cdf2_8a955ce6","updated":"2014-05-30 16:15:11.000000000","message":"fine for me, i will provide patch port check","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"066edf612f14aad147044a5cbb2b3af513e8b75d","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# Define CLI opts to test specific features, with a calback for the test"},{"line_number":61,"context_line":"OPTS \u003d ["},{"line_number":62,"context_line":"    BoolOptCallback(\u0027vxlan\u0027, test_vxlan, default\u003dFalse,"},{"line_number":63,"context_line":"                    help\u003d_(\u0027Check for vxlan support\u0027)),"},{"line_number":64,"context_line":"]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_8a955ce6","line":63,"in_reply_to":"1ae5cdf2_c22592ac","updated":"2014-05-30 15:46:21.000000000","message":"I\u0027m ok with this being done as a separate enhancement. I mostly interested in getting the design right for being able to implement these kinds of checks and for removing the runtime checks with this patch.","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"f7b240420237681eb380edd93d53348f403513c5","unresolved":false,"context_lines":[{"line_number":82,"context_line":"    return res"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def main():"},{"line_number":86,"context_line":"    eventlet.monkey_patch()"},{"line_number":87,"context_line":"    cfg.CONF.register_cli_opts(OPTS)"},{"line_number":88,"context_line":"    cfg.CONF.register_opts(agent_opts, \u0027AGENT\u0027)"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_076f8c93","line":85,"updated":"2014-05-30 12:42:26.000000000","message":"Do we really need to enable eventlet ?","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"066edf612f14aad147044a5cbb2b3af513e8b75d","unresolved":false,"context_lines":[{"line_number":82,"context_line":"    return res"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def main():"},{"line_number":86,"context_line":"    eventlet.monkey_patch()"},{"line_number":87,"context_line":"    cfg.CONF.register_cli_opts(OPTS)"},{"line_number":88,"context_line":"    cfg.CONF.register_opts(agent_opts, \u0027AGENT\u0027)"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"1ae5cdf2_2a09a831","line":85,"in_reply_to":"1ae5cdf2_076f8c93","updated":"2014-05-30 15:46:21.000000000","message":"Oops, nope!","commit_id":"5ff68cf81e41c8ba7241f8ec8f98e9d3f7208ed2"}],"neutron/agent/linux/ovs_lib.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f7c0a6cf85daaa8dc4df6c901c7fe6626f9cba7a","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    def port_exists(self, port_name):"},{"line_number":100,"context_line":"        return bool(self.get_bridge_name_for_port_name(port_name))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_8fcd1c60","line":102,"updated":"2014-06-03 00:42:13.000000000","message":"I\u0027d like to see this method moved to the check script on the basis of YAGNI.  I don\u0027t see any need to expose it in the ovs_lib api since it isn\u0027t  used anywhere else and should not be called at runtime.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d351119128259843663cddb93138a06950e4eeeb","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    def port_exists(self, port_name):"},{"line_number":100,"context_line":"        return bool(self.get_bridge_name_for_port_name(port_name))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_06be6065","line":102,"updated":"2014-06-02 19:12:29.000000000","message":"nit: vxlan_supported is defined in BaseOVS and depends on OVSBridge which is a subclass of BaseOVS ... IMO, it seems more logic to define it in OVSBridge (as a class method) or even better as a module function.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e57fffbd1f3292841ff185701f067865b637e272","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    def port_exists(self, port_name):"},{"line_number":100,"context_line":"        return bool(self.get_bridge_name_for_port_name(port_name))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_40a4cb56","line":102,"in_reply_to":"1ae5cdf2_06be6065","updated":"2014-06-02 21:07:07.000000000","message":"Either way it kind of sucks.. If you look, OVSBase already depends on OVSBridge in a kind of silly way. And having vxlan_supported, which instantiates an OVSBridge itself, in OVSBridge isn\u0027t any cleaner. And this has to be in one of the two, otherwise testing becomes more of a pain because we need to have execute() mocked out.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"9a5145493605df49c11a87733e38beccaa65da95","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    def port_exists(self, port_name):"},{"line_number":100,"context_line":"        return bool(self.get_bridge_name_for_port_name(port_name))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_63cf1180","line":102,"in_reply_to":"1ae5cdf2_40a4cb56","updated":"2014-06-02 21:28:30.000000000","message":"that\u0027s why, IMHO, a module function seems better/clearer","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f7c0a6cf85daaa8dc4df6c901c7fe6626f9cba7a","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"},{"line_number":106,"context_line":"            port_int \u003d -1"},{"line_number":107,"context_line":"            try:"},{"line_number":108,"context_line":"                port_int \u003d int(port)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_2fe968b8","line":105,"updated":"2014-06-03 00:42:13.000000000","message":"Would it make sense to integrate this kind of error handling to add_tunnel_port() to ensure that failures to allocate vxlan ports could be logged as errors indicating that there may be a problem with vxlan support?","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"2015e637b01f9eefbe4c5c5aebafe27c320e1aef","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"},{"line_number":106,"context_line":"            port_int \u003d -1"},{"line_number":107,"context_line":"            try:"},{"line_number":108,"context_line":"                port_int \u003d int(port)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_522654cf","line":105,"in_reply_to":"1ae5cdf2_2fe968b8","updated":"2014-06-03 06:19:32.000000000","message":"+1","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"7bcaf02c27520a92b56baaf772d5627377ec9f15","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def vxlan_supported(self, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":103,"context_line":"        name \u003d \"vxlantest-\" + common_utils.get_random_string(6)"},{"line_number":104,"context_line":"        with OVSBridge(name, self.root_helper) as br:"},{"line_number":105,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, p_const.TYPE_VXLAN)"},{"line_number":106,"context_line":"            port_int \u003d -1"},{"line_number":107,"context_line":"            try:"},{"line_number":108,"context_line":"                port_int \u003d int(port)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_8a218ab6","line":105,"in_reply_to":"1ae5cdf2_2fe968b8","updated":"2014-06-03 01:53:31.000000000","message":"+1, that actually makes a lot of sense to me.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"49c72124b384c8ddfa412236adc0cca2801accbe","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        # This can return a non-integer string, like \u0027[]\u0027 so ensure a"},{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"        except ValueError:"},{"line_number":181,"context_line":"            return \u0027-1\u0027"},{"line_number":182,"context_line":"        return ofport"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_008704d8","line":179,"updated":"2014-06-03 15:26:16.000000000","message":"return int(port)","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c37edd1f603faefb9202b3fdb8cf479636f66735","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        # This can return a non-integer string, like \u0027[]\u0027 so ensure a"},{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"        except ValueError:"},{"line_number":181,"context_line":"            return \u0027-1\u0027"},{"line_number":182,"context_line":"        return ofport"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_54daade8","line":179,"in_reply_to":"1ae5cdf2_008704d8","updated":"2014-06-03 16:09:03.000000000","message":"It has always returned a string before. I don\u0027t want to change the type of the return value. So I\u0027d have to return ofport after doing the int(ofport) anyway. I can go ahead and move the return ofport inside the try if you prefer.","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return \u0027-1\u0027"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_datapath_id(self):"},{"line_number":185,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_43975148","line":182,"updated":"2014-06-03 19:21:42.000000000","message":"Please prefer a constant to a magic string.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return \u0027-1\u0027"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_datapath_id(self):"},{"line_number":185,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_7e4780bc","line":182,"in_reply_to":"1ae5cdf2_43975148","updated":"2014-06-03 20:58:24.000000000","message":"Done","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"8989c261476a083cd8ceeaee64c1a6ccfc8ccc25","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_350ee9e5","line":180,"range":{"start_line":180,"start_character":19,"end_line":180,"end_character":25},"updated":"2014-06-04 04:22:16.000000000","message":"probably it isn\u0027t your fault, but....\n\nwhy not returning an integer?\nit seems inconsistent as INVALID_OFPORT is an integer.","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"4fe00006340086cf2db5be57e58bede2ba702845","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_b59d9968","line":180,"in_reply_to":"1ae5cdf2_350ee9e5","updated":"2014-06-04 04:26:29.000000000","message":"Because it has never returned an integer and I don\u0027t want to change the return type of a library function as part of a completely unrelated patch because that just seems like a really bad idea.","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"2825be4c7db4e1161970e289f8acf1823513208f","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_95017d3a","line":180,"in_reply_to":"1ae5cdf2_35fd096f","updated":"2014-06-04 04:46:36.000000000","message":"Honestly, after reading the code in the file, the TODO: would be something like \"rewrite all of this as an ovsdb client\". The entire premise of doing the get_port_ofport seems like a bad idea to me as it is used in things like add_port after running vsctl to create the port, then calling this to get the ofport by name afterward. This seems inherently race-condition prone to me since all kinds of things could happen between those calls.\n\nThe only reason I\u0027m touching this function at all is because I was tired of seeing various versions of this try/except scattered around all over the place and I would have had to add another one when adding the vxlan add_tunnel_port error log message.\n\nSo I\u0027m really not sure refactoring just the return value of this function really buys us anything. We aren\u0027t going to do math on the value, so whether it is a string or a number doesn\u0027t really matter.","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"4969240797849dd77c1efa70b3e94950d38aaf64","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_d5014507","line":180,"in_reply_to":"1ae5cdf2_95017d3a","updated":"2014-06-04 04:55:42.000000000","message":"ok, i agree it should be a separate topic from this.","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"c2c188d8e350cf638a2e2efc9081d17aabe6bb49","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        # common failure case"},{"line_number":178,"context_line":"        try:"},{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_35fd096f","line":180,"in_reply_to":"1ae5cdf2_b59d9968","updated":"2014-06-04 04:34:53.000000000","message":"how about leaving a todo comment?","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c28a1f7fd9a3498096619926b8e31c1e2bcc60d","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_datapath_id(self):"},{"line_number":185,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_c2cfbb54","line":182,"updated":"2014-06-04 14:04:52.000000000","message":"Based on the conversation above, shouldn\u0027t this be a string then (or better in the constants module itself)?","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"1335b58021858af65bd8d1ebf753b0a09e048a8b","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            int(ofport)"},{"line_number":180,"context_line":"            return ofport"},{"line_number":181,"context_line":"        except ValueError:"},{"line_number":182,"context_line":"            return constants.INVALID_OFPORT"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_datapath_id(self):"},{"line_number":185,"context_line":"        return self.db_get_val(\u0027Bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_e18674da","line":182,"in_reply_to":"1ae5cdf2_c2cfbb54","updated":"2014-06-04 14:27:40.000000000","message":"Bah. Good catch. Changed INVALID_OFPORT to \u0027-1\u0027 in constants.","commit_id":"1c08e30de1ffdb9fab6f927d3790229d1f5646c8"}],"neutron/cmd/sanity/checks.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"49c72124b384c8ddfa412236adc0cca2801accbe","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"def vxlan_supported(root_helper, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":24,"context_line":"        name \u003d \"vxlantest-\" + utils.get_random_string(6)"},{"line_number":25,"context_line":"        with ovs_lib.OVSBridge(name, root_helper) as br:"},{"line_number":26,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_604c48c9","line":23,"updated":"2014-06-03 15:26:16.000000000","message":"small nit: Invalid indentation","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c37edd1f603faefb9202b3fdb8cf479636f66735","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"def vxlan_supported(root_helper, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"},{"line_number":24,"context_line":"        name \u003d \"vxlantest-\" + utils.get_random_string(6)"},{"line_number":25,"context_line":"        with ovs_lib.OVSBridge(name, root_helper) as br:"},{"line_number":26,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, const.TYPE_VXLAN)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_d796bf3e","line":23,"in_reply_to":"1ae5cdf2_604c48c9","updated":"2014-06-03 16:09:03.000000000","message":"Done","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"49c72124b384c8ddfa412236adc0cca2801accbe","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        name \u003d \"vxlantest-\" + utils.get_random_string(6)"},{"line_number":25,"context_line":"        with ovs_lib.OVSBridge(name, root_helper) as br:"},{"line_number":26,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, const.TYPE_VXLAN)"},{"line_number":27,"context_line":"            return False if port \u003d\u003d \u0027-1\u0027 else True"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_20dde0e0","line":27,"updated":"2014-06-03 15:26:16.000000000","message":"seems clearer:\n\n return port !\u003d \u0027-1\u0027","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c37edd1f603faefb9202b3fdb8cf479636f66735","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        name \u003d \"vxlantest-\" + utils.get_random_string(6)"},{"line_number":25,"context_line":"        with ovs_lib.OVSBridge(name, root_helper) as br:"},{"line_number":26,"context_line":"            port \u003d br.add_tunnel_port(from_ip, to_ip, const.TYPE_VXLAN)"},{"line_number":27,"context_line":"            return False if port \u003d\u003d \u0027-1\u0027 else True"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_74fa4944","line":27,"in_reply_to":"1ae5cdf2_20dde0e0","updated":"2014-06-03 16:09:03.000000000","message":"True.","commit_id":"d3672c330b30327a26709d86738e6d4f021ad0c8"}],"neutron/cmd/sanity_check.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":21,"context_line":"from neutron.common import config"},{"line_number":22,"context_line":"from neutron.openstack.common import log as logging"},{"line_number":23,"context_line":"import neutron.plugins.openvswitch.common.config  # noqa / registers agent ops"},{"line_number":24,"context_line":"from oslo.config import cfg"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_03d0e9c7","line":23,"updated":"2014-06-03 19:21:42.000000000","message":"Though you wouldn\u0027t know it from reviewing the existing codebase, ensuring the availability of configuration options by importing modules in this fashion is not good practice.  The way to explicitly accomplish the same thing is by using the import_opt/import_group methods of the config object, e.g.\n\nhttps://github.com/openstack/neutron/blob/master/neutron/tests/unit/ml2/test_ml2_plugin.py#L39","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":21,"context_line":"from neutron.common import config"},{"line_number":22,"context_line":"from neutron.openstack.common import log as logging"},{"line_number":23,"context_line":"import neutron.plugins.openvswitch.common.config  # noqa / registers agent ops"},{"line_number":24,"context_line":"from oslo.config import cfg"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_de517437","line":23,"in_reply_to":"1ae5cdf2_03d0e9c7","updated":"2014-06-03 20:58:24.000000000","message":"Ah, handy. Thanks. I\u0027ll use import_group.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class BoolOptCallback(cfg.BoolOpt):"},{"line_number":31,"context_line":"    def __init__(self, name, callback, **kwargs):"},{"line_number":32,"context_line":"        self.callback \u003d callback"},{"line_number":33,"context_line":"        super(BoolOptCallback, self).__init__(name, **kwargs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_63da8de9","line":30,"updated":"2014-06-03 19:21:42.000000000","message":"I\u0027m assuming you took this approach of modularity in response to comments from previous reviewers, but in future please avoid overcomplicating implementation until clear use cases present (YAGNI).","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class BoolOptCallback(cfg.BoolOpt):"},{"line_number":31,"context_line":"    def __init__(self, name, callback, **kwargs):"},{"line_number":32,"context_line":"        self.callback \u003d callback"},{"line_number":33,"context_line":"        super(BoolOptCallback, self).__init__(name, **kwargs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_791b1a73","line":30,"in_reply_to":"1ae5cdf2_63da8de9","updated":"2014-06-03 20:58:24.000000000","message":"I\u0027m not sure what approach to modularity this comment is referring to.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def check_ovs_vxlan():"},{"line_number":37,"context_line":"    result \u003d checks.vxlan_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":38,"context_line":"    if result is not True:"},{"line_number":39,"context_line":"        LOG.error(_(\u0027Check for Open vSwitch VXLAN support failed. \u0027"},{"line_number":40,"context_line":"                    \u0027Please ensure that the version of openvswitch \u0027"},{"line_number":41,"context_line":"                    \u0027being used has VXLAN support.\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_83fa3941","line":38,"updated":"2014-06-03 19:21:42.000000000","message":"Prefer implicit boolean evaluation, e.g.\n\nif not result:\n   pass","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def check_ovs_vxlan():"},{"line_number":37,"context_line":"    result \u003d checks.vxlan_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":38,"context_line":"    if result is not True:"},{"line_number":39,"context_line":"        LOG.error(_(\u0027Check for Open vSwitch VXLAN support failed. \u0027"},{"line_number":40,"context_line":"                    \u0027Please ensure that the version of openvswitch \u0027"},{"line_number":41,"context_line":"                    \u0027being used has VXLAN support.\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_992f0688","line":38,"in_reply_to":"1ae5cdf2_83fa3941","updated":"2014-06-03 20:58:24.000000000","message":"Done","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"}],"neutron/common/config.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"ce33fcc7f6763ef1854350006e337aa44dab33b4","unresolved":false,"context_lines":[{"line_number":132,"context_line":"                        sqlite_db\u003d\u0027\u0027, max_pool_size\u003d10,"},{"line_number":133,"context_line":"                        max_overflow\u003d20, pool_timeout\u003d10)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"def parse(args, **kwargs):"},{"line_number":137,"context_line":"    cfg.CONF(args\u003dargs, project\u003d\u0027neutron\u0027,"},{"line_number":138,"context_line":"             version\u003d\u0027%%prog %s\u0027 % version.version_info.release_string(),"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_e8f33e08","line":135,"updated":"2014-05-30 19:18:02.000000000","message":"A tiny change but I think it should be split off to a separate patch.","commit_id":"54422893f0f0728d782d9f5092638e0120560f82"}],"neutron/tests/functional/sanity/test_ovs_sanity.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from neutron.tests import base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class OVSSanityTestCase(base.BaseTestCase):"},{"line_number":26,"context_line":"    def setUp(self):"},{"line_number":27,"context_line":"        super(OVSSanityTestCase, self).setUp()"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_a38e55ce","line":25,"updated":"2014-06-03 19:21:42.000000000","message":"Is there a reason not to extend BaseLinuxTestCase or refactor it for reuse, rather than duplicating things like check_sudo_enabled()?","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from neutron.tests import base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class OVSSanityTestCase(base.BaseTestCase):"},{"line_number":26,"context_line":"    def setUp(self):"},{"line_number":27,"context_line":"        super(OVSSanityTestCase, self).setUp()"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_f9a3aafd","line":25,"in_reply_to":"1ae5cdf2_a38e55ce","updated":"2014-06-03 20:58:24.000000000","message":"Basically just because I didn\u0027t want to pollute this patch with more refactoring of other code. It starts to get hard to review after a while and it was such a very small thing that I needed, it just didn\u0027t seem worth it. Seemed like doing a refactor as a separate step later if there was more overlap would be the thing to do.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self.check_sudo_enabled()"},{"line_number":44,"context_line":"        with mock.patch("},{"line_number":45,"context_line":"            \u0027neutron.agent.linux.ovs_lib.OVSBridge.add_tunnel_port\u0027,"},{"line_number":46,"context_line":"            return_value\u003doutput) as add_tunnel_port:"},{"line_number":47,"context_line":"            result \u003d checks.vxlan_supported(self.root_helper)"},{"line_number":48,"context_line":"            self.assertTrue(add_tunnel_port.called)"},{"line_number":49,"context_line":"            self.assertEqual(result, expected,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_0398e968","line":46,"updated":"2014-06-03 19:21:42.000000000","message":"As per pep8 (unfortunately no automatic check), please indent this line and the one previous by 4 spaces to differentiate from the block that follows.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self.check_sudo_enabled()"},{"line_number":44,"context_line":"        with mock.patch("},{"line_number":45,"context_line":"            \u0027neutron.agent.linux.ovs_lib.OVSBridge.add_tunnel_port\u0027,"},{"line_number":46,"context_line":"            return_value\u003doutput) as add_tunnel_port:"},{"line_number":47,"context_line":"            result \u003d checks.vxlan_supported(self.root_helper)"},{"line_number":48,"context_line":"            self.assertTrue(add_tunnel_port.called)"},{"line_number":49,"context_line":"            self.assertEqual(result, expected,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_7974fa35","line":46,"in_reply_to":"1ae5cdf2_0398e968","updated":"2014-06-03 20:58:24.000000000","message":"Done","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"192c5c24d4b80684a226e8aa83a6d08a9d32cd8a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                             \u0027With output %s, expected %s and got %s\u0027 % ("},{"line_number":51,"context_line":"                             output, expected, result))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_vxlan_supported(self):"},{"line_number":54,"context_line":"        expected_results \u003d {"},{"line_number":55,"context_line":"            \u0027[]\u0027: False,"},{"line_number":56,"context_line":"            \u0027-1\u0027: False,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_c3060186","line":53,"updated":"2014-06-03 19:21:42.000000000","message":"Should this test be run as part of the unit test suite instead of the functional suite? test_ovs_vxlan_support_runs() validates system interaction, and would appear to already cover this method\u0027s validation of the bridge lifecycle.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5ff82f354d6ec3243d652cfc078dbbb034031295","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                             \u0027With output %s, expected %s and got %s\u0027 % ("},{"line_number":51,"context_line":"                             output, expected, result))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_vxlan_supported(self):"},{"line_number":54,"context_line":"        expected_results \u003d {"},{"line_number":55,"context_line":"            \u0027[]\u0027: False,"},{"line_number":56,"context_line":"            \u0027-1\u0027: False,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ae5cdf2_39d0b21e","line":53,"in_reply_to":"1ae5cdf2_c3060186","updated":"2014-06-03 20:58:24.000000000","message":"It *could* be. I\u0027d have to mock out some extra stuff like bridge creation/deletion, etc. It really isn\u0027t exactly a unit test and it isn\u0027t really exactly a functional test. I put it here mostly for the sake of simplicity/keeping related tests together. I\u0027ll go ahead and see about moving it.","commit_id":"5ef26ad3e564f6345c29b01e839735188a07e287"}],"neutron/tests/unit/agent/linux/test_ovs_lib.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"2dc4dd19eda97fb855199c91a529b119f2454b96","unresolved":false,"context_lines":[{"line_number":916,"context_line":"        self._check_ovs_vxlan_version(min_vxlan_ver, min_vxlan_ver,"},{"line_number":917,"context_line":"                                      min_kernel_ver, expecting_ok\u003dTrue)"},{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    def test_ofctl_arg_supported(self):"},{"line_number":920,"context_line":"        with mock.patch(\u0027neutron.common.utils.get_random_string\u0027) as utils:"},{"line_number":921,"context_line":"            utils.return_value \u003d \u0027test\u0027"},{"line_number":922,"context_line":"            supported \u003d ovs_lib.ofctl_arg_supported(self.root_helper, \u0027cmd\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_53de586e","side":"PARENT","line":919,"updated":"2014-05-29 17:56:30.000000000","message":"Oops, deleted an extra test. I\u0027ll fix that too. Stupid rebase merge conflicts!","commit_id":"b33ecb3d1617e61e7ff1bb84d774d2a14dc31a54"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d351119128259843663cddb93138a06950e4eeeb","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    def test_port_exists_returns_false_for_none(self):"},{"line_number":107,"context_line":"        self._test_port_exists(None, False)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _test_vxlan_supported(self, output, expected):"},{"line_number":110,"context_line":"        with mock.patch("},{"line_number":111,"context_line":"            \u0027neutron.agent.linux.ovs_lib.OVSBridge.add_tunnel_port\u0027,"},{"line_number":112,"context_line":"            return_value\u003doutput):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_510bc024","line":109,"updated":"2014-06-02 19:12:29.000000000","message":"OVSBridge create/destroy are not mocked \u003d\u003d\u003e OVSBridge will really call ovs-vsctl \u003d\u003d\u003e. tests will fail","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e57fffbd1f3292841ff185701f067865b637e272","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    def test_port_exists_returns_false_for_none(self):"},{"line_number":107,"context_line":"        self._test_port_exists(None, False)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _test_vxlan_supported(self, output, expected):"},{"line_number":110,"context_line":"        with mock.patch("},{"line_number":111,"context_line":"            \u0027neutron.agent.linux.ovs_lib.OVSBridge.add_tunnel_port\u0027,"},{"line_number":112,"context_line":"            return_value\u003doutput):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_2046df7c","line":109,"in_reply_to":"1ae5cdf2_510bc024","updated":"2014-06-02 21:07:07.000000000","message":"Bah, I even tested this at one point, but in my own test class which mocked out execute() like OVS_Lib_Test, but w/o creating a bridge, etc. and then moved it thinking this class took care of it when it didn\u0027t. I\u0027ll move stuff around more. I repeat. Bah.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"}],"setup.cfg":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4da7dfc0468069e4bcc075b8a58299cf7adb32df","unresolved":false,"context_lines":[{"line_number":75,"context_line":"scripts \u003d"},{"line_number":76,"context_line":"    bin/neutron-rootwrap"},{"line_number":77,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":78,"context_line":"    bin/neutron-check"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"[global]"},{"line_number":81,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"1ae5cdf2_71819c49","line":78,"updated":"2014-06-02 19:21:30.000000000","message":"please, remove this script and install an entry point. At one time, the entire bin directory was dropped in favor of entry points. See pointer below. Also could I suggest a better name, i.e. neutron-sanity-checks perhaps?","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"39a7a22baf3716530740253dc043635e185dfc53","unresolved":false,"context_lines":[{"line_number":75,"context_line":"scripts \u003d"},{"line_number":76,"context_line":"    bin/neutron-rootwrap"},{"line_number":77,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":78,"context_line":"    bin/neutron-check"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"[global]"},{"line_number":81,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"1ae5cdf2_4314d5f4","line":78,"in_reply_to":"1ae5cdf2_03a00df3","updated":"2014-06-02 21:20:16.000000000","message":"I am not sure where we\u0027d go debating about this; there were good reasons when this decision was made back then, if you\u0027re interested I can try and find out the exact commit id :)\n\nAs for where this would leave in the tree, maybe we could have a main under /neutron that pulls the various commands that live in the specific subparts of the tree, a bit like you did.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"9a5145493605df49c11a87733e38beccaa65da95","unresolved":false,"context_lines":[{"line_number":75,"context_line":"scripts \u003d"},{"line_number":76,"context_line":"    bin/neutron-rootwrap"},{"line_number":77,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":78,"context_line":"    bin/neutron-check"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"[global]"},{"line_number":81,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"1ae5cdf2_239b4968","line":78,"in_reply_to":"1ae5cdf2_4314d5f4","updated":"2014-06-02 21:28:30.000000000","message":"Script are difficult to test, entry-points as function are easier to test (which do not imply we do it :))","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e57fffbd1f3292841ff185701f067865b637e272","unresolved":false,"context_lines":[{"line_number":75,"context_line":"scripts \u003d"},{"line_number":76,"context_line":"    bin/neutron-rootwrap"},{"line_number":77,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":78,"context_line":"    bin/neutron-check"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"[global]"},{"line_number":81,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"1ae5cdf2_03a00df3","line":78,"in_reply_to":"1ae5cdf2_71819c49","updated":"2014-06-02 21:07:07.000000000","message":"I\u0027m not (yet!) convinced that entry points offer any benefit to this script and really just add more obfuscation. What is the benefit, here? Just convention? And if just convention, why do we have any bin/ scripts? Also, where would a script like this live in the tree? I don\u0027t see any place that looks like a particularly good place for something as general as this.","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4da7dfc0468069e4bcc075b8a58299cf7adb32df","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"[entry_points]"},{"line_number":86,"context_line":"console_scripts \u003d"},{"line_number":87,"context_line":"    neutron-check-nsx-config \u003d neutron.plugins.vmware.check_nsx_config:main"},{"line_number":88,"context_line":"    neutron-db-manage \u003d neutron.db.migration.cli:main"},{"line_number":89,"context_line":"    neutron-debug \u003d neutron.debug.shell:main"},{"line_number":90,"context_line":"    neutron-dhcp-agent \u003d neutron.agent.dhcp_agent:main"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"1ae5cdf2_715c3c26","line":87,"updated":"2014-06-02 19:21:30.000000000","message":"here (in reference to my previous comment)","commit_id":"785894820a78c0efadf60b0e463efed69f1b3491"}]}