)]}'
{"neutron/tests/unit/agent/test_securitygroups_rpc.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":476,"context_line":"                        {\u0027direction\u0027: \u0027egress\u0027, \u0027ethertype\u0027: const.IPv6,"},{"line_number":477,"context_line":"                         \u0027security_group_id\u0027: sg2_id},"},{"line_number":478,"context_line":"                        {\u0027direction\u0027: u\u0027ingress\u0027,"},{"line_number":479,"context_line":"                         \u0027source_ip_prefix\u0027: port_fixed_ip2 + u\u0027/32\u0027,"},{"line_number":480,"context_line":"                         \u0027protocol\u0027: const.PROTO_NAME_TCP,"},{"line_number":481,"context_line":"                         \u0027ethertype\u0027: const.IPv4,"},{"line_number":482,"context_line":"                         \u0027port_range_max\u0027: 25, \u0027port_range_min\u0027: 24,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_38c67348","line":479,"range":{"start_line":479,"start_character":62,"end_line":479,"end_character":63},"updated":"2016-04-13 19:42:30.000000000","message":"Curious why you\u0027re using a unicode literal here.  Any advantage?","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":476,"context_line":"                        {\u0027direction\u0027: \u0027egress\u0027, \u0027ethertype\u0027: const.IPv6,"},{"line_number":477,"context_line":"                         \u0027security_group_id\u0027: sg2_id},"},{"line_number":478,"context_line":"                        {\u0027direction\u0027: u\u0027ingress\u0027,"},{"line_number":479,"context_line":"                         \u0027source_ip_prefix\u0027: port_fixed_ip2 + u\u0027/32\u0027,"},{"line_number":480,"context_line":"                         \u0027protocol\u0027: const.PROTO_NAME_TCP,"},{"line_number":481,"context_line":"                         \u0027ethertype\u0027: const.IPv4,"},{"line_number":482,"context_line":"                         \u0027port_range_max\u0027: 25, \u0027port_range_min\u0027: 24,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_a9b7d384","line":479,"range":{"start_line":479,"start_character":62,"end_line":479,"end_character":63},"in_reply_to":"5a18252c_38c67348","updated":"2016-04-13 20:31:17.000000000","message":"Oops, that\u0027s just an artifact of debugging why this was failing with the grenade I put in the IPAM code while identifying tests that needed work.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"}],"neutron/tests/unit/db/test_db_base_plugin_v2.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":921,"context_line":"            with self.subnet(network\u003dnetwork, cidr\u003d\u002710.0.0.0/24\u0027) as subnet:"},{"line_number":922,"context_line":"                keys \u003d [(\u0027admin_state_up\u0027, True),"},{"line_number":923,"context_line":"                        (\u0027status\u0027, self.port_create_status),"},{"line_number":924,"context_line":"                        (\u0027fixed_ips\u0027, [{\u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027],"},{"line_number":925,"context_line":"                                        \u0027ip_address\u0027: \u002710.0.0.2\u0027}])]"},{"line_number":926,"context_line":"                port_res \u003d self._create_port(self.fmt,"},{"line_number":927,"context_line":"                                             network[\u0027network\u0027][\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_2378042a","side":"PARENT","line":924,"range":{"start_line":924,"start_character":26,"end_line":924,"end_character":35},"updated":"2016-04-13 19:42:30.000000000","message":"This seems to change what behavior is being tested in another way.  It is like you swapped this test with test_create_port_json.","commit_id":"f65185ff8f115b450820714f20a6277a36a600c5"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":921,"context_line":"            with self.subnet(network\u003dnetwork, cidr\u003d\u002710.0.0.0/24\u0027) as subnet:"},{"line_number":922,"context_line":"                keys \u003d [(\u0027admin_state_up\u0027, True),"},{"line_number":923,"context_line":"                        (\u0027status\u0027, self.port_create_status),"},{"line_number":924,"context_line":"                        (\u0027fixed_ips\u0027, [{\u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027],"},{"line_number":925,"context_line":"                                        \u0027ip_address\u0027: \u002710.0.0.2\u0027}])]"},{"line_number":926,"context_line":"                port_res \u003d self._create_port(self.fmt,"},{"line_number":927,"context_line":"                                             network[\u0027network\u0027][\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_c4503661","side":"PARENT","line":924,"range":{"start_line":924,"start_character":26,"end_line":924,"end_character":35},"in_reply_to":"5a18252c_2378042a","updated":"2016-04-13 20:31:17.000000000","message":"Right, either way we get coverage on both \"any\" and \"specific\" allocations. However, you pointing this out has me re-evaluating what the least invasive change is. In this case we probably want a \"specific\" allocation.","commit_id":"f65185ff8f115b450820714f20a6277a36a600c5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"class TestPortsV2(NeutronDbPluginV2TestCase):"},{"line_number":872,"context_line":"    def test_create_port_json(self):"},{"line_number":873,"context_line":"        port_keys \u003d [(\u0027admin_state_up\u0027, True),"},{"line_number":874,"context_line":"                     (\u0027status\u0027, self.port_create_status)]"},{"line_number":875,"context_line":"        with self.network(shared\u003dTrue) as network:"},{"line_number":876,"context_line":"            with self.subnet(network\u003dnetwork) as subnet:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_a359b400","line":873,"range":{"start_line":873,"start_character":8,"end_line":873,"end_character":18},"updated":"2016-04-13 19:42:30.000000000","message":"nit:  This rename is strictly not necessary.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"class TestPortsV2(NeutronDbPluginV2TestCase):"},{"line_number":872,"context_line":"    def test_create_port_json(self):"},{"line_number":873,"context_line":"        port_keys \u003d [(\u0027admin_state_up\u0027, True),"},{"line_number":874,"context_line":"                     (\u0027status\u0027, self.port_create_status)]"},{"line_number":875,"context_line":"        with self.network(shared\u003dTrue) as network:"},{"line_number":876,"context_line":"            with self.subnet(network\u003dnetwork) as subnet:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_18ca926d","line":873,"range":{"start_line":873,"start_character":8,"end_line":873,"end_character":18},"in_reply_to":"5a18252c_a359b400","updated":"2016-04-13 20:31:17.000000000","message":"Done","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":874,"context_line":"                     (\u0027status\u0027, self.port_create_status)]"},{"line_number":875,"context_line":"        with self.network(shared\u003dTrue) as network:"},{"line_number":876,"context_line":"            with self.subnet(network\u003dnetwork) as subnet:"},{"line_number":877,"context_line":"                fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027,"},{"line_number":878,"context_line":"                                  \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":879,"context_line":"                with self.port(subnet\u003dsubnet, name\u003d\u0027myname\u0027,"},{"line_number":880,"context_line":"                               fixed_ips\u003dfixed_ip_data) as port:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_83e11801","line":877,"range":{"start_line":877,"start_character":16,"end_line":877,"end_character":29},"updated":"2016-04-13 19:42:30.000000000","message":"Doesn\u0027t this kind of change what behavior is being tested?","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":874,"context_line":"                     (\u0027status\u0027, self.port_create_status)]"},{"line_number":875,"context_line":"        with self.network(shared\u003dTrue) as network:"},{"line_number":876,"context_line":"            with self.subnet(network\u003dnetwork) as subnet:"},{"line_number":877,"context_line":"                fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027,"},{"line_number":878,"context_line":"                                  \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":879,"context_line":"                with self.port(subnet\u003dsubnet, name\u003d\u0027myname\u0027,"},{"line_number":880,"context_line":"                               fixed_ips\u003dfixed_ip_data) as port:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_04c18e2e","line":877,"range":{"start_line":877,"start_character":16,"end_line":877,"end_character":29},"in_reply_to":"5a18252c_83e11801","updated":"2016-04-13 20:31:17.000000000","message":"Yes, but I contend it doesn\u0027t really matter as a change to what you\u0027re testing needs to change no matter what. This gives us coverage on \"specific\" allocations (coverage we get in many other tests), but you could also perform an \"any\" allocation and get coverage on that as you point out on L886.\n\nIMO, the assertion on L880 in BASE is a poor one. In fact, I believe to be such a bad assertion that it requires the semantics of the test to be changed one way or another. As written in BASE, L880 is ambiguous given how the port was being created. So, my contention is that we can go either way with this.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":883,"context_line":"                    self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":884,"context_line":"                    ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":885,"context_line":"                    self.assertEqual(1, len(ips))"},{"line_number":886,"context_line":"                    self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":887,"context_line":"                    self.assertEqual(\u0027myname\u0027, port[\u0027port\u0027][\u0027name\u0027])"},{"line_number":888,"context_line":""},{"line_number":889,"context_line":"    def test_create_port_as_admin(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_e3fa8cbc","line":886,"range":{"start_line":886,"start_character":20,"end_line":886,"end_character":70},"updated":"2016-04-13 19:42:30.000000000","message":"Why not just assert the ip address is valid in the subnet and leave the rest the same?","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":883,"context_line":"                    self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":884,"context_line":"                    ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":885,"context_line":"                    self.assertEqual(1, len(ips))"},{"line_number":886,"context_line":"                    self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":887,"context_line":"                    self.assertEqual(\u0027myname\u0027, port[\u0027port\u0027][\u0027name\u0027])"},{"line_number":888,"context_line":""},{"line_number":889,"context_line":"    def test_create_port_as_admin(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_641322b9","line":886,"range":{"start_line":886,"start_character":20,"end_line":886,"end_character":70},"in_reply_to":"5a18252c_e3fa8cbc","updated":"2016-04-13 20:31:17.000000000","message":"Sure, we can do that. As I stated on L887 I don\u0027t think it really matters. My philosophy with this patch has been to do the least invasive thing and now that you point it out the least invasive thing appears to be asserting an \"any\" allocation in this test.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"8877b3c3c2e2e74fae9a373c022fa9637b987324","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"    def test_update_port_update_ip(self):"},{"line_number":1473,"context_line":"        \"\"\"Test update of port IP."},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"        Check that a configured IP 10.0.0.2 is replaced by 10.0.0.10."},{"line_number":1476,"context_line":"        \"\"\""},{"line_number":1477,"context_line":"        with self.subnet() as subnet:"},{"line_number":1478,"context_line":"            fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_e7c0cf2c","line":1475,"updated":"2016-04-13 22:51:06.000000000","message":"I was just working through some adjustments and noticed this comment. Judging by this comment the author of the test intends to assert here that you can change 10.0.0.2 to 10.0.0.10. That requires a \"specific\" allocation and a change to the API signature exercised by the test is in order.\n\nThis is an example of why I push back just a little bit on just blindly leaving the API signature invoked by the test alone without examining the context, and especially doing so just for the sake of coverage.\n\nIn this case, why add extra logic and complicate the test to handle a random IP allocation when that is not really required to assert proper behavior? If we want to err on the safe side, I would argue being too invasive in these tests introduces more risk than what I\u0027ve proposed. There are a number of changes in this patch where I think this argument can apply and has merit.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        Check that a configured IP 10.0.0.2 is replaced by 10.0.0.10."},{"line_number":1476,"context_line":"        \"\"\""},{"line_number":1477,"context_line":"        with self.subnet() as subnet:"},{"line_number":1478,"context_line":"            fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027,"},{"line_number":1479,"context_line":"                             \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":1480,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1481,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_de6927c0","line":1478,"range":{"start_line":1478,"start_character":12,"end_line":1478,"end_character":25},"updated":"2016-04-13 19:42:30.000000000","message":"This test changes the API call signature that is tested by this method.  These changes make me wonder if we\u0027re losing coverage somewhere or if we balance it out by some other change in another UT test.  If I want to be sure that this patch leaves us with at least the same API coverage as we had before, it is going to be a big chore.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        Check that a configured IP 10.0.0.2 is replaced by 10.0.0.10."},{"line_number":1476,"context_line":"        \"\"\""},{"line_number":1477,"context_line":"        with self.subnet() as subnet:"},{"line_number":1478,"context_line":"            fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027,"},{"line_number":1479,"context_line":"                             \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":1480,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1481,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_24080a2c","line":1478,"range":{"start_line":1478,"start_character":12,"end_line":1478,"end_character":25},"in_reply_to":"5a18252c_de6927c0","updated":"2016-04-13 20:31:17.000000000","message":"In this case, the assertion is that we can change the IP address of a port. To construct a test that asserts that an IP address can be changed, it\u0027s probably easiest to update with a specific IP (which this test does) so you don\u0027t attempt to update by passing the original IP the port has. To ensure that the hard-coded \"new\" IP doesn\u0027t conflict with IP originally allocated, it takes less code to simply perform a \"specific\" to get the initial IP address. The initial IP address really isn\u0027t a core part of the assertion of the test, it only matters to the extent it doesn\u0027t conflict with the update you\u0027re trying to make.\n\nTLDR I believe a \"specific\" allocation is the least invasive thing to do here as \"any vs. specific\" allocation has no bearing on the core assertion of the test.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":1480,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1481,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1482,"context_line":"                self.assertEqual(1, len(ips))"},{"line_number":1483,"context_line":"                self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":1484,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":1485,"context_line":"                data \u003d {\u0027port\u0027: {\u0027fixed_ips\u0027: [{\u0027subnet_id\u0027:"},{"line_number":1486,"context_line":"                                                subnet[\u0027subnet\u0027][\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_fe8ce380","line":1483,"range":{"start_line":1483,"start_character":21,"end_line":1483,"end_character":44},"updated":"2016-04-13 19:42:30.000000000","message":"As far as I can tell, this is the reason you changed to passing fixed_ip through the API.\n\nWhat I expected was to see this changed to a test just to be sure the returned IP is contained within the subnet.  Then, get another IP from the subnet (instead of 10.0.0.10) to request as the second IP address.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":1480,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1481,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1482,"context_line":"                self.assertEqual(1, len(ips))"},{"line_number":1483,"context_line":"                self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":1484,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":1485,"context_line":"                data \u003d {\u0027port\u0027: {\u0027fixed_ips\u0027: [{\u0027subnet_id\u0027:"},{"line_number":1486,"context_line":"                                                subnet[\u0027subnet\u0027][\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_b836fee4","line":1483,"range":{"start_line":1483,"start_character":21,"end_line":1483,"end_character":44},"in_reply_to":"5a18252c_fe8ce380","updated":"2016-04-13 20:31:17.000000000","message":"I contend we don\u0027t need this assertion, it\u0027s not a core assertion of the test. The assertions that really matter start on L1492.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"de3aad65faf28637a6db2519d38165336a1671a2","unresolved":false,"context_lines":[{"line_number":1496,"context_line":"    def test_update_port_update_ip_address_only(self):"},{"line_number":1497,"context_line":"        with self.subnet() as subnet:"},{"line_number":1498,"context_line":"            ip_address \u003d \u002710.0.0.2\u0027"},{"line_number":1499,"context_line":"            fixed_ip_data \u003d [{\u0027ip_address\u0027: ip_address,"},{"line_number":1500,"context_line":"                              \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":1501,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1502,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_694e5b70","line":1499,"range":{"start_line":1499,"start_character":12,"end_line":1499,"end_character":25},"updated":"2016-04-13 19:42:30.000000000","message":"I\u0027m going to have to step away and think some more  about this.  What I see isn\u0027t simply changing the tests\u0027 assertions to remove unnecessary assumptions about IPAM.  What I see is actually changing the way all of the tests call the port API in order to be able to keep the same asserts in place.  For example, we change a lot of port create calls that didn\u0027t used to pass fixed_ips to ones that do just so that we can make sure we got 10.0.0.2 (for example) back as the IP address.\n\nWhen I write tests that exercise an API, I write each one to make a very particular call to the API and then check to make sure that particular way of calling the API worked correctly using assertions.  If later, I find that I made too many assumptions when I asserted the results, I don\u0027t change the way all of my tests call the API so that my assumptions work.  I change to the asserts to eliminate the assumptions.\n\nIn my opinion, the most important part about a test is that it calls the API in a certain way and forever ensures that way of calling the API is supported correctly.  If I change how they all call the API, I feel like I have to check to make sure I haven\u0027t thrown my API coverage completely out of whack.\n\nThat\u0027s how I feel I have to review this patch set and I feel like it is going to take me and other reviews a lot longer to review for that reason because I have to check exhaustively to be sure that all of the important ways of calling the API are exercised.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3317533154bc6100ea59fabcfa6dece7a2a46619","unresolved":false,"context_lines":[{"line_number":1496,"context_line":"    def test_update_port_update_ip_address_only(self):"},{"line_number":1497,"context_line":"        with self.subnet() as subnet:"},{"line_number":1498,"context_line":"            ip_address \u003d \u002710.0.0.2\u0027"},{"line_number":1499,"context_line":"            fixed_ip_data \u003d [{\u0027ip_address\u0027: ip_address,"},{"line_number":1500,"context_line":"                              \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":1501,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ip_data) as port:"},{"line_number":1502,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a18252c_b8ca7e6c","line":1499,"range":{"start_line":1499,"start_character":12,"end_line":1499,"end_character":25},"in_reply_to":"5a18252c_694e5b70","updated":"2016-04-13 20:31:17.000000000","message":"Legitimate concerns. However, something to think about is that perhaps the contracts on the IPAM API\u0027s have been too ambiguous. In my estimation, the assertion that IPAM allocates IP addresses sequentially was never really codified by the API. That is an implementation detail, one that shouldn\u0027t have ever been relied upon. I contend you can\u0027t really fix these tests without changing how they exercise they exercise the API. Some tests need \"specific\" allocations, while some can get by with using \"any\" allocations. What I see is tests where the notion of \"any\" and \"specific\" allocations is not clearly differentiated. It requires us to go test-by-test, looking closely at the core assertions of each test to determine which params the test should pass through the API to support its core assertions. That may mean we throw some coverage out of whack, but I believe we have sufficient coverage targeted at both \"any\" and \"specific\" allocations that in most cases it doesn\u0027t matter and we should just do the least invasive thing that doesn\u0027t change the core assertions of a test.","commit_id":"265c1e0541c74c1293119d0749cb04d6986a7f5a"},{"author":{"_account_id":13768,"name":"Pavel Bondar","email":"pbondar@infoblox.com","username":"pavel_bondar"},"change_message_id":"1a81110207c83bc369f785ed5570dc2e552c81ea","unresolved":false,"context_lines":[{"line_number":1886,"context_line":"                    exp_ip_addr \u003d self._calc_ipv6_addr_by_EUI64(port, subnet)"},{"line_number":1887,"context_line":"                else:"},{"line_number":1888,"context_line":"                    #TODO(tidwellr) can\u0027t count on a specific IP, what to do?"},{"line_number":1889,"context_line":"                    exp_ip_addr \u003d \u0027fe80::2\u0027"},{"line_number":1890,"context_line":"                port_fixed_ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1891,"context_line":"                self.assertEqual(1, len(port_fixed_ips))"},{"line_number":1892,"context_line":"                self.assertEqual(exp_ip_addr,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a18252c_2c71e776","line":1889,"range":{"start_line":1889,"start_character":34,"end_line":1889,"end_character":42},"updated":"2016-04-14 13:37:21.000000000","message":"You probably can verify that allocated ip belongs to \u0027fe80::/64\u0027 (or whatever mask it uses).\nSo validations for SLAAC and regular ipv6 will be different.","commit_id":"ed807907522634bb3bd9c7e0b641cb323cf4b47b"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"5b8086752b671b53fe845daec6a718fa2ecde8eb","unresolved":false,"context_lines":[{"line_number":1886,"context_line":"                    exp_ip_addr \u003d self._calc_ipv6_addr_by_EUI64(port, subnet)"},{"line_number":1887,"context_line":"                else:"},{"line_number":1888,"context_line":"                    #TODO(tidwellr) can\u0027t count on a specific IP, what to do?"},{"line_number":1889,"context_line":"                    exp_ip_addr \u003d \u0027fe80::2\u0027"},{"line_number":1890,"context_line":"                port_fixed_ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1891,"context_line":"                self.assertEqual(1, len(port_fixed_ips))"},{"line_number":1892,"context_line":"                self.assertEqual(exp_ip_addr,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a18252c_8691eff3","line":1889,"range":{"start_line":1889,"start_character":34,"end_line":1889,"end_character":42},"in_reply_to":"5a18252c_2c71e776","updated":"2016-04-14 15:52:55.000000000","message":"Ooh, I missed a TODO. I feel like there may be 2 tests embedded in here that need to be separated. One that asserts the SLAAC address (as is done on L1892), and one that asserts the address is inside the proper subnet. This test as written doesn\u0027t seem isolated from changes to the IP allocation strategy. Thoughts?","commit_id":"ed807907522634bb3bd9c7e0b641cb323cf4b47b"},{"author":{"_account_id":13768,"name":"Pavel Bondar","email":"pbondar@infoblox.com","username":"pavel_bondar"},"change_message_id":"64a551a0c6de81c74386ba49491905144421966f","unresolved":false,"context_lines":[{"line_number":1886,"context_line":"                    exp_ip_addr \u003d self._calc_ipv6_addr_by_EUI64(port, subnet)"},{"line_number":1887,"context_line":"                else:"},{"line_number":1888,"context_line":"                    #TODO(tidwellr) can\u0027t count on a specific IP, what to do?"},{"line_number":1889,"context_line":"                    exp_ip_addr \u003d \u0027fe80::2\u0027"},{"line_number":1890,"context_line":"                port_fixed_ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1891,"context_line":"                self.assertEqual(1, len(port_fixed_ips))"},{"line_number":1892,"context_line":"                self.assertEqual(exp_ip_addr,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a122d0e_bf0d3c40","line":1889,"range":{"start_line":1889,"start_character":34,"end_line":1889,"end_character":42},"in_reply_to":"5a18252c_8691eff3","updated":"2016-04-19 11:01:06.000000000","message":"agree about two test cases here","commit_id":"ed807907522634bb3bd9c7e0b641cb323cf4b47b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"class TestPortsV2(NeutronDbPluginV2TestCase):"},{"line_number":872,"context_line":"    def test_create_port_json(self):"},{"line_number":873,"context_line":"        keys \u003d [(\u0027admin_state_up\u0027, True), (\u0027status\u0027, self.port_create_status)]"},{"line_number":874,"context_line":"        with self.port(name\u003d\u0027myname\u0027) as port:"},{"line_number":875,"context_line":"            for k, v in keys:"},{"line_number":876,"context_line":"                self.assertEqual(port[\u0027port\u0027][k], v)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_af5c8d19","side":"PARENT","line":873,"range":{"start_line":873,"start_character":8,"end_line":873,"end_character":78},"updated":"2016-04-20 21:42:35.000000000","message":"nit:  Inlining this looks like a little bit of unnecessary refactoring.","commit_id":"63e8c6da190489bbfd9268644ef4055ea65b21b1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"57d50c2e893846d7bbf41ca18a81e3691d0fd9d3","unresolved":false,"context_lines":[{"line_number":870,"context_line":""},{"line_number":871,"context_line":"class TestPortsV2(NeutronDbPluginV2TestCase):"},{"line_number":872,"context_line":"    def test_create_port_json(self):"},{"line_number":873,"context_line":"        keys \u003d [(\u0027admin_state_up\u0027, True), (\u0027status\u0027, self.port_create_status)]"},{"line_number":874,"context_line":"        with self.port(name\u003d\u0027myname\u0027) as port:"},{"line_number":875,"context_line":"            for k, v in keys:"},{"line_number":876,"context_line":"                self.assertEqual(port[\u0027port\u0027][k], v)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_83aeca2b","side":"PARENT","line":873,"range":{"start_line":873,"start_character":8,"end_line":873,"end_character":78},"in_reply_to":"1a122d0e_af5c8d19","updated":"2016-04-20 22:42:59.000000000","message":"Artifact of refactoring. Done","commit_id":"63e8c6da190489bbfd9268644ef4055ea65b21b1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":1845,"context_line":"        with self.network() as network:"},{"line_number":1846,"context_line":"            # Create an IPv4 and an IPv6 SLAAC subnet on the network"},{"line_number":1847,"context_line":"            with self.subnet(network) as subnet_v4,\\"},{"line_number":1848,"context_line":"                self.subnet(network,"},{"line_number":1849,"context_line":"                            cidr\u003d\u00272607:f0d0:1002:51::/64\u0027,"},{"line_number":1850,"context_line":"                            ip_version\u003d6,"},{"line_number":1851,"context_line":"                            gateway_ip\u003d\u0027fe80::1\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_2f119d04","line":1848,"updated":"2016-04-20 21:42:35.000000000","message":"nit:  don\u0027t fiddle with the whitespace on L1848 - L1852.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"            fixed_ips \u003d [{\u0027subnet_id\u0027: subnet_id}]"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"            if addr_mode !\u003d constants.IPV6_SLAAC:"},{"line_number":1904,"context_line":"                fixed_ips[0][\u0027ip_address\u0027] \u003d \u0027fe80::2\u0027"},{"line_number":1905,"context_line":""},{"line_number":1906,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ips) as port:"},{"line_number":1907,"context_line":"                if addr_mode \u003d\u003d constants.IPV6_SLAAC:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_0fa4a1b7","line":1904,"range":{"start_line":1904,"start_character":16,"end_line":1904,"end_character":54},"updated":"2016-04-20 21:42:35.000000000","message":"I think this changes your API call in a way that fundamentally changes the test.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"57d50c2e893846d7bbf41ca18a81e3691d0fd9d3","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"            fixed_ips \u003d [{\u0027subnet_id\u0027: subnet_id}]"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"            if addr_mode !\u003d constants.IPV6_SLAAC:"},{"line_number":1904,"context_line":"                fixed_ips[0][\u0027ip_address\u0027] \u003d \u0027fe80::2\u0027"},{"line_number":1905,"context_line":""},{"line_number":1906,"context_line":"            with self.port(subnet\u003dsubnet, fixed_ips\u003dfixed_ips) as port:"},{"line_number":1907,"context_line":"                if addr_mode \u003d\u003d constants.IPV6_SLAAC:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_43eaf274","line":1904,"range":{"start_line":1904,"start_character":16,"end_line":1904,"end_character":54},"in_reply_to":"1a122d0e_0fa4a1b7","updated":"2016-04-20 22:42:59.000000000","message":"In this case I agree, and I\u0027ve been puzzling over this test. See comments at L1910.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":1907,"context_line":"                if addr_mode \u003d\u003d constants.IPV6_SLAAC:"},{"line_number":1908,"context_line":"                    exp_ip_addr \u003d self._calc_ipv6_addr_by_EUI64(port, subnet)"},{"line_number":1909,"context_line":"                else:"},{"line_number":1910,"context_line":"                    #TODO(tidwellr) can\u0027t count on a specific IP, what to do?"},{"line_number":1911,"context_line":"                    exp_ip_addr \u003d \u0027fe80::2\u0027"},{"line_number":1912,"context_line":"                port_fixed_ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1913,"context_line":"                self.assertEqual(1, len(port_fixed_ips))"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_8fd1d125","line":1910,"range":{"start_line":1910,"start_character":36,"end_line":1910,"end_character":76},"updated":"2016-04-20 21:42:35.000000000","message":"Don\u0027t assert on a specific IP.  Just assert that the IP is in the expected subnet.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"57d50c2e893846d7bbf41ca18a81e3691d0fd9d3","unresolved":false,"context_lines":[{"line_number":1907,"context_line":"                if addr_mode \u003d\u003d constants.IPV6_SLAAC:"},{"line_number":1908,"context_line":"                    exp_ip_addr \u003d self._calc_ipv6_addr_by_EUI64(port, subnet)"},{"line_number":1909,"context_line":"                else:"},{"line_number":1910,"context_line":"                    #TODO(tidwellr) can\u0027t count on a specific IP, what to do?"},{"line_number":1911,"context_line":"                    exp_ip_addr \u003d \u0027fe80::2\u0027"},{"line_number":1912,"context_line":"                port_fixed_ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1913,"context_line":"                self.assertEqual(1, len(port_fixed_ips))"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_23ca561b","line":1910,"range":{"start_line":1910,"start_character":36,"end_line":1910,"end_character":76},"in_reply_to":"1a122d0e_8fd1d125","updated":"2016-04-20 22:42:59.000000000","message":"I wasn\u0027t sure what to do here since that changes the assertion. If we\u0027re using SLAAC, do we want to assert a specific IP? I guess I\u0027ve been a little puzzled with this test because I see a case where asserting the subnet is good enough, but also a case where asserting a specific IP is what we want. Thoughts?","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":2180,"context_line":"                ports_to_delete \u003d []"},{"line_number":2181,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":2182,"context_line":"                self.assertEqual(1, len(ips))"},{"line_number":2183,"context_line":"                self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":2184,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":2185,"context_line":"                # Allocate specific IP"},{"line_number":2186,"context_line":"                kwargs \u003d {\"fixed_ips\": [{\u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_9228d69b","line":2183,"range":{"start_line":2183,"start_character":33,"end_line":2183,"end_character":43},"updated":"2016-04-20 21:42:35.000000000","message":"I\u0027d prefer you change this assert to just test that the IP address is valid on the subnet instead of depending on a specific address.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"3e865ae0fec0956e6a72f297cdc4746b1976ee64","unresolved":false,"context_lines":[{"line_number":2183,"context_line":"                self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":2184,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":2185,"context_line":"                # Allocate specific IP"},{"line_number":2186,"context_line":"                kwargs \u003d {\"fixed_ips\": [{\u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027],"},{"line_number":2187,"context_line":"                                         \u0027ip_address\u0027: \u002710.0.0.5\u0027}]}"},{"line_number":2188,"context_line":"                net_id \u003d port[\u0027port\u0027][\u0027network_id\u0027]"},{"line_number":2189,"context_line":"                res \u003d self._create_port(self.fmt, net_id\u003dnet_id, **kwargs)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_e4f18496","line":2186,"updated":"2016-04-21 00:12:15.000000000","message":"I wonder about the value of this test. Now that I look at it with fresh eyes, it seems to be asserting in-order allocation of IP addresses, asserting that IPAM allocates around 10.0.0.5. We have plenty of tests that assert we don\u0027t double-allocate an IP address. I wonder if we can just delete this test.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"                    fixed_ip_data \u003d [{\u0027ip_address\u0027: a,"},{"line_number":2201,"context_line":"                                      \u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]}]"},{"line_number":2202,"context_line":"                    res \u003d self._create_port(self.fmt, net_id\u003dnet_id,"},{"line_number":2203,"context_line":"                                            fixed_ips\u003dfixed_ip_data)"},{"line_number":2204,"context_line":"                    port2 \u003d self.deserialize(self.fmt, res)"},{"line_number":2205,"context_line":"                    ports_to_delete.append(port2)"},{"line_number":2206,"context_line":"                    ips \u003d port2[\u0027port\u0027][\u0027fixed_ips\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_326f42fd","line":2203,"range":{"start_line":2203,"start_character":54,"end_line":2203,"end_character":67},"updated":"2016-04-20 21:42:35.000000000","message":"I still don\u0027t like changing the API call to validate the assertions.  I\u0027d much rather assert below that we get IPs that are valid and we haven\u0027t seen before.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"                          [{\u0027subnet_id\u0027: subnet[\u0027subnet\u0027][\u0027id\u0027]},"},{"line_number":1766,"context_line":"                           {\u0027subnet_id\u0027: subnet2[\u0027subnet\u0027][\u0027id\u0027]}]}"},{"line_number":1767,"context_line":"                res \u003d self._create_port(self.fmt, net_id\u003dnet_id, **kwargs)"},{"line_number":1768,"context_line":"                port3 \u003d self.deserialize(self.fmt, res)"},{"line_number":1769,"context_line":"                ips \u003d port3[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1770,"context_line":"                self.assertEqual(2, len(ips))"},{"line_number":1771,"context_line":"                self.assertIn({\u0027ip_address\u0027: \u002710.0.0.2\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_d7e41d3d","side":"PARENT","line":1768,"updated":"2016-05-16 22:26:40.000000000","message":"Heh why is this called port3. Where did 1 and 2 go, or are we counting from 3 nowadays?","commit_id":"60db12f9d2f8a44d411e2ee8742c33be08078864"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                    self.assertEqual(port[\u0027port\u0027][k], v)"},{"line_number":953,"context_line":"                port_ip \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027][0][\u0027ip_address\u0027]"},{"line_number":954,"context_line":"                port_ip \u003d netaddr.IPAddress(port_ip)"},{"line_number":955,"context_line":"                self.assertTrue(port_ip in netaddr.IPSet(ip_net))"},{"line_number":956,"context_line":"                self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":957,"context_line":"                self._delete(\u0027ports\u0027, port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":958,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_571dcd60","line":955,"updated":"2016-05-16 22:26:40.000000000","message":"Should use assertIn because if it fails the description would actually tell you what\u0027s wrong (Instead of False is not True which is really not helpful).","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                    self.assertEqual(port[\u0027port\u0027][k], v)"},{"line_number":953,"context_line":"                port_ip \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027][0][\u0027ip_address\u0027]"},{"line_number":954,"context_line":"                port_ip \u003d netaddr.IPAddress(port_ip)"},{"line_number":955,"context_line":"                self.assertTrue(port_ip in netaddr.IPSet(ip_net))"},{"line_number":956,"context_line":"                self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":957,"context_line":"                self._delete(\u0027ports\u0027, port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":958,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_4d0c1aec","line":955,"in_reply_to":"dab17558_571dcd60","updated":"2016-05-16 22:54:58.000000000","message":"Oops","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"    def test_update_port_add_additional_ip(self):"},{"line_number":1602,"context_line":"        \"\"\"Test update of port with additional IP.\"\"\""},{"line_number":1603,"context_line":"        with self.subnet() as subnet:"},{"line_number":1604,"context_line":"            network_ip_set \u003d netaddr.IPSet("},{"line_number":1605,"context_line":"                netaddr.IPNetwork(subnet[\u0027subnet\u0027][\u0027cidr\u0027]))"},{"line_number":1606,"context_line":"            with self.port(subnet\u003dsubnet) as port:"},{"line_number":1607,"context_line":"                data \u003d {\u0027port\u0027: {\u0027admin_state_up\u0027: False,"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_f7796194","line":1604,"updated":"2016-05-16 22:26:40.000000000","message":"I think it\u0027s more readable to define this variable in line 1618, otherwise I have to look to see if it\u0027s used before then.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"dc818c2971d8c762f7ee2aa9870c89d64bf15035","unresolved":false,"context_lines":[{"line_number":1707,"context_line":"            with self.port(subnet\u003dsubnet) as port:"},{"line_number":1708,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1709,"context_line":"                self.assertEqual(1, len(ips))"},{"line_number":1710,"context_line":"                self.assertIn(netaddr.IPAddress(ips[0][\u0027ip_address\u0027]),"},{"line_number":1711,"context_line":"                              netaddr.IPSet(subnet_ip_net))"},{"line_number":1712,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":1713,"context_line":"                # Check configuring of duplicate IP"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_2513b4b9","line":1710,"updated":"2016-05-16 20:33:22.000000000","message":"You should be able to assertIn subnet_ip_net directly.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":1707,"context_line":"            with self.port(subnet\u003dsubnet) as port:"},{"line_number":1708,"context_line":"                ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":1709,"context_line":"                self.assertEqual(1, len(ips))"},{"line_number":1710,"context_line":"                self.assertIn(netaddr.IPAddress(ips[0][\u0027ip_address\u0027]),"},{"line_number":1711,"context_line":"                              netaddr.IPSet(subnet_ip_net))"},{"line_number":1712,"context_line":"                self.assertEqual(ips[0][\u0027subnet_id\u0027], subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":1713,"context_line":"                # Check configuring of duplicate IP"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_cdee4aec","line":1710,"in_reply_to":"dab17558_2513b4b9","updated":"2016-05-16 22:54:58.000000000","message":"Yep, you learn something new every day :)","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":1790,"context_line":"                subnet1_net \u003d netaddr.IPNetwork(subnet[\u0027subnet\u0027][\u0027cidr\u0027])"},{"line_number":1791,"context_line":"                subnet2_net \u003d netaddr.IPNetwork(subnet2[\u0027subnet\u0027][\u0027cidr\u0027])"},{"line_number":1792,"context_line":"                network_ip_set \u003d netaddr.IPSet(subnet1_net)"},{"line_number":1793,"context_line":"                network_ip_set.add(subnet2_net)"},{"line_number":1794,"context_line":"                self.assertEqual(2, len(ips))"},{"line_number":1795,"context_line":"                self.assertIn(ips[0][\u0027ip_address\u0027], network_ip_set)"},{"line_number":1796,"context_line":"                self.assertIn(ips[1][\u0027ip_address\u0027], network_ip_set)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_f76321af","line":1793,"updated":"2016-05-16 22:26:40.000000000","message":"Is there a guarantee about ordering? For example here we asked for [subnet, subnet2] in that order, does that mean that we are supposed to get the fixed_ips allocated in that order? If that is the intention then we should make sure that the first IP belongs to the first subnet, and the second IP to the second subnet. Right now we just use a single IPSet so we don\u0027t know.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":1790,"context_line":"                subnet1_net \u003d netaddr.IPNetwork(subnet[\u0027subnet\u0027][\u0027cidr\u0027])"},{"line_number":1791,"context_line":"                subnet2_net \u003d netaddr.IPNetwork(subnet2[\u0027subnet\u0027][\u0027cidr\u0027])"},{"line_number":1792,"context_line":"                network_ip_set \u003d netaddr.IPSet(subnet1_net)"},{"line_number":1793,"context_line":"                network_ip_set.add(subnet2_net)"},{"line_number":1794,"context_line":"                self.assertEqual(2, len(ips))"},{"line_number":1795,"context_line":"                self.assertIn(ips[0][\u0027ip_address\u0027], network_ip_set)"},{"line_number":1796,"context_line":"                self.assertIn(ips[1][\u0027ip_address\u0027], network_ip_set)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_4d325a5f","line":1793,"in_reply_to":"dab17558_f76321af","updated":"2016-05-16 22:54:58.000000000","message":"That\u0027s a good point, no ordering can be assumed. This might be worth a re-spin since the point is to assert the IP addresses you got back are from a specific subnet.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"40f3186403c8cabc26e96ec5f7a5ad83783b1d70","unresolved":false,"context_lines":[{"line_number":955,"context_line":"                for k, v in keys:"},{"line_number":956,"context_line":"                    self.assertEqual(port[\u0027port\u0027][k], v)"},{"line_number":957,"context_line":"                port_ip \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027][0][\u0027ip_address\u0027]"},{"line_number":958,"context_line":"                port_ip \u003d netaddr.IPAddress(port_ip)"},{"line_number":959,"context_line":"                self.assertIn(port_ip, ip_net)"},{"line_number":960,"context_line":"                self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":961,"context_line":"                self._delete(\u0027ports\u0027, port[\u0027port\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_63ae4f2f","line":958,"range":{"start_line":958,"start_character":34,"end_line":958,"end_character":43},"updated":"2016-05-17 00:23:44.000000000","message":"for future reference you don\u0027t need to turn it into an IPAddress object first. You can check if a string is in an IPNetwork and netaddr automatically turns it into an IP.","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f676c70a2ea74e1297aa567ab903433962ab5965","unresolved":false,"context_lines":[{"line_number":1816,"context_line":"                self._delete(\u0027ports\u0027, port4[\u0027port\u0027][\u0027id\u0027])"},{"line_number":1817,"context_line":""},{"line_number":1818,"context_line":"    def _test_requested_port_subnet_ids(self, ips, expected_subnet_ids):"},{"line_number":1819,"context_line":"        observed_subnet_ids \u003d [x[\u0027subnet_id\u0027] for x in ips]"},{"line_number":1820,"context_line":"        for subnet_id in expected_subnet_ids:"},{"line_number":1821,"context_line":"            self.assertIn(subnet_id, observed_subnet_ids)"},{"line_number":1822,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_236f27fd","line":1819,"updated":"2016-05-17 00:26:17.000000000","message":"Can be:\n\nself.assertEqual(set(x[\u0027subnet_id\u0027] for x in ips), set(expected_subnet_ids))","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f676c70a2ea74e1297aa567ab903433962ab5965","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"        ip_net_v4 \u003d netaddr.IPNetwork(cidr_v4)"},{"line_number":1826,"context_line":"        ip_net_v6 \u003d netaddr.IPNetwork(cidr_v6)"},{"line_number":1827,"context_line":"        for address in ips:"},{"line_number":1828,"context_line":"            ip_addr \u003d netaddr.IPAddress(address[\u0027ip_address\u0027])"},{"line_number":1829,"context_line":"            if ip_addr.version \u003d\u003d 4:"},{"line_number":1830,"context_line":"                expected_ip_net \u003d ip_net_v4"},{"line_number":1831,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_c390dbef","line":1828,"updated":"2016-05-17 00:26:17.000000000","message":"Could be:\n\nexpected_ip_net \u003d ip_net_v4 if ip_addr.version \u003d\u003d 4 else ip_net_v6","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"}],"neutron/tests/unit/extensions/test_dns.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24702868cba7ac2630c97afc3d16ac6ba2a6a723","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":122,"context_line":"            ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":123,"context_line":"            self.assertEqual(1, len(ips))"},{"line_number":124,"context_line":"            self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":125,"context_line":"            self.assertEqual(\u0027myname\u0027, port[\u0027port\u0027][\u0027name\u0027])"},{"line_number":126,"context_line":"            self._verify_dns_assigment(port[\u0027port\u0027],"},{"line_number":127,"context_line":"                                       ips_list\u003d[\u002710.0.0.2\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_95fc40b9","line":124,"range":{"start_line":124,"start_character":0,"end_line":124,"end_character":62},"updated":"2016-04-20 21:42:35.000000000","message":"Change this assert, not the API call.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"57d50c2e893846d7bbf41ca18a81e3691d0fd9d3","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            self.assertIn(\u0027mac_address\u0027, port[\u0027port\u0027])"},{"line_number":122,"context_line":"            ips \u003d port[\u0027port\u0027][\u0027fixed_ips\u0027]"},{"line_number":123,"context_line":"            self.assertEqual(1, len(ips))"},{"line_number":124,"context_line":"            self.assertEqual(\u002710.0.0.2\u0027, ips[0][\u0027ip_address\u0027])"},{"line_number":125,"context_line":"            self.assertEqual(\u0027myname\u0027, port[\u0027port\u0027][\u0027name\u0027])"},{"line_number":126,"context_line":"            self._verify_dns_assigment(port[\u0027port\u0027],"},{"line_number":127,"context_line":"                                       ips_list\u003d[\u002710.0.0.2\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_43a15245","line":124,"range":{"start_line":124,"start_character":0,"end_line":124,"end_character":62},"in_reply_to":"1a122d0e_95fc40b9","updated":"2016-04-20 22:42:59.000000000","message":"Done","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"}],"neutron/tests/unit/extensions/test_l3.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":2478,"context_line":"                               ) as private_port:"},{"line_number":2479,"context_line":""},{"line_number":2480,"context_line":"                    fp1 \u003d self._make_floatingip(self.fmt, network_ex_id1,"},{"line_number":2481,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2482,"context_line":"                    fp2 \u003d self._make_floatingip(self.fmt, network_ex_id2,"},{"line_number":2483,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2484,"context_line":"                    self.assertEqual(fp1[\u0027floatingip\u0027][\u0027router_id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_9778d5c5","line":2481,"updated":"2016-05-16 22:26:40.000000000","message":"You removed these because of the risk of a conflict I imagine, did you check there\u0027s still tests that test this behavior explicitly? Asking for a specific floating IP?","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6c4589582450f4e621b65faace32bc2808bc31cf","unresolved":false,"context_lines":[{"line_number":2478,"context_line":"                               ) as private_port:"},{"line_number":2479,"context_line":""},{"line_number":2480,"context_line":"                    fp1 \u003d self._make_floatingip(self.fmt, network_ex_id1,"},{"line_number":2481,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2482,"context_line":"                    fp2 \u003d self._make_floatingip(self.fmt, network_ex_id2,"},{"line_number":2483,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2484,"context_line":"                    self.assertEqual(fp1[\u0027floatingip\u0027][\u0027router_id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_2d46f67a","line":2481,"in_reply_to":"dab17558_8d73a27f","updated":"2016-05-16 22:59:38.000000000","message":"Thank you.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":2478,"context_line":"                               ) as private_port:"},{"line_number":2479,"context_line":""},{"line_number":2480,"context_line":"                    fp1 \u003d self._make_floatingip(self.fmt, network_ex_id1,"},{"line_number":2481,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2482,"context_line":"                    fp2 \u003d self._make_floatingip(self.fmt, network_ex_id2,"},{"line_number":2483,"context_line":"                                            private_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":2484,"context_line":"                    self.assertEqual(fp1[\u0027floatingip\u0027][\u0027router_id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_8d73a27f","line":2481,"in_reply_to":"dab17558_9778d5c5","updated":"2016-05-16 22:54:58.000000000","message":"Yes, see test_create_floatingip_with_specific_ip() as an example.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"}],"neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        return ipam_subnet.allocate(address_request)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def test_allocate_any_v4_address_succeeds(self):"},{"line_number":358,"context_line":"        self._test_allocate_any_address_succeeds(\u002710.0.0.0/24\u0027, 4)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def test_allocate_any_v6_address_succeeds(self):"},{"line_number":361,"context_line":"        self._test_allocate_any_address_succeeds(\u0027fde3:abcd:4321:1::/64\u0027, 6)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_4af080fd","line":358,"updated":"2016-05-16 22:26:40.000000000","message":"For readability purposes it\u0027s nicer to call with:\n\nself._test_allocate_any_address_succeeds(\u002710.0.0.0/24\u0027, ip_version\u003d4)\n\nInstead of the reader wondering what the 4 stands for. In this case the function\u0027s definition is a few lines down, I just try to make this a habit.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"}],"neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6a9f4db32d68fee34b8ce3a8b444198e96a7ef8b","unresolved":false,"context_lines":[{"line_number":712,"context_line":"                                                ip_address\u003d\u002710.0.0.2\u0027)],"},{"line_number":713,"context_line":"                                   \u0027before\u0027: [l2pop_rpc.PortInfo("},{"line_number":714,"context_line":"                                                mac_address\u003dp1[\u0027mac_address\u0027],"},{"line_number":715,"context_line":"                                                ip_address\u003d\u002710.0.0.1\u0027)]}}}}"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"                self.mock_fanout.assert_any_call("},{"line_number":718,"context_line":"                    mock.ANY, \u0027update_fdb_entries\u0027, add_expected)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a1b2928_57825c95","line":715,"updated":"2016-04-11 19:45:11.000000000","message":"Are these in reverse order? i.e. should be before/after ?","commit_id":"9d4ece37ff1866cbf85f2068889c3d11e616e2a2"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"27feff5dc36ac10517256b0d7a02af924ee0ccc9","unresolved":false,"context_lines":[{"line_number":712,"context_line":"                                                ip_address\u003d\u002710.0.0.2\u0027)],"},{"line_number":713,"context_line":"                                   \u0027before\u0027: [l2pop_rpc.PortInfo("},{"line_number":714,"context_line":"                                                mac_address\u003dp1[\u0027mac_address\u0027],"},{"line_number":715,"context_line":"                                                ip_address\u003d\u002710.0.0.1\u0027)]}}}}"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"                self.mock_fanout.assert_any_call("},{"line_number":718,"context_line":"                    mock.ANY, \u0027update_fdb_entries\u0027, add_expected)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a1b2928_823cb094","line":715,"in_reply_to":"7a1b2928_57825c95","updated":"2016-04-11 19:56:20.000000000","message":"This passes in my sandbox, but fails in the check job. Reverting this change to see what happens.","commit_id":"9d4ece37ff1866cbf85f2068889c3d11e616e2a2"}],"neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py":[{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"edab83d3d732cab75728361385a7c59655e8edb0","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":71,"context_line":"        if ipv4:"},{"line_number":72,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":73,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first),"},{"line_number":74,"context_line":"                          netaddr.IPAddress(ip_net.last / 2))"},{"line_number":75,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start),"},{"line_number":76,"context_line":"                                 \u0027end\u0027: str(end)}]"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a122d0e_fad5a6e4","line":73,"updated":"2016-04-20 19:28:19.000000000","message":"duh, this is silly. I\u0027m embarassed.........","commit_id":"14c150793d381700d29f13ac0d0963f9d83b8dd9"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"d3d4f2a0cf829ea1af7ab82d26f98e454b2d7931","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        original_ips \u003d [ip[\u0027ip_address\u0027] for ip in port[\u0027fixed_ips\u0027]]"},{"line_number":377,"context_line":"        kwargs \u003d {\u0027fixed_ips\u0027: []}"},{"line_number":378,"context_line":"        for ip in port[\u0027fixed_ips\u0027]:"},{"line_number":379,"context_line":"            ip_net \u003d netaddr.IPNetwork(ip[\u0027fixed_ip\u0027])"},{"line_number":380,"context_line":"            start, end \u003d (netaddr.IPAddress((ip_net.last / 2) + 1),"},{"line_number":381,"context_line":"                          netaddr.IPAddress(ip_net.last))"},{"line_number":382,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start), \u0027end\u0027: str(end)}]"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a122d0e_1a6ce29a","line":379,"updated":"2016-04-20 19:22:07.000000000","message":"this isn\u0027t right, we don\u0027t have the prefix length here","commit_id":"14c150793d381700d29f13ac0d0963f9d83b8dd9"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"d3d4f2a0cf829ea1af7ab82d26f98e454b2d7931","unresolved":false,"context_lines":[{"line_number":386,"context_line":"            req.get_response(self.api)"},{"line_number":387,"context_line":"            kwargs[\u0027fixed_ips\u0027].append("},{"line_number":388,"context_line":"                {\u0027subnet_id\u0027: ip[\u0027subnet_id\u0027]})"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        port, dns_data_db \u003d self._update_port_for_test(port,"},{"line_number":391,"context_line":"                                                       new_dns_name\u003dNone,"},{"line_number":392,"context_line":"                                                       **kwargs)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a122d0e_dafc2aba","line":389,"updated":"2016-04-20 19:22:07.000000000","message":"oops","commit_id":"14c150793d381700d29f13ac0d0963f9d83b8dd9"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"57d50c2e893846d7bbf41ca18a81e3691d0fd9d3","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d ip_net.first + subnet_size / 2"},{"line_number":77,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first + 2),"},{"line_number":78,"context_line":"                          netaddr.IPAddress(subnet_mid_point))"},{"line_number":79,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start),"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_633a8ee8","line":76,"updated":"2016-04-20 22:42:59.000000000","message":"Oops, not getting integer values back under python 3 :( Silly mistake, will correct with the next patch set.","commit_id":"cf569cc3a09810502466bca479f9e99fd3685a0e"},{"author":{"_account_id":13768,"name":"Pavel Bondar","email":"pbondar@infoblox.com","username":"pavel_bondar"},"change_message_id":"5a7f45718b4853da587f3ec47e4886ca7324d4a6","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"},{"line_number":77,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first + 2),"},{"line_number":78,"context_line":"                          netaddr.IPAddress(subnet_mid_point))"},{"line_number":79,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start),"},{"line_number":80,"context_line":"                                 \u0027end\u0027: str(end)}]"},{"line_number":81,"context_line":"            self._create_subnet(self.fmt, network[\u0027network\u0027][\u0027id\u0027],"},{"line_number":82,"context_line":"                                str(ip_net), ip_version\u003d4,"},{"line_number":83,"context_line":"                                allocation_pools\u003dallocation_pools)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a122d0e_5f483ab3","line":80,"range":{"start_line":75,"start_character":12,"end_line":80,"end_character":50},"updated":"2016-04-27 13:51:12.000000000","message":"Didn\u0027t catch why allocation pools building is needed here and why allocation pool end point is in the middle of subnet?","commit_id":"42f164dde44f32be9db5927223321d736247f6d9"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"313e1b3b2bf46d8faeb721e1e71dbd4e991e4cd5","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"},{"line_number":77,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first + 2),"},{"line_number":78,"context_line":"                          netaddr.IPAddress(subnet_mid_point))"},{"line_number":79,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start),"},{"line_number":80,"context_line":"                                 \u0027end\u0027: str(end)}]"},{"line_number":81,"context_line":"            self._create_subnet(self.fmt, network[\u0027network\u0027][\u0027id\u0027],"},{"line_number":82,"context_line":"                                str(ip_net), ip_version\u003d4,"},{"line_number":83,"context_line":"                                allocation_pools\u003dallocation_pools)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a122d0e_6e52a168","line":80,"range":{"start_line":75,"start_character":12,"end_line":80,"end_character":50},"in_reply_to":"1a122d0e_5f483ab3","updated":"2016-04-27 19:14:42.000000000","message":"test_update_port_fixed_ips_with_subnet_ids wants update a port, changing its IP address with an \"any\" request. To ensure that the IP address actually changes and IPAM doesn\u0027t hand back the same IP address, the allocation pool is changed before updating the port. DNS events won\u0027t be triggered if the same IP address comes back from IPAM, so we need to ensure the test get\u0027s a different IP back on the port update. It seemed simple enough to use the bottom half of the subnet by default. In test_update_port_fixed_ips_with_subnet_ids() we change the allocation pool to the upper half of the subnet.","commit_id":"42f164dde44f32be9db5927223321d736247f6d9"},{"author":{"_account_id":13768,"name":"Pavel Bondar","email":"pbondar@infoblox.com","username":"pavel_bondar"},"change_message_id":"3603778a77b2cd8908f60292b5c85dbfbf6fa3fb","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"},{"line_number":77,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first + 2),"},{"line_number":78,"context_line":"                          netaddr.IPAddress(subnet_mid_point))"},{"line_number":79,"context_line":"            allocation_pools \u003d [{\u0027start\u0027: str(start),"},{"line_number":80,"context_line":"                                 \u0027end\u0027: str(end)}]"},{"line_number":81,"context_line":"            self._create_subnet(self.fmt, network[\u0027network\u0027][\u0027id\u0027],"},{"line_number":82,"context_line":"                                str(ip_net), ip_version\u003d4,"},{"line_number":83,"context_line":"                                allocation_pools\u003dallocation_pools)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a122d0e_443cb8e0","line":80,"range":{"start_line":75,"start_character":12,"end_line":80,"end_character":50},"in_reply_to":"1a122d0e_6e52a168","updated":"2016-04-29 09:05:57.000000000","message":"Thanks for explanation, now it make sense to me.\nnit: It would be nice to have this comment in code.","commit_id":"42f164dde44f32be9db5927223321d736247f6d9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        res \u003d self._create_network(self.fmt, \u0027test_network\u0027, True,"},{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_8a6ee82c","line":73,"updated":"2016-05-16 22:26:40.000000000","message":"After a superficial read of test_create_port it\u0027s not clear to me why we need to make sure that the subnet allocation pools only cover the first half of the subnet. I think that a good test shouldn\u0027t require an in-depth read to understand what\u0027s going on. Can you add a comment explaining why is it important to specify the allocation pool?","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        res \u003d self._create_network(self.fmt, \u0027test_network\u0027, True,"},{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_8dde026b","line":73,"in_reply_to":"dab17558_8a6ee82c","updated":"2016-05-16 22:54:58.000000000","message":"Good question. This was an ugly one to untangle. The test we\u0027re really trying to address wants to use an \"any\" request to change the fixed IP of a port, then assert that the DNS record is updated. Since removing the test is probably unwise, we need to massage the environment so we don\u0027t get back the same IP address we had when we perform an \"any\" allocation. As an optimization, the DNS integration won\u0027t update the DNS record if the IP address doesn\u0027t change. What we\u0027re trying to do is force the IP address to change by manipulating the allocation pools so we never get the same IP address back when the test updates the port. Any other ideas for how to handle this? I\u0027m not gonna argue it\u0027s ideal :)","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6c4589582450f4e621b65faace32bc2808bc31cf","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        res \u003d self._create_network(self.fmt, \u0027test_network\u0027, True,"},{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_0d64d21f","line":73,"in_reply_to":"dab17558_8dde026b","updated":"2016-05-16 22:59:38.000000000","message":"I wasn\u0027t saying we need to necessarily do this any other way, but documenting the intent is important as it\u0027s not immediately obvious.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            ip_net \u003d netaddr.IPNetwork(\u002710.0.0.0/24\u0027)"},{"line_number":75,"context_line":"            subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":76,"context_line":"            subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"},{"line_number":77,"context_line":"            start, end \u003d (netaddr.IPAddress(ip_net.first + 2),"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_2a11bc04","line":74,"updated":"2016-05-16 22:26:40.000000000","message":"It looks like you\u0027re repeating lines 74 to 80 three times. Perhaps extract to a function?","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"6aec7a08ac1d31a816db09177b02400113cf1091","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        recordsets \u003d []"},{"line_number":119,"context_line":"        if records_v4:"},{"line_number":120,"context_line":"            recordsets.append({\u0027id\u0027: V4UUID, \u0027records\u0027: records_v4})"},{"line_number":121,"context_line":"        if records_v6:"},{"line_number":122,"context_line":"            recordsets.append({\u0027id\u0027: V6UUID, \u0027records\u0027: records_v6})"},{"line_number":123,"context_line":"        mock_client.recordsets.list.return_value \u003d recordsets"},{"line_number":124,"context_line":"        mock_admin_client.reset_mock()"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_0a903842","line":121,"updated":"2016-05-16 22:26:40.000000000","message":"\u0027Oy vey\u0027 as my people say. This really belongs in its own patch.","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"fe364b1c3e8f69445e07cfe8e3bf7fe645241207","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        recordsets \u003d []"},{"line_number":119,"context_line":"        if records_v4:"},{"line_number":120,"context_line":"            recordsets.append({\u0027id\u0027: V4UUID, \u0027records\u0027: records_v4})"},{"line_number":121,"context_line":"        if records_v6:"},{"line_number":122,"context_line":"            recordsets.append({\u0027id\u0027: V6UUID, \u0027records\u0027: records_v6})"},{"line_number":123,"context_line":"        mock_client.recordsets.list.return_value \u003d recordsets"},{"line_number":124,"context_line":"        mock_admin_client.reset_mock()"}],"source_content_type":"text/x-python","patch_set":15,"id":"dab17558_ad4306ae","line":121,"in_reply_to":"dab17558_0a903842","updated":"2016-05-16 22:54:58.000000000","message":"meh, probably","commit_id":"e689168932087e8df9db07c6dbfba691ed9c6bc1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f676c70a2ea74e1297aa567ab903433962ab5965","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        if ipv6:"},{"line_number":77,"context_line":"            cidr \u003d \u0027fd3d:bdd4:da60::/64\u0027"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        self._create_subnet_for_test(network[\u0027network\u0027][\u0027id\u0027], cidr)"}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_034b0388","line":76,"updated":"2016-05-17 00:26:17.000000000","message":"Oops, doesn\u0027t this mean that the subnet can never be dual stack? (It can be in the previous patchset).","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"154105ea6291f46fb78bb7b03215d2f00d8e087d","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        if ipv6:"},{"line_number":77,"context_line":"            cidr \u003d \u0027fd3d:bdd4:da60::/64\u0027"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        self._create_subnet_for_test(network[\u0027network\u0027][\u0027id\u0027], cidr)"}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_de8d5232","line":76,"in_reply_to":"dab17558_034b0388","updated":"2016-05-17 00:50:38.000000000","message":"ooooh, good catch","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"1928879b499455a8242da50532aa8a7a701d9140","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        if ipv6:"},{"line_number":77,"context_line":"            cidr \u003d \u0027fd3d:bdd4:da60::/64\u0027"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        self._create_subnet_for_test(network[\u0027network\u0027][\u0027id\u0027], cidr)"}],"source_content_type":"text/x-python","patch_set":16,"id":"dab17558_b90a2777","line":76,"in_reply_to":"dab17558_de8d5232","updated":"2016-05-17 02:25:05.000000000","message":"Doh!","commit_id":"9e2998753b352e45c3805bd7b853fae208e6f705"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"2fe4ae9ba3af1ac4c7babbef34a4e362a5bb684d","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        res \u003d self._create_network(self.fmt, \u0027test_network\u0027, True,"},{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        if ipv6:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dab17558_341e155f","line":73,"updated":"2016-05-17 16:12:23.000000000","message":"if both ipv4 and ipv6 were True before this patch, you created two subnets on the network. In patchsets 16 and 17 you create only an ipv6 subnet.","commit_id":"bc321dc5aad0fdade5730997e0b4d0ddcdd2016c"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"968d02ff176e077b4ff3f21bb42f3cc3563562ec","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        res \u003d self._create_network(self.fmt, \u0027test_network\u0027, True,"},{"line_number":71,"context_line":"                                   **net_kwargs)"},{"line_number":72,"context_line":"        network \u003d self.deserialize(self.fmt, res)"},{"line_number":73,"context_line":"        if ipv4:"},{"line_number":74,"context_line":"            cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        if ipv6:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dab17558_7427fdc9","line":73,"in_reply_to":"dab17558_341e155f","updated":"2016-05-17 16:30:59.000000000","message":"Doh, getting sloppy","commit_id":"bc321dc5aad0fdade5730997e0b4d0ddcdd2016c"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"8d2e432aaa45cde57d5f9a18637931a018e1d25b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        return network[\u0027network\u0027], port, dns_data_db"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _create_subnet_for_test(self, network_id, cidr):"},{"line_number":98,"context_line":"        ip_net \u003d netaddr.IPNetwork(cidr)"},{"line_number":99,"context_line":"        # initialize the allocation_pool to the lower half of the subnet"},{"line_number":100,"context_line":"        subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":101,"context_line":"        subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dab17558_f2865d63","line":98,"updated":"2016-05-17 16:59:32.000000000","message":"Does it make sense to share the code here up to lines 105, and down below lines 389 to 394?","commit_id":"cbc15d2e1db22fbefaac0a97363589aebf834b24"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f51792bdbda6340c83911675c9bd0d1636ce6e1e","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        return network[\u0027network\u0027], port, dns_data_db"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _create_subnet_for_test(self, network_id, cidr):"},{"line_number":98,"context_line":"        ip_net \u003d netaddr.IPNetwork(cidr)"},{"line_number":99,"context_line":"        # initialize the allocation_pool to the lower half of the subnet"},{"line_number":100,"context_line":"        subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":101,"context_line":"        subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dab17558_ac32b1ef","line":98,"in_reply_to":"dab17558_633e1db9","updated":"2016-05-17 19:09:53.000000000","message":"Yeah looking at it I don\u0027t see any obvious way to extract anything.","commit_id":"cbc15d2e1db22fbefaac0a97363589aebf834b24"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"b02f25946a98e406ce1755adcd7b351ed0b43e95","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        return network[\u0027network\u0027], port, dns_data_db"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _create_subnet_for_test(self, network_id, cidr):"},{"line_number":98,"context_line":"        ip_net \u003d netaddr.IPNetwork(cidr)"},{"line_number":99,"context_line":"        # initialize the allocation_pool to the lower half of the subnet"},{"line_number":100,"context_line":"        subnet_size \u003d ip_net.last - ip_net.first"},{"line_number":101,"context_line":"        subnet_mid_point \u003d int(ip_net.first + subnet_size / 2)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dab17558_633e1db9","line":98,"in_reply_to":"dab17558_f2865d63","updated":"2016-05-17 17:55:02.000000000","message":"I suppose there could be a utility to generate allocation pools, I just haven\u0027t come up with anything useful yet. These blocks of code use different halves of the subnet for the allocation pool and are different calculations.\n\nI started toying with this and just wasn\u0027t seeing a lot of utility in a method along the lines of _make_allocation_pools_for_subnet(self, ip_net, lower\u003dTrue). I also looked at a method that parameterizes these calculations, but because they are different calculations I found myself doing essentially the same thing, basically only breaking out L104/394 into a method which didn\u0027t seem very compelling either.\n\nI\u0027m open to suggestions, this does have a smell to it.","commit_id":"cbc15d2e1db22fbefaac0a97363589aebf834b24"}],"neutron/tests/unit/plugins/ml2/test_plugin.py":[{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"e0c7c5bb1fbd101977c8ea547e6126bef1dac112","unresolved":false,"context_lines":[{"line_number":794,"context_line":"        self._test_operation_resillient_to_ipallocation_failure(make_port)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"    def test_port_update_resillient_to_duplicate_records(self):"},{"line_number":797,"context_line":"        fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.2\u0027}]"},{"line_number":798,"context_line":"        with self.port(fixed_ips\u003dfixed_ip_data) as p:"},{"line_number":799,"context_line":"            data \u003d {\u0027port\u0027: {\u0027fixed_ips\u0027: [{\u0027ip_address\u0027: \u002710.0.0.9\u0027}]}}"},{"line_number":800,"context_line":"            req \u003d self.new_update_request(\u0027ports\u0027, data, p[\u0027port\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a061dce_86432b6c","line":797,"updated":"2016-04-08 21:37:38.000000000","message":"This is not right, the test should perform an \"ANY\" allocation, then find an unused IP to update the port with. Basically this is cheating :(","commit_id":"9ceab601e3cb2fa467339fb5a75e19ca04454bf3"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"27feff5dc36ac10517256b0d7a02af924ee0ccc9","unresolved":false,"context_lines":[{"line_number":522,"context_line":"    def test_update_port_fixed_ip_changed(self):"},{"line_number":523,"context_line":"        ctx \u003d context.get_admin_context()"},{"line_number":524,"context_line":"        plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":525,"context_line":"        fixed_ip_data \u003d [{\u0027ip_address\u0027: \u002710.0.0.4\u0027}]"},{"line_number":526,"context_line":"        with self.port(fixed_ips\u003dfixed_ip_data) as port,\\"},{"line_number":527,"context_line":"            mock.patch.object("},{"line_number":528,"context_line":"                plugin.notifier,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a061dce_e53adce6","line":525,"updated":"2016-04-11 19:56:20.000000000","message":"Allocate a specific IP to insulate this test so we can safely update the IP with a specific IP in this test without regard to the algorithm used for performing an \"ANY\" allocation.","commit_id":"9d4ece37ff1866cbf85f2068889c3d11e616e2a2"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"27feff5dc36ac10517256b0d7a02af924ee0ccc9","unresolved":false,"context_lines":[{"line_number":795,"context_line":""},{"line_number":796,"context_line":"    def test_port_update_resillient_to_duplicate_records(self):"},{"line_number":797,"context_line":"        cidr \u003d \u002710.0.0.0/24\u0027"},{"line_number":798,"context_line":"        allocation_pools \u003d [{\u0027start\u0027: \u002710.0.0.2\u0027, \u0027end\u0027: \u002710.0.0.8\u0027}]"},{"line_number":799,"context_line":"        with self.subnet(cidr\u003dcidr,"},{"line_number":800,"context_line":"                         allocation_pools\u003dallocation_pools) as subnet:"},{"line_number":801,"context_line":"            with self.port(subnet\u003dsubnet) as p:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a061dce_c548788a","line":798,"updated":"2016-04-11 19:56:20.000000000","message":"Using an allocation pool on the test subnet to insulate this test so that we can be sure the IP address used to update the port (see L802) will not be chosen by any allocation strategy and cause an unexpected conflict.","commit_id":"9d4ece37ff1866cbf85f2068889c3d11e616e2a2"}]}