)]}'
{"networking_ovn/common/ovn_client.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"b7fc4ba2b3a28ebc8dc25b3a79fb1d9c1b7f2329","unresolved":false,"context_lines":[{"line_number":1268,"context_line":"            # host\u0027s DNS resolvers."},{"line_number":1269,"context_line":"            dns_nameservers \u003d config.get_dns_servers()"},{"line_number":1270,"context_line":"            if not dns_nameservers:"},{"line_number":1271,"context_line":"                dns_nameservers \u003d utils.get_system_dns_resolvers()"},{"line_number":1272,"context_line":"            dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join(dns_nameservers)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        if dns_servers:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_94636b1a","line":1271,"range":{"start_line":1271,"start_character":16,"end_line":1271,"end_character":31},"updated":"2018-05-30 09:24:50.000000000","message":"I\u0027d log a warning here to let the operator know that there\u0027s not going to be DNS if get_system_dns_resolvers returns []. What do you think?","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0b3017bcfa0806a13db698c11ff4bb325c06b45f","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"                dns_nameservers \u003d utils.get_system_dns_resolvers()"},{"line_number":1272,"context_line":"            dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join(dns_nameservers)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        if dns_servers:"},{"line_number":1275,"context_line":"            options[\u0027dns_server\u0027] \u003d dns_servers"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"        routes \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_caa7f84f","line":1274,"range":{"start_line":1274,"start_character":8,"end_line":1274,"end_character":23},"updated":"2018-05-30 10:58:02.000000000","message":"This will always be true because the if and else above sets it.\n\nIt can be changed to:\n\n if not dns_servers \u003d\u003d \u0027{}\u0027:\n    ...","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9e35baa976290973842dce69d074d3e6977fff39","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"                dns_nameservers \u003d utils.get_system_dns_resolvers()"},{"line_number":1272,"context_line":"            dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join(dns_nameservers)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        if dns_servers:"},{"line_number":1275,"context_line":"            options[\u0027dns_server\u0027] \u003d dns_servers"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"        routes \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_cdb39109","line":1274,"range":{"start_line":1274,"start_character":8,"end_line":1274,"end_character":23},"in_reply_to":"5f7c97a3_2d0a45d1","updated":"2018-05-30 12:33:29.000000000","message":"If you want to simplify it you can do:\n\ndns_servers \u003d (subnet.get(\u0027dns_nameservers\u0027) or\n               config.get_dns_servers() or\n               utils.get_system_dns_resolvers())\n\nif dns_servers:\n    options[\u0027dns_server\u0027] \u003d \u0027{%s}\u0027 % \u0027, \u0027,join(dns_servers)","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"83d0db9c5953f9024cbfdc4885a32f424ec6f5ee","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"                dns_nameservers \u003d utils.get_system_dns_resolvers()"},{"line_number":1272,"context_line":"            dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join(dns_nameservers)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        if dns_servers:"},{"line_number":1275,"context_line":"            options[\u0027dns_server\u0027] \u003d dns_servers"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"        routes \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_2d0a45d1","line":1274,"range":{"start_line":1274,"start_character":8,"end_line":1274,"end_character":23},"in_reply_to":"5f7c97a3_ca03780f","updated":"2018-05-30 12:28:16.000000000","message":"Even then, cause the value of dns_servers will be \"{}\" if the list is empty.\n\nIn [1]: dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join([])\n\nIn [2]: dns_servers\nOut[2]: \u0027{}\u0027\n\nIn [3]: if dns_servers:\n   ...:     print(\"hi\")\n   ...: \nhi","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"ce38bd31110fe6eb42e1d260cf5a45f9956ea6e1","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"                dns_nameservers \u003d utils.get_system_dns_resolvers()"},{"line_number":1272,"context_line":"            dns_servers \u003d \u0027{%s}\u0027 % \u0027, \u0027.join(dns_nameservers)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        if dns_servers:"},{"line_number":1275,"context_line":"            options[\u0027dns_server\u0027] \u003d dns_servers"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"        routes \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_ca03780f","line":1274,"range":{"start_line":1274,"start_character":8,"end_line":1274,"end_character":23},"in_reply_to":"5f7c97a3_caa7f84f","updated":"2018-05-30 11:01:59.000000000","message":"Not all the time. What if .get_system_dns_resolvers returns empty list.","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"}],"networking_ovn/common/utils.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0b3017bcfa0806a13db698c11ff4bb325c06b45f","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        pass"},{"line_number":346,"context_line":"        return resolvers"},{"line_number":347,"context_line":"    except IOError:"},{"line_number":348,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_2a6a2c19","line":348,"updated":"2018-05-30 10:58:02.000000000","message":"I think we can improve this function. Currently it can raise IndexErrors and exceptions is not being logged which can hide some other errors.\n\nWe can use a regex to extract the IP\u0027s, what about:\n\nimport re\n\ndef get_resolv():\n    resolvers \u003d []\n    rconf \u003d open(DNS_RESOLVER_FILE, \u0027r\u0027)\n    for line in rconf.readlines():\n        if not line.startswith(\u0027nameserver\u0027):\n            continue\n\n        line \u003d line.split(\u0027nameserver\u0027)[1].strip()\n        ipv4 \u003d re.search(r\u0027^(?:[0-9]{1,3}\\.){3}[0-9]{1,3}\u0027, line)\n        if ipv4:\n            resolvers.append(ipv4.group(0))\n    return resolvers","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"597b9fb42cc204bfaa460edc23db48bb243c356a","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        pass"},{"line_number":346,"context_line":"        return resolvers"},{"line_number":347,"context_line":"    except IOError:"},{"line_number":348,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_50571a27","line":348,"in_reply_to":"5f7c97a3_2a6a2c19","updated":"2018-05-30 18:24:32.000000000","message":"\u003e I think we can improve this function. Currently it can raise\n \u003e IndexErrors and exceptions is not being logged which can hide some\n \u003e other errors.\n \u003e \n \u003e We can use a regex to extract the IP\u0027s, what about:\n \u003e \n \u003e import re\n \u003e \n \u003e def get_resolv():\n \u003e resolvers \u003d []\n \u003e rconf \u003d open(DNS_RESOLVER_FILE, \u0027r\u0027)\n \u003e for line in rconf.readlines():\n \u003e if not line.startswith(\u0027nameserver\u0027):\n \u003e continue\n \u003e \n \u003e line \u003d line.split(\u0027nameserver\u0027)[1].strip()\n \u003e ipv4 \u003d re.search(r\u0027^(?:[0-9]{1,3}\\.){3}[0-9]{1,3}\u0027, line)\n \u003e if ipv4:\n \u003e resolvers.append(ipv4.group(0))\n \u003e return resolvers","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"ce38bd31110fe6eb42e1d260cf5a45f9956ea6e1","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        pass"},{"line_number":346,"context_line":"        return resolvers"},{"line_number":347,"context_line":"    except IOError:"},{"line_number":348,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_25101f4e","line":348,"in_reply_to":"5f7c97a3_2a6a2c19","updated":"2018-05-30 11:01:59.000000000","message":"Wow. thanks for improving.","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"62df3256b22706eb32b95dd7571b257088e40810","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        pass"},{"line_number":346,"context_line":"        return resolvers"},{"line_number":347,"context_line":"    except IOError:"},{"line_number":348,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_420bc88c","line":348,"in_reply_to":"5f7c97a3_50571a27","updated":"2018-05-30 18:25:27.000000000","message":"Ignore this comment :)","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c4024a1f4c985dbc327224cd1d70673e6085281d","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def get_system_dns_resolvers():"},{"line_number":335,"context_line":"    resolvers \u003d []"},{"line_number":336,"context_line":"    rconf \u003d open(DNS_RESOLVER_FILE, \u0027r\u0027)"},{"line_number":337,"context_line":"    for line in rconf.readlines():"},{"line_number":338,"context_line":"        if not line.startswith(\u0027nameserver\u0027):"},{"line_number":339,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_798f9a74","line":336,"range":{"start_line":336,"start_character":0,"end_line":336,"end_character":40},"updated":"2018-05-30 20:07:32.000000000","message":"Sorry it\u0027s my fault.\n\nI looked at my previous comment and my example didn\u0027t call close() for rconf at the end of the function. Using a context manager would be more pythonic than open() and close() btw, e.g:\n\nresolvers \u003d []\nwith open(DNS_RESOLVER_FILE, \u0027r\u0027) as rconf:\n    for line in rconf.readlines():\n        ...\n\n(sorry for that!)","commit_id":"4908158b4a278cea61317334f80dbfb77cc947da"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"50dfc99720e6c50612a40fcfea5c7dd60792983a","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def get_system_dns_resolvers():"},{"line_number":335,"context_line":"    resolvers \u003d []"},{"line_number":336,"context_line":"    with open(DNS_RESOLVER_FILE, \u0027r\u0027) as rconf:"},{"line_number":337,"context_line":"        for line in rconf.readlines():"},{"line_number":338,"context_line":"            if not line.startswith(\u0027nameserver\u0027):"},{"line_number":339,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_15197e20","line":336,"updated":"2018-05-31 12:21:30.000000000","message":"Interesting failure here [0].\n\nShould we account for situations which the /etc/resolv.conf does not exist ? Perhaps:\n\n if not os.path.exists(DNS_RESOLVER_FILE):\n     return []\n\n[0] http://logs.openstack.org/06/571006/4/check/openstack-tox-py27/28301f8/job-output.txt.gz#_2018-05-31_08_52_18_289156","commit_id":"5460a538f02e1a648f3e72b872bf1a4767ed867c"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"152e071941fafb7bf85974a0b4cae12d6c737eee","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def get_system_dns_resolvers():"},{"line_number":335,"context_line":"    resolvers \u003d []"},{"line_number":336,"context_line":"    with open(DNS_RESOLVER_FILE, \u0027r\u0027) as rconf:"},{"line_number":337,"context_line":"        for line in rconf.readlines():"},{"line_number":338,"context_line":"            if not line.startswith(\u0027nameserver\u0027):"},{"line_number":339,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_a79f3b4e","line":336,"in_reply_to":"5f7c97a3_15197e20","updated":"2018-05-31 14:59:05.000000000","message":"It\u0027s trying to open \u0027/tmp/tmpoAMMpJ/tmpaU18o7/resolv.conf\u0027\n\nhm?","commit_id":"5460a538f02e1a648f3e72b872bf1a4767ed867c"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97a79a440aa959635514fcb2feadd093a8bcc45","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def get_system_dns_resolvers():"},{"line_number":335,"context_line":"    resolvers \u003d []"},{"line_number":336,"context_line":"    with open(DNS_RESOLVER_FILE, \u0027r\u0027) as rconf:"},{"line_number":337,"context_line":"        for line in rconf.readlines():"},{"line_number":338,"context_line":"            if not line.startswith(\u0027nameserver\u0027):"},{"line_number":339,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_474d07c2","line":336,"in_reply_to":"5f7c97a3_a79f3b4e","updated":"2018-05-31 15:08:18.000000000","message":"Good point, the unittest is leaking because it\u0027s modifying a module variable.\n\nI\u0027ve added another comment to alert @Numan about it.","commit_id":"5460a538f02e1a648f3e72b872bf1a4767ed867c"}],"networking_ovn/tests/unit/common/test_utils.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"b7fc4ba2b3a28ebc8dc25b3a79fb1d9c1b7f2329","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":28,"context_line":"        utils.DNS_RESOLVER_FILE \u003d \u0027/tmp/resolv.conf\u0027"},{"line_number":29,"context_line":"        contents \u003d \"\"\"# TEST TEST TEST"},{"line_number":30,"context_line":"# Geneated by OVN test"},{"line_number":31,"context_line":"nameserver 10.0.0.1"},{"line_number":32,"context_line":"#nameserver 10.0.0.2"},{"line_number":33,"context_line":"nameserver 10.0.0.3"},{"line_number":34,"context_line":"nameserver foo 10.0.0.4"},{"line_number":35,"context_line":"nameserver aef0::4"},{"line_number":36,"context_line":"foo 10.0.0.5"},{"line_number":37,"context_line":"\"\"\""},{"line_number":38,"context_line":"        tmp_resolv_file \u003d open(\u0027/tmp/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":39,"context_line":"        tmp_resolv_file.writelines(contents)"},{"line_number":40,"context_line":"        tmp_resolv_file.close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_d42a438f","line":37,"range":{"start_line":30,"start_character":0,"end_line":37,"end_character":3},"updated":"2018-05-30 09:24:50.000000000","message":"nit: This is a constant, I would define it as such up there as:\n\nRESOLV_CONF_TEMPLATE \u003d \"\"\"\n# TEST TEST TEST\n# Geneated by OVN test\nnameserver 10.0.0.1\n#nameserver 10.0.0.2\nnameserver 10.0.0.3\nnameserver foo 10.0.0.4\nnameserver aef0::4\nfoo 10.0.0.5\n\"\"\"","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"597b9fb42cc204bfaa460edc23db48bb243c356a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":28,"context_line":"        utils.DNS_RESOLVER_FILE \u003d \u0027/tmp/resolv.conf\u0027"},{"line_number":29,"context_line":"        contents \u003d \"\"\"# TEST TEST TEST"},{"line_number":30,"context_line":"# Geneated by OVN test"},{"line_number":31,"context_line":"nameserver 10.0.0.1"},{"line_number":32,"context_line":"#nameserver 10.0.0.2"},{"line_number":33,"context_line":"nameserver 10.0.0.3"},{"line_number":34,"context_line":"nameserver foo 10.0.0.4"},{"line_number":35,"context_line":"nameserver aef0::4"},{"line_number":36,"context_line":"foo 10.0.0.5"},{"line_number":37,"context_line":"\"\"\""},{"line_number":38,"context_line":"        tmp_resolv_file \u003d open(\u0027/tmp/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":39,"context_line":"        tmp_resolv_file.writelines(contents)"},{"line_number":40,"context_line":"        tmp_resolv_file.close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_c2cd985d","line":37,"range":{"start_line":30,"start_character":0,"end_line":37,"end_character":3},"in_reply_to":"5f7c97a3_d42a438f","updated":"2018-05-30 18:24:32.000000000","message":"\u003e nit: This is a constant, I would define it as such up there as:\n \u003e \n \u003e RESOLV_CONF_TEMPLATE \u003d \"\"\"\n \u003e # TEST TEST TEST\n \u003e # Geneated by OVN test\n \u003e nameserver 10.0.0.1\n \u003e #nameserver 10.0.0.2\n \u003e nameserver 10.0.0.3\n \u003e nameserver foo 10.0.0.4\n \u003e nameserver aef0::4\n \u003e foo 10.0.0.5\n \u003e \"\"\"","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"ce38bd31110fe6eb42e1d260cf5a45f9956ea6e1","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":28,"context_line":"        utils.DNS_RESOLVER_FILE \u003d \u0027/tmp/resolv.conf\u0027"},{"line_number":29,"context_line":"        contents \u003d \"\"\"# TEST TEST TEST"},{"line_number":30,"context_line":"# Geneated by OVN test"},{"line_number":31,"context_line":"nameserver 10.0.0.1"},{"line_number":32,"context_line":"#nameserver 10.0.0.2"},{"line_number":33,"context_line":"nameserver 10.0.0.3"},{"line_number":34,"context_line":"nameserver foo 10.0.0.4"},{"line_number":35,"context_line":"nameserver aef0::4"},{"line_number":36,"context_line":"foo 10.0.0.5"},{"line_number":37,"context_line":"\"\"\""},{"line_number":38,"context_line":"        tmp_resolv_file \u003d open(\u0027/tmp/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":39,"context_line":"        tmp_resolv_file.writelines(contents)"},{"line_number":40,"context_line":"        tmp_resolv_file.close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_cf8e2ad0","line":37,"range":{"start_line":30,"start_character":0,"end_line":37,"end_character":3},"in_reply_to":"5f7c97a3_d42a438f","updated":"2018-05-30 11:01:59.000000000","message":"Ack","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"84c889b9baa29e21f732f1b65f94c2c59c07f614","unresolved":false,"context_lines":[{"line_number":35,"context_line":"nameserver aef0::4"},{"line_number":36,"context_line":"foo 10.0.0.5"},{"line_number":37,"context_line":"\"\"\""},{"line_number":38,"context_line":"        tmp_resolv_file \u003d open(\u0027/tmp/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":39,"context_line":"        tmp_resolv_file.writelines(contents)"},{"line_number":40,"context_line":"        tmp_resolv_file.close()"},{"line_number":41,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_3e8a5f63","line":38,"range":{"start_line":38,"start_character":31,"end_line":38,"end_character":49},"updated":"2018-05-31 06:45:51.000000000","message":"What do you think about:\n\ntmp_resolv_filename \u003d os.path.join(os.path.dirname(__file__), \u0027resolv.conf\u0027)\n\nAnd when removing the file later you can still use \u0027tmp_resolv_filename\u0027?","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"b7fc4ba2b3a28ebc8dc25b3a79fb1d9c1b7f2329","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        tmp_resolv_file.close()"},{"line_number":41,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"},{"line_number":42,"context_line":"        observed_dns_resolvers \u003d utils.get_system_dns_resolvers()"},{"line_number":43,"context_line":"        os.remove(\u0027/tmp/resolv.conf\u0027)"},{"line_number":44,"context_line":"        self.assertEqual(expected_dns_resolvers, observed_dns_resolvers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_af2dee95","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":17},"updated":"2018-05-30 09:24:50.000000000","message":"Can we add this to the cleanup of the test? it\u0027s not super critical as it\u0027s in the tmp but it\u0027s better, what do you think?","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"597b9fb42cc204bfaa460edc23db48bb243c356a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        tmp_resolv_file.close()"},{"line_number":41,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"},{"line_number":42,"context_line":"        observed_dns_resolvers \u003d utils.get_system_dns_resolvers()"},{"line_number":43,"context_line":"        os.remove(\u0027/tmp/resolv.conf\u0027)"},{"line_number":44,"context_line":"        self.assertEqual(expected_dns_resolvers, observed_dns_resolvers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_a2d2dcfe","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":17},"in_reply_to":"5f7c97a3_0f8982c8","updated":"2018-05-30 18:24:32.000000000","message":"I will leave AS IS for now. Because if we add more tests in this file, it doesn\u0027t make sense for the cleanup to delete this file as this file is created only in this test case.","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"84c889b9baa29e21f732f1b65f94c2c59c07f614","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        tmp_resolv_file.close()"},{"line_number":41,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"},{"line_number":42,"context_line":"        observed_dns_resolvers \u003d utils.get_system_dns_resolvers()"},{"line_number":43,"context_line":"        os.remove(\u0027/tmp/resolv.conf\u0027)"},{"line_number":44,"context_line":"        self.assertEqual(expected_dns_resolvers, observed_dns_resolvers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_1e48c359","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":17},"in_reply_to":"5f7c97a3_a2d2dcfe","updated":"2018-05-31 06:45:51.000000000","message":"def test_get_system_dns_resolvers(self):\n    self.addCleanup(os.remove, RESOLVER_FILE)\n    ... your code goes here","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"ce38bd31110fe6eb42e1d260cf5a45f9956ea6e1","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        tmp_resolv_file.close()"},{"line_number":41,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"},{"line_number":42,"context_line":"        observed_dns_resolvers \u003d utils.get_system_dns_resolvers()"},{"line_number":43,"context_line":"        os.remove(\u0027/tmp/resolv.conf\u0027)"},{"line_number":44,"context_line":"        self.assertEqual(expected_dns_resolvers, observed_dns_resolvers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_0f8982c8","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":17},"in_reply_to":"5f7c97a3_af2dee95","updated":"2018-05-30 11:01:59.000000000","message":"Makes sense","commit_id":"e457e52cd941f8494f6e707dd42db1234a334455"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c4024a1f4c985dbc327224cd1d70673e6085281d","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class TestUtils(base.TestCase):"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setUp(self):"},{"line_number":35,"context_line":"        super(TestUtils, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":38,"context_line":"        utils.DNS_RESOLVER_FILE \u003d \u0027/tmp/resolv.conf\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_d9c26e85","line":35,"range":{"start_line":34,"start_character":0,"end_line":35,"end_character":38},"updated":"2018-05-30 20:07:32.000000000","message":"nit: Overwriting this method is not needed if we won\u0027t add anything to it.","commit_id":"4908158b4a278cea61317334f80dbfb77cc947da"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0fb96d0187d9bb20877f63ec766f63275f2153e5","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        tmp_resolv_file.close()"},{"line_number":39,"context_line":"        expected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]"},{"line_number":40,"context_line":"        observed_dns_resolvers \u003d utils.get_system_dns_resolvers()"},{"line_number":41,"context_line":"        os.remove(\u0027/tmp/resolv.conf\u0027)"},{"line_number":42,"context_line":"        self.assertEqual(expected_dns_resolvers, observed_dns_resolvers)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_01bdc4b5","line":41,"range":{"start_line":41,"start_character":0,"end_line":41,"end_character":37},"updated":"2018-05-31 08:15:59.000000000","message":"Following daniel\u0027s suggestion, perhaps we don\u0027t even need to add this to the clean up, we can create a temporary file and it will be removed automatically\n\ntf \u003d tempfile.NamedTemporaryFile(delete\u003dTrue)\ntf.writelines(RESOLV_CONF_TEMPLATE)\ntf.flush()\nutils.DNS_RESOLVER\u003eFILE \u003d tf.name\nexpected_dns_resolvers \u003d [\u002710.0.0.1\u0027, \u002710.0.0.3\u0027]\nobserved_dns_resolvers \u003d utils.get_system_dns_resolvers()\ntf.close()\nself.assertEqual(expected_dns_resolvers, observed_dns_resolvers)","commit_id":"5e78275465a86ef8ad0d412daedb444e579cbcac"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97a79a440aa959635514fcb2feadd093a8bcc45","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":37,"context_line":"        tempdir \u003d self.useFixture(fixtures.TempDir()).path"},{"line_number":38,"context_line":"        utils.DNS_RESOLVER_FILE \u003d tempdir + \u0027/resolv.conf\u0027"},{"line_number":39,"context_line":"        tmp_resolv_file \u003d open(tempdir + \u0027/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":40,"context_line":"        tmp_resolv_file.writelines(RESOLV_CONF_TEMPLATE)"},{"line_number":41,"context_line":"        tmp_resolv_file.close()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_e75c537e","line":38,"range":{"start_line":38,"start_character":0,"end_line":38,"end_character":58},"updated":"2018-05-31 15:08:18.000000000","message":"This is leaking. Since we are changing a global variable other tests that running after this test is executed will see the fixture value for this variable. See [0].\n\nI think we could mock it as:\n\nwith patch(\u0027utils.DNS_RESOLVER_FILE\u0027, tempdir + \u0027/resolvconf\u0027)::\n    ...\n\n[0] http://logs.openstack.org/06/571006/4/check/openstack-tox-py27/28301f8/job-output.txt.gz#_2018-05-31_08_52_18_289156","commit_id":"5460a538f02e1a648f3e72b872bf1a4767ed867c"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"27e3de6f353f6d24bd4cb1dfd6d3485048928c4a","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def test_get_system_dns_resolvers(self):"},{"line_number":37,"context_line":"        tempdir \u003d self.useFixture(fixtures.TempDir()).path"},{"line_number":38,"context_line":"        utils.DNS_RESOLVER_FILE \u003d tempdir + \u0027/resolv.conf\u0027"},{"line_number":39,"context_line":"        tmp_resolv_file \u003d open(tempdir + \u0027/resolv.conf\u0027, \u0027w\u0027)"},{"line_number":40,"context_line":"        tmp_resolv_file.writelines(RESOLV_CONF_TEMPLATE)"},{"line_number":41,"context_line":"        tmp_resolv_file.close()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_2e3300f8","line":38,"range":{"start_line":38,"start_character":0,"end_line":38,"end_character":58},"in_reply_to":"5f7c97a3_e75c537e","updated":"2018-05-31 19:18:48.000000000","message":"See p.s 5. For some reason I couldn\u0027t make it work. So I found another approach which I think should be fine :)","commit_id":"5460a538f02e1a648f3e72b872bf1a4767ed867c"}]}