)]}'
{"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bb383a4a13889552f619a70d63df5761a2683790","unresolved":false,"context_lines":[{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    output \u003d list_network_namespaces(**kwargs)"},{"line_number":999,"context_line":"    exists \u003d namespace in output"},{"line_number":1000,"context_line":"    if not try_is_ready and exists:"},{"line_number":1001,"context_line":"        return True"},{"line_number":1002,"context_line":"    elif not exists:"},{"line_number":1003,"context_line":"        return False"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"    try:"},{"line_number":1006,"context_line":"        privileged.open_namespace(namespace)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_75258e08","line":1003,"range":{"start_line":1000,"start_character":0,"end_line":1003,"end_character":20},"updated":"2019-03-26 11:56:14.000000000","message":"You can do it like:\n\n    if not try_is_ready:\n        return exists","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9acb3260570f53e49b47b36330a0df5871313f1f","unresolved":false,"context_lines":[{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    output \u003d list_network_namespaces(**kwargs)"},{"line_number":999,"context_line":"    exists \u003d namespace in output"},{"line_number":1000,"context_line":"    if not try_is_ready and exists:"},{"line_number":1001,"context_line":"        return True"},{"line_number":1002,"context_line":"    elif not exists:"},{"line_number":1003,"context_line":"        return False"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"    try:"},{"line_number":1006,"context_line":"        privileged.open_namespace(namespace)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_b5ebf675","line":1003,"range":{"start_line":1000,"start_character":0,"end_line":1003,"end_character":20},"in_reply_to":"5fc1f717_75258e08","updated":"2019-03-26 12:02:00.000000000","message":"Uffff I need to review again to my boolean algebra classes","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"e4d4fd0eeb72535f5954ebc77e53d0543eda0793","unresolved":false,"context_lines":[{"line_number":999,"context_line":"    exists \u003d namespace in output"},{"line_number":1000,"context_line":"    if not try_is_ready:"},{"line_number":1001,"context_line":"        return exists"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"    try:"},{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_94002504","line":1002,"updated":"2019-03-27 09:00:02.000000000","message":"Do we need to try open_namespace when exists is False?\n\nThe current logic looks tricky.\nIf try_is_ready is True, L.998-1001 is completely unnecessary.\nIt looks better to have a separate function if we keep the current proposed logic.","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"703636a55ef67e8e07c51a45690ed1a1d1c309e5","unresolved":false,"context_lines":[{"line_number":999,"context_line":"    exists \u003d namespace in output"},{"line_number":1000,"context_line":"    if not try_is_ready:"},{"line_number":1001,"context_line":"        return exists"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"    try:"},{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_8de2549f","line":1002,"in_reply_to":"5fc1f717_94002504","updated":"2019-03-27 10:53:16.000000000","message":"I agree with the idea of just try to open the ns if try_is_ready\u003dTrue.\n\nI don\u0027t think we need a separate function. With this modification we don\u0027t need to modify the logic of the current code and tests.","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"d03ebc31d0c0ee65a80bd9f63da92ebdacd43193","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"    try:"},{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"},{"line_number":1006,"context_line":"    except:"},{"line_number":1007,"context_line":"        return False"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_a9295b37","line":1006,"updated":"2019-03-26 14:37:01.000000000","message":"Pretty sure pep8 is going to complain about a bare \u0027except\u0027; it should be at least \u0027except Exception\u0027 and should be as specific as possible.","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4b28c9327f04d589d6706dd294bf408ba541a571","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"    try:"},{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"},{"line_number":1006,"context_line":"    except:"},{"line_number":1007,"context_line":"        return False"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_b569a825","line":1006,"in_reply_to":"5fc1f717_a9295b37","updated":"2019-03-26 20:48:48.000000000","message":"That\u0027s right. About the type of exception, if when opening a namespace we have any exception, we return False.","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0728e194cbf6041421406376f820bc199b87ad0e","unresolved":false,"context_lines":[{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"},{"line_number":1006,"context_line":"    except:"},{"line_number":1007,"context_line":"        return False"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":""},{"line_number":1010,"context_line":"def ensure_device_is_ready(device_name, namespace\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_a9e87bb2","line":1007,"updated":"2019-03-26 14:45:24.000000000","message":"This should be \u0027pass\u0027 and the return False should be at a lower indentation, otherwise this will return None\n\nYou should also have a test for this case of passing try_is_ready\u003dTrue","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4b28c9327f04d589d6706dd294bf408ba541a571","unresolved":false,"context_lines":[{"line_number":1004,"context_line":"        privileged.open_namespace(namespace)"},{"line_number":1005,"context_line":"        return True"},{"line_number":1006,"context_line":"    except:"},{"line_number":1007,"context_line":"        return False"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":""},{"line_number":1010,"context_line":"def ensure_device_is_ready(device_name, namespace\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_9557cc58","line":1007,"in_reply_to":"5fc1f717_a9e87bb2","updated":"2019-03-26 20:48:48.000000000","message":"Yes and yes. I should skip it and the return False. And I need to add a test for this.","commit_id":"b511992900d00f5f0df42ca92f13934b00de0734"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"afb8afc0c226b665fee0c52b2807b0bbb206129f","unresolved":false,"context_lines":[{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    if not try_is_ready:"},{"line_number":999,"context_line":"        output \u003d list_network_namespaces(**kwargs)"},{"line_number":1000,"context_line":"        return namespace in output"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"    try:"},{"line_number":1003,"context_line":"        privileged.open_namespace(namespace)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_02005caa","line":1000,"updated":"2019-03-28 08:06:19.000000000","message":"hmm, now I start thinking if we really need this part check at all? Maybe we should always test if namespace is ready as this should be most interesting for us always, right?","commit_id":"434b29a2cb131d38ed151e6a1d7f1ab756408198"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9135ef6baad99182e17be6025d1864e91f92ccbb","unresolved":false,"context_lines":[{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    if not try_is_ready:"},{"line_number":999,"context_line":"        output \u003d list_network_namespaces(**kwargs)"},{"line_number":1000,"context_line":"        return namespace in output"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"    try:"},{"line_number":1003,"context_line":"        privileged.open_namespace(namespace)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_c84d939a","line":1000,"in_reply_to":"5fc1f717_02005caa","updated":"2019-03-28 10:42:56.000000000","message":"This is more expensive and we don\u0027t always need to check if the ns is ready to be opened.","commit_id":"434b29a2cb131d38ed151e6a1d7f1ab756408198"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"3e1a730b74596df3bc0ef0c4a7a4d6b4f1e9dbdf","unresolved":false,"context_lines":[{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    if not try_is_ready:"},{"line_number":999,"context_line":"        output \u003d list_network_namespaces(**kwargs)"},{"line_number":1000,"context_line":"        return namespace in output"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"    try:"},{"line_number":1003,"context_line":"        privileged.open_namespace(namespace)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_f3e164d1","line":1000,"in_reply_to":"5fc1f717_c84d939a","updated":"2019-03-28 11:18:18.000000000","message":"ok, if it is more expensive than that makes sense :)","commit_id":"434b29a2cb131d38ed151e6a1d7f1ab756408198"}],"neutron/privileged/agent/linux/ip_lib.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bb383a4a13889552f619a70d63df5761a2683790","unresolved":false,"context_lines":[{"line_number":180,"context_line":"# NOTE(slaweq): Because of issue with pyroute2.NetNS objects running in threads"},{"line_number":181,"context_line":"# we need to lock this function to workaround this issue."},{"line_number":182,"context_line":"# For details please check https://bugs.launchpad.net/neutron/+bug/1811515"},{"line_number":183,"context_line":"@lockutils.synchronized(\"privileged-ip-lib\")"},{"line_number":184,"context_line":"def open_namespace(namespace):"},{"line_number":185,"context_line":"    \"\"\"Open namespace to test if the namespace is ready to be manipulated\"\"\""},{"line_number":186,"context_line":"    with pyroute2.NetNS(namespace, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_351b86ca","line":183,"updated":"2019-03-26 11:56:14.000000000","message":"do we still need this lock? I think that with Your fix in pyroute2 it shouldn\u0027t be necessary, right?","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9acb3260570f53e49b47b36330a0df5871313f1f","unresolved":false,"context_lines":[{"line_number":180,"context_line":"# NOTE(slaweq): Because of issue with pyroute2.NetNS objects running in threads"},{"line_number":181,"context_line":"# we need to lock this function to workaround this issue."},{"line_number":182,"context_line":"# For details please check https://bugs.launchpad.net/neutron/+bug/1811515"},{"line_number":183,"context_line":"@lockutils.synchronized(\"privileged-ip-lib\")"},{"line_number":184,"context_line":"def open_namespace(namespace):"},{"line_number":185,"context_line":"    \"\"\"Open namespace to test if the namespace is ready to be manipulated\"\"\""},{"line_number":186,"context_line":"    with pyroute2.NetNS(namespace, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_358ce6d5","line":183,"in_reply_to":"5fc1f717_351b86ca","updated":"2019-03-26 12:02:00.000000000","message":"You are right and we need to check if this problem was solved.\n\nIn this function is not needed but I\u0027ll remove the lock for the other functions in other commit.","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"}],"neutron/tests/fullstack/test_dhcp_agent.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bb383a4a13889552f619a70d63df5761a2683790","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _assert_namespace_exists(self, ns_name):"},{"line_number":89,"context_line":"        common_utils.wait_until_true("},{"line_number":90,"context_line":"            lambda: ip_lib.network_namespace_exists(ns_name,"},{"line_number":91,"context_line":"                                                    try_is_ready\u003dTrue))"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def test_dhcp_assignment(self):"},{"line_number":94,"context_line":"        # First check if network was scheduled to one DHCP agent"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_d562423e","line":91,"updated":"2019-03-26 11:56:14.000000000","message":"this can be IMO moved to base fullstack class as it may be possibly used in other tests too","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9acb3260570f53e49b47b36330a0df5871313f1f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _assert_namespace_exists(self, ns_name):"},{"line_number":89,"context_line":"        common_utils.wait_until_true("},{"line_number":90,"context_line":"            lambda: ip_lib.network_namespace_exists(ns_name,"},{"line_number":91,"context_line":"                                                    try_is_ready\u003dTrue))"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def test_dhcp_assignment(self):"},{"line_number":94,"context_line":"        # First check if network was scheduled to one DHCP agent"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_b5b9b679","line":91,"in_reply_to":"5fc1f717_d562423e","updated":"2019-03-26 12:02:00.000000000","message":"You are right. And the extra test (try_is_ready) could help other tests.\n\nDone!","commit_id":"ad45b4860855b77c2a5399511561feeb857bb04e"}]}