)]}'
{"ovsdbapp/schema/open_vswitch/helpers.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6470f2535af32bcdf3810c10e0f3580ab815c1dd","unresolved":false,"context_lines":[{"line_number":30,"context_line":"#                 set Manager outside of ovsdbapp."},{"line_number":31,"context_line":"def enable_connection_uri(conn_uri, execute\u003dNone, **kwargs):"},{"line_number":32,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 5)"},{"line_number":33,"context_line":"    probe \u003d kwargs.pop(\u0027inactivity_probe\u0027, None)"},{"line_number":34,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":35,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":36,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_8fdc8547","line":33,"range":{"start_line":33,"start_character":24,"end_line":33,"end_character":40},"updated":"2020-04-20 18:55:04.000000000","message":"This is clearly a better argument name--the only thing I worry about is that ovsdbapp is not used just in OpenStack projects, so theoretically this would be an API change that could break existing code (though it seems unlikely). Not sure if it is worth doing:\n\n probe \u003d kwargs.pop(\u0027set_timeout\u0027, kwargs.pop(\u0027inactivity_probe\u0027, None))\n\nto handle either?","commit_id":"929131c0212fc423da4b6159a277120b80681db9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8bceab24b3703d04fc2f22c1188dd9cdddeae940","unresolved":false,"context_lines":[{"line_number":30,"context_line":"#                 set Manager outside of ovsdbapp."},{"line_number":31,"context_line":"def enable_connection_uri(conn_uri, execute\u003dNone, **kwargs):"},{"line_number":32,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 5)"},{"line_number":33,"context_line":"    probe \u003d kwargs.pop(\u0027inactivity_probe\u0027, None)"},{"line_number":34,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":35,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":36,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_01512163","line":33,"range":{"start_line":33,"start_character":24,"end_line":33,"end_character":40},"in_reply_to":"1f493fa4_8fdc8547","updated":"2020-04-23 16:30:12.000000000","message":"Right, I didn\u0027t think about this. I just made a search in http://codesearch.openstack.org/.\n\nThis is indeed breaking the current API. I\u0027ll add your solution.","commit_id":"929131c0212fc423da4b6159a277120b80681db9"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b9993b56f9a183c90d086880c85a024159a8a330","unresolved":false,"context_lines":[{"line_number":30,"context_line":"#                 set Manager outside of ovsdbapp."},{"line_number":31,"context_line":"def enable_connection_uri(conn_uri, execute\u003dNone, **kwargs):"},{"line_number":32,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 5)"},{"line_number":33,"context_line":"    probe \u003d kwargs.pop(\u0027set_timeout\u0027, kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":34,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":35,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":36,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f493fa4_38027e0a","line":33,"range":{"start_line":33,"start_character":23,"end_line":33,"end_character":36},"updated":"2020-04-24 07:13:56.000000000","message":"from original code it seems \u0027set_timeout\u0027 is a bool parameter","commit_id":"16feda5d3c12b5f66c856696152b41fa2f82e1cd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"42698d4d87336a3b45238e7303b71d59ebb14252","unresolved":false,"context_lines":[{"line_number":30,"context_line":"#                 set Manager outside of ovsdbapp."},{"line_number":31,"context_line":"def enable_connection_uri(conn_uri, execute\u003dNone, **kwargs):"},{"line_number":32,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 5)"},{"line_number":33,"context_line":"    probe \u003d kwargs.pop(\u0027set_timeout\u0027, kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":34,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":35,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":36,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f493fa4_f8a65653","line":33,"range":{"start_line":33,"start_character":23,"end_line":33,"end_character":36},"in_reply_to":"1f493fa4_38027e0a","updated":"2020-04-24 08:07:38.000000000","message":"It is, you are right. I need to refactor this. Good catch!!","commit_id":"16feda5d3c12b5f66c856696152b41fa2f82e1cd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"40e53aed1b64c89dfbaf28913d0414396cef216f","unresolved":false,"context_lines":[{"line_number":30,"context_line":"#                 set Manager outside of ovsdbapp."},{"line_number":31,"context_line":"def enable_connection_uri(conn_uri, execute\u003dNone, **kwargs):"},{"line_number":32,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 5)"},{"line_number":33,"context_line":"    probe \u003d kwargs.pop(\u0027set_timeout\u0027, kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":34,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":35,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":36,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f493fa4_732549ab","line":33,"range":{"start_line":33,"start_character":23,"end_line":33,"end_character":36},"in_reply_to":"1f493fa4_f8a65653","updated":"2020-04-24 08:20:06.000000000","message":"But now I review the code, the old code needs to be changed.\n\n\"timeout\" (the command timeout) is in seconds, the probe timeout is defined in milliseconds. The current code works but not in the way it should.","commit_id":"16feda5d3c12b5f66c856696152b41fa2f82e1cd"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"86f282c0c78103b304723fb19c67870ad4004dac","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_776ff491","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"updated":"2020-04-24 09:47:14.000000000","message":"for probe shouldn\u0027t exact \u0027inactivity_probe\u0027 be prioritized over timeout/set_timeout?","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6c1d47bac7cbe57cdd90b8d7c6e7d745d50d0e6e","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_713c1ce2","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"in_reply_to":"1f493fa4_3a3f11d7","updated":"2020-04-24 14:44:29.000000000","message":"Neutron does not use \"set_timeout\". If any other project is calling this method and \"set_timeout\" is defined, \"timeout * 1000) will be set. Instead of this, Neutron will use only \"inactivity_probe\".\n\nAnyway, because both are exclusive parameters (set_timeout / inactivity_probe) and Neutron is only going to call this method with the second one, the parameter prevalence won\u0027t matter. Any other project will keep the current behavior.\n\nAbout the API, we need to be careful with \"kwargs\" because is also used in the command execution:\n  return execute(cmd, **kwargs).rstrip()\n\nAny extra argument added should be removed first.","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ab657e5dffb94c88daf213efe8ef93fd72f9304f","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_d4223900","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"in_reply_to":"1f493fa4_4c04e77b","updated":"2020-04-24 17:35:11.000000000","message":"Correct","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a07bb0337fe086d7f724a59c78ad831994d44aab","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_4c04e77b","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"in_reply_to":"1f493fa4_713c1ce2","updated":"2020-04-24 15:00:18.000000000","message":"I guess need to change kwargs.get(\u0027set_timeout\u0027) -\u003e kwargs.pop(\u0027set_timeout\u0027, None) as well","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"803185b5ac68e26981cb242ae3a52f6acacfcede","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_9a56255c","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"in_reply_to":"1f493fa4_776ff491","updated":"2020-04-24 11:14:59.000000000","message":"We should keep the current API: https://review.opendev.org/#/c/720785/2/ovsdbapp/schema/open_vswitch/helpers.py@33\n\nHowever, with this patch we are not keeping 100% the old API. We are assigning to \"probe\" the timeout but converting the units.","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"00e9e9676becd90de7d280c49a8b483d4a9278ab","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # NOTE(ralonsoh): the command timeout , \"timeout\", is defined in seconds;"},{"line_number":34,"context_line":"    # the probe timeout is defined in milliseconds. If \"timeout\" is used, must"},{"line_number":35,"context_line":"    # be converted to ms."},{"line_number":36,"context_line":"    probe \u003d (timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else"},{"line_number":37,"context_line":"             kwargs.pop(\u0027inactivity_probe\u0027, None))"},{"line_number":38,"context_line":"    man_uri \u003d _connection_to_manager_uri(conn_uri)"},{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_3a3f11d7","line":37,"range":{"start_line":36,"start_character":13,"end_line":37,"end_character":49},"in_reply_to":"1f493fa4_9a56255c","updated":"2020-04-24 11:26:19.000000000","message":"But kwargs allows us to keep API, doesn\u0027t it? I meant change to:\n\nprobe \u003d kwargs.pop(\u0027inactivity_probe\u0027, timeout * 1000 if kwargs.get(\u0027set_timeout\u0027) else None)\n\nJust a nit anyway.","commit_id":"c2890c5be2e9f3c163729d862d85c205e3f80f85"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"66589a6af3e5824fbdcd1f810d426d80c2a7e35a","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"},{"line_number":41,"context_line":"           \u0027--\u0027, \u0027add\u0027, \u0027Open_vSwitch\u0027, \u0027.\u0027, \u0027manager_options\u0027, \u0027@manager\u0027]"},{"line_number":42,"context_line":"    if probe:"},{"line_number":43,"context_line":"        cmd +\u003d [\u0027--\u0027, \u0027set\u0027, \u0027Manager\u0027, man_uri, \u0027inactivity_probe\u003d%s\u0027 % probe]"},{"line_number":44,"context_line":"    if execute:"},{"line_number":45,"context_line":"        return execute(cmd, **kwargs).rstrip()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f493fa4_0d119354","line":42,"range":{"start_line":42,"start_character":7,"end_line":42,"end_character":12},"updated":"2020-05-04 07:26:40.000000000","message":"We have to check this to None otherwise we\u0027ll be never able to turn the probe off with the inactivity_probe parameter. From http://www.openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.txt\n\nA value of 0 disables inactivity probes.","commit_id":"94cf9f412e5ec98bf4ceeb72ffd5f6cae4213d3c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f1e4453db0d532060b688ad51b27fce794fc9371","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    cmd \u003d [\u0027ovs-vsctl\u0027, \u0027--timeout\u003d%d\u0027 % timeout, \u0027--id\u003d@manager\u0027,"},{"line_number":40,"context_line":"           \u0027--\u0027, \u0027create\u0027, \u0027Manager\u0027, \u0027target\u003d\"%s\"\u0027 % man_uri,"},{"line_number":41,"context_line":"           \u0027--\u0027, \u0027add\u0027, \u0027Open_vSwitch\u0027, \u0027.\u0027, \u0027manager_options\u0027, \u0027@manager\u0027]"},{"line_number":42,"context_line":"    if probe:"},{"line_number":43,"context_line":"        cmd +\u003d [\u0027--\u0027, \u0027set\u0027, \u0027Manager\u0027, man_uri, \u0027inactivity_probe\u003d%s\u0027 % probe]"},{"line_number":44,"context_line":"    if execute:"},{"line_number":45,"context_line":"        return execute(cmd, **kwargs).rstrip()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f493fa4_a72b582e","line":42,"range":{"start_line":42,"start_character":7,"end_line":42,"end_character":12},"in_reply_to":"1f493fa4_0d119354","updated":"2020-05-04 14:04:46.000000000","message":"Good catch!","commit_id":"94cf9f412e5ec98bf4ceeb72ffd5f6cae4213d3c"}]}