)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7f718d364dd3942cfc55f0c07163f6b87eb79efc","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Allow only one IPv4 and one IPv6 fixed IP on router gateway port"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Validate a router\u0027s gateway port during a router update by ensuring"},{"line_number":10,"context_line":"it has no more than one v4 fixed IP and one v6 fixed IP."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I6a328048b99af39ab9497fd9f265d1a9b95b7148"},{"line_number":13,"context_line":"Closes-Bug: 1438819"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"9aa53dc9_658847e8","line":10,"updated":"2015-04-07 16:45:34.000000000","message":"This sounds good.  However, the patch does an extra step that is not right and that you don\u0027t mention here in the commit message.  It checks the number of subnets on the external network to make sure that there is only one of each.","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"14a2fd7e90933392fd215900d919c3959a7aa011","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Andrew Boik \u003cdboik@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-28 20:49:54 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow only one IPv4 and one IPv6 fixed IP on router gateway port"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Validate a router\u0027s gateway port during a router update by ensuring"},{"line_number":10,"context_line":"it has no more than one v4 fixed IP and one v6 fixed IP."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"3acd31a7_1b4a3c84","line":7,"updated":"2015-04-29 16:31:48.000000000","message":"if the public net has multiple v6 SLAAC subnets, setting the router gateway to this net will cause multiple ipv6 addresses allocated to the external gateway port. \n\nThe title should be changed, and the commit message should have a note on that.","commit_id":"fc728e634d1ac0f2489bf4e3e7cb38c587acbf96"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1de98540b193b1891fcbff377b3e4cedf559c178","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":1114,"context_line":"        if ip_address:"},{"line_number":1115,"context_line":"            return netaddr.IPAddress(ip_address).version"},{"line_number":1116,"context_line":"        return self._get_subnet(context, fip.get(\u0027subnet_id\u0027))[\u0027ip_version\u0027]"},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"    def _gw_port_exceeds_max_ips(self, context, fixed_ips):"},{"line_number":1119,"context_line":"        \"\"\"Check if router gateway port has more than one v4 and v6 address\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_8e9d60be","line":1116,"updated":"2015-04-15 16:30:44.000000000","message":"is it really possible that ip_address is None?","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"9a7fd43a078b2766a34c05ad2c5f37933efff74e","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":1114,"context_line":"        if ip_address:"},{"line_number":1115,"context_line":"            return netaddr.IPAddress(ip_address).version"},{"line_number":1116,"context_line":"        return self._get_subnet(context, fip.get(\u0027subnet_id\u0027))[\u0027ip_version\u0027]"},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"    def _gw_port_exceeds_max_ips(self, context, fixed_ips):"},{"line_number":1119,"context_line":"        \"\"\"Check if router gateway port has more than one v4 and v6 address\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"5aa745d1_9d0d233a","line":1116,"in_reply_to":"7aaa499b_8e9d60be","updated":"2015-04-15 19:24:05.000000000","message":"When the gateway port is updated from a subnet create there is no IP address at this point, so you have to look at the subnet\u0027s ip_version.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1109,"context_line":"                    pool\u003dpool_range,"},{"line_number":1110,"context_line":"                    ip_address\u003dgateway_ip)"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"    def _get_ip_version(self, subnets_dict, fip):"},{"line_number":1113,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":1114,"context_line":"        if ip_address:"},{"line_number":1115,"context_line":"            return netaddr.IPAddress(ip_address).version"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_f2afa17b","line":1112,"updated":"2015-04-28 20:49:32.000000000","message":"It seems like overkill to pass an entire dict full of subnets down to a method that should really be very simple.  It violates the Law of Demeter making the method more complex than it needs to be.\n\nUpdate:  See below (L1142), I think this may not be necessary.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"    def _get_ip_version(self, subnets_dict, fip):"},{"line_number":1113,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":1114,"context_line":"        if ip_address:"},{"line_number":1115,"context_line":"            return netaddr.IPAddress(ip_address).version"},{"line_number":1116,"context_line":"        return subnets_dict.get(fip.get(\u0027subnet_id\u0027))[\u0027ip_version\u0027]"},{"line_number":1117,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_6d1df065","line":1114,"updated":"2015-04-28 20:49:32.000000000","message":"Why wouldn\u0027t there be an ip_address?\n\nUpdate:  See below (L1142); I think this can be simplified.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"384e6da109bd0cfb3ca5ebbc20e87edf0eb61d24","unresolved":false,"context_lines":[{"line_number":1125,"context_line":"        return self._get_ip_version(subnets_dict, fixed_ips[0]) \u003d\u003d ("},{"line_number":1126,"context_line":"                self._get_ip_version(subnets_dict, fixed_ips[1]))"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"    def _update_router_gw_ports(self, context, subnet_id, network_id):"},{"line_number":1129,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":1130,"context_line":"                service_constants.L3_ROUTER_NAT)"},{"line_number":1131,"context_line":"        if not l3plugin:"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_b783f455","line":1128,"updated":"2015-04-24 15:52:35.000000000","message":"unrelated question (I won\u0027t -1 for this).\nwhat on earth is this method doing in db_base_plugin_v2?????","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1125,"context_line":"        return self._get_ip_version(subnets_dict, fixed_ips[0]) \u003d\u003d ("},{"line_number":1126,"context_line":"                self._get_ip_version(subnets_dict, fixed_ips[1]))"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"    def _update_router_gw_ports(self, context, subnet_id, network_id):"},{"line_number":1129,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":1130,"context_line":"                service_constants.L3_ROUTER_NAT)"},{"line_number":1131,"context_line":"        if not l3plugin:"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_324ab984","line":1128,"in_reply_to":"bab921fb_b783f455","updated":"2015-04-28 20:49:32.000000000","message":"+1  It doesn\u0027t seem to fit.  But, I agree that it shouldn\u0027t be addressed here.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1129,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":1130,"context_line":"                service_constants.L3_ROUTER_NAT)"},{"line_number":1131,"context_line":"        if not l3plugin:"},{"line_number":1132,"context_line":"            return"},{"line_number":1133,"context_line":"        gw_ports \u003d self._get_router_gw_ports_by_network(context,"},{"line_number":1134,"context_line":"                network_id)"},{"line_number":1135,"context_line":"        router_ids \u003d [p[\u0027device_id\u0027] for p in gw_ports]"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_cf59a190","line":1132,"updated":"2015-04-28 20:49:32.000000000","message":"Changing this muddies up the patch by forcing all of the indentation below to change.\n\nIn general, I do prefer the return fast approach that you\u0027ve changed to.  But, I also try to minimize the amount of change in a patch.  In this case, I\u0027d prefer to minimize the scope of the change in the patch and leave the old indentation.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"6f3a0c0f583054d5978c188973e6c9f920c0d4f8","unresolved":false,"context_lines":[{"line_number":1129,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":1130,"context_line":"                service_constants.L3_ROUTER_NAT)"},{"line_number":1131,"context_line":"        if not l3plugin:"},{"line_number":1132,"context_line":"            return"},{"line_number":1133,"context_line":"        gw_ports \u003d self._get_router_gw_ports_by_network(context,"},{"line_number":1134,"context_line":"                network_id)"},{"line_number":1135,"context_line":"        router_ids \u003d [p[\u0027device_id\u0027] for p in gw_ports]"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_b7547254","line":1132,"in_reply_to":"5ad0253b_cf59a190","updated":"2015-04-29 00:53:40.000000000","message":"Done","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        for id in router_ids:"},{"line_number":1140,"context_line":"            router \u003d l3plugin.get_router(ctx_admin, id)"},{"line_number":1141,"context_line":"            external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1142,"context_line":"            external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1143,"context_line":"                                         {\u0027subnet_id\u0027: subnet_id})"},{"line_number":1144,"context_line":"            if self._gw_port_exceeds_max_ips(subnets_dict,"},{"line_number":1145,"context_line":"                    external_gateway_info[\u0027external_fixed_ips\u0027]):"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_32c75914","line":1142,"updated":"2015-04-28 20:49:32.000000000","message":"This change should just grab the ip version of the new subnet (identified by subnet_id) and checked the external gateway to see if it already has an IP address of that type.  If not, append it (L1142) otherwise skip it.  The flow would be improved IMO and it looks like most of the complexity of this patch will be eliminated.\n\nAll existing fixed ips should have an address so _get_ip_version should not be necessary.  You should always be able to tell the version of the IP from the IP address itself.  That means you wouldn\u0027t have to have L1137/8 at all.  I think the whole thing gets much simpler.  Am I missing something?","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"6f3a0c0f583054d5978c188973e6c9f920c0d4f8","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        for id in router_ids:"},{"line_number":1140,"context_line":"            router \u003d l3plugin.get_router(ctx_admin, id)"},{"line_number":1141,"context_line":"            external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1142,"context_line":"            external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1143,"context_line":"                                         {\u0027subnet_id\u0027: subnet_id})"},{"line_number":1144,"context_line":"            if self._gw_port_exceeds_max_ips(subnets_dict,"},{"line_number":1145,"context_line":"                    external_gateway_info[\u0027external_fixed_ips\u0027]):"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_574bdeb0","line":1142,"in_reply_to":"5ad0253b_32c75914","updated":"2015-04-29 00:53:40.000000000","message":"Done. Thanks Carl, much cleaner.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"            external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1142,"context_line":"            external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1143,"context_line":"                                         {\u0027subnet_id\u0027: subnet_id})"},{"line_number":1144,"context_line":"            if self._gw_port_exceeds_max_ips(subnets_dict,"},{"line_number":1145,"context_line":"                    external_gateway_info[\u0027external_fixed_ips\u0027]):"},{"line_number":1146,"context_line":"                LOG.debug(\"Gateway port of router %s will not be updated. \""},{"line_number":1147,"context_line":"                          \"Port would exceed maximum of one IPv4 and one \""}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_54064521","line":1144,"updated":"2015-04-28 20:49:32.000000000","message":"This pattern seems a little funny to me.  It appends a fixed IP and then checks if that is okay.  If it isn\u0027t, it leaves it that way but doesn\u0027t notify the router.  I think this leads to a lot of complication.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":1143,"context_line":"                                         {\u0027subnet_id\u0027: subnet_id})"},{"line_number":1144,"context_line":"            if self._gw_port_exceeds_max_ips(subnets_dict,"},{"line_number":1145,"context_line":"                    external_gateway_info[\u0027external_fixed_ips\u0027]):"},{"line_number":1146,"context_line":"                LOG.debug(\"Gateway port of router %s will not be updated. \""},{"line_number":1147,"context_line":"                          \"Port would exceed maximum of one IPv4 and one \""},{"line_number":1148,"context_line":"                          \"IPv6 address.\" % id)"},{"line_number":1149,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_0da67cc1","line":1146,"updated":"2015-04-28 20:49:32.000000000","message":"I don\u0027t think this needs a log message at all.  This is normal operation.  It is normal (at least with ipv4) to have multiple subnets on the external network.  And, it is normal to only allocate one IPv4 address from them to the router.  I\u0027d rather not see any message in the logs.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"384e6da109bd0cfb3ca5ebbc20e87edf0eb61d24","unresolved":false,"context_lines":[{"line_number":1145,"context_line":"                    external_gateway_info[\u0027external_fixed_ips\u0027]):"},{"line_number":1146,"context_line":"                LOG.debug(\"Gateway port of router %s will not be updated. \""},{"line_number":1147,"context_line":"                          \"Port would exceed maximum of one IPv4 and one \""},{"line_number":1148,"context_line":"                          \"IPv6 address.\" % id)"},{"line_number":1149,"context_line":"                continue"},{"line_number":1150,"context_line":"            info \u003d {\u0027router\u0027: {\u0027external_gateway_info\u0027:"},{"line_number":1151,"context_line":"                external_gateway_info}}"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_7759dcfd","line":1148,"updated":"2015-04-24 15:52:35.000000000","message":"encapsulation is not done very well here imho. You have encapsulated the verification logic in its own method but with this log statement you are making an assumption on how _gw_port_exceeds_max_ips works.\n\nI am sorry for my pedantry - I spent all these words just to telly you that the log statement is better placed in the helper method. Also is probably deserves a WARN severity.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"2a8112e5d75c69b3eb10240d9979bf6169945cfa","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1159,"context_line":"                fips \u003d external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_063f43d7","line":1161,"updated":"2015-05-01 11:16:11.000000000","message":"Nit: may be clearer to read with:\n    if num_fips \u003e 1:\n        continue\n    if num_fips \u003d\u003d 1 and netaddr.IPAddress(...):\n        continue\n\nor you can consider the negative logic version (assuming nothing is done if no FIPs):\n    if num_fips \u003d\u003d 1 and netaddr.IPAddress(fips[0][\u0027ip_address\u0027]).version !\u003d subnet_ip_version:\n        external_gateway_info[...]...\n        ...","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c6d3b308813715ec182bd9a37c51e5cd95d10fb5","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1159,"context_line":"                fips \u003d external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_68223903","line":1161,"in_reply_to":"3acd31a7_063f43d7","updated":"2015-05-01 20:31:11.000000000","message":"I agree that this logic is still a bit too context sensitive and less straight-forward than it could be.  However, it has been cleaned up a lot from prior iterations.  That is great.\n\nI think good code reads as closely as possible to what it is trying to do.  My understanding is that this code wants to skip (i.e. continue) any router that already has a fixed ip of the right ip version.  The following code says that as closely as possible in python I think:\n\n  existing_fip \u003d next(f for f in fips\n                      if netaddr.IPAddress(f[\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version)\n  if existing_fip:\n      continue\n\nOr, getting fancier:\n\n  def is_same_ip_version(address):\n      return netaddr.IPAddress(address).version \u003d\u003d subnet_ip_version\n  existing_fip \u003d next(f for f in fips if is_same_ip_version(f[\u0027ip_address\u0027]))\n  if existing_fip:\n      continue\n\nThis minimizes what the code reader has to know to understand the code I think.","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"aaafaf5d8cd02dfba53c5cd2ce5623e6c2f9aa3a","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1159,"context_line":"                fips \u003d external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_119289f6","line":1161,"in_reply_to":"3acd31a7_063f43d7","updated":"2015-05-05 14:41:46.000000000","message":"I\u0027ve changed those lines to Paul\u0027s first suggestion as that reads the easiest to me. I also added a comment to explain it.","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6ff17b8a32372c16d5f9c7e7655ef3ccb992837f","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                external_gateway_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":1159,"context_line":"                fips \u003d external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_034e066f","line":1161,"in_reply_to":"3acd31a7_68223903","updated":"2015-05-01 20:35:26.000000000","message":"I realized that my code samples above don\u0027t quite work right.  You\u0027d need to give a default to avoid an exception:\n\n  next( (\u003citerable\u003e), None)","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":11343,"name":"Gal Sagie","email":"gal.sagie@gmail.com","username":"GalSagie"},"change_message_id":"4727af07db987c9bb781cd0ca52fda1bb0bac484","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"},{"line_number":1165,"context_line":"                external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1166,"context_line":"                                             {\u0027subnet_id\u0027: subnet_id})"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_225100d9","line":1163,"updated":"2015-05-01 18:49:58.000000000","message":"I would add a comment here, or re-write it as Paul suggested, its a bit hard to understand what this achieve without knowing the context\n\nAlso it seems like there is no feedback to the user but the configuration is just not happening (unless i am mistaken here)","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"aaafaf5d8cd02dfba53c5cd2ce5623e6c2f9aa3a","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                if num_fips \u003e 0:"},{"line_number":1162,"context_line":"                    if num_fips \u003e 1 or netaddr.IPAddress("},{"line_number":1163,"context_line":"                       fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"},{"line_number":1164,"context_line":"                        continue"},{"line_number":1165,"context_line":"                external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1166,"context_line":"                                             {\u0027subnet_id\u0027: subnet_id})"}],"source_content_type":"text/x-python","patch_set":24,"id":"3acd31a7_f1edcdb7","line":1163,"in_reply_to":"3acd31a7_225100d9","updated":"2015-05-05 14:41:46.000000000","message":"It is normal behavior for there to only be one address (per family) on the external port, so this does not generate any log message or message to the user. The user should expect this to happen, unlike the current behavior where IP addresses continue to be added after creating each external subnet.","commit_id":"282d3159c0a68062ba86a02757d35d193c6acc6a"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"5f7f06805378f9530319cd8e35fe3de5e41a1625","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1161,"context_line":"                # Don\u0027t add the fixed IP to the port if it already"},{"line_number":1162,"context_line":"                # has a fixed IP of the same IP version"},{"line_number":1163,"context_line":"                if num_fips \u003e 1:"},{"line_number":1164,"context_line":"                    continue"},{"line_number":1165,"context_line":"                if num_fips \u003d\u003d 1 and netaddr.IPAddress("},{"line_number":1166,"context_line":"                        fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet_ip_version:"}],"source_content_type":"text/x-python","patch_set":25,"id":"3acd31a7_a78037c1","line":1163,"updated":"2015-05-05 15:26:16.000000000","message":"It just occurred to me that this will not work in the case of SLAAC. For example, if you had a couple of SLAAC subnets. and now you\u0027d like to add v4 to it, it wouldn\u0027t be added due to this check.","commit_id":"f7f006c81c0f8e60253ec7c34fd7600b93a072c2"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"5a50e8a71676a466f18053c0c6813b2687b02ff8","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"            ctx_admin \u003d ctx.get_admin_context()"},{"line_number":1154,"context_line":"            subnet_ip_version \u003d self._get_subnet(context,"},{"line_number":1155,"context_line":"                                                 subnet_id)[\u0027ip_version\u0027]"},{"line_number":1156,"context_line":"            ext_subnets \u003d self._get_subnets_by_network(context, network_id)"},{"line_number":1157,"context_line":"            ext_subnets_dict \u003d {s[\u0027id\u0027]: s for s in ext_subnets}"},{"line_number":1158,"context_line":"            for id in router_ids:"},{"line_number":1159,"context_line":"                router \u003d l3plugin.get_router(ctx_admin, id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1aca2d91_ce74f4d0","line":1156,"updated":"2015-05-06 13:38:25.000000000","message":"One optimization is to directly pass in \u0027network\u0027 from the caller, and this DB query is no longer needed, and use \u0027network.subnets\u0027 instead.","commit_id":"88ee94cd1dd07bc70fe08db75930d68399a2d742"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"467046995d1828c3982c7084238401dafa1cff6d","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"            ctx_admin \u003d ctx.get_admin_context()"},{"line_number":1154,"context_line":"            subnet_ip_version \u003d self._get_subnet(context,"},{"line_number":1155,"context_line":"                                                 subnet_id)[\u0027ip_version\u0027]"},{"line_number":1156,"context_line":"            ext_subnets \u003d self._get_subnets_by_network(context, network_id)"},{"line_number":1157,"context_line":"            ext_subnets_dict \u003d {s[\u0027id\u0027]: s for s in ext_subnets}"},{"line_number":1158,"context_line":"            for id in router_ids:"},{"line_number":1159,"context_line":"                router \u003d l3plugin.get_router(ctx_admin, id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1aca2d91_f15e7144","line":1156,"in_reply_to":"1aca2d91_ce74f4d0","updated":"2015-05-06 13:57:36.000000000","message":"Oops, forgot the caller had the network object already. Thanks.","commit_id":"88ee94cd1dd07bc70fe08db75930d68399a2d742"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"fc1a7768f3554b86f4b9c812f9ee4a807a2aa8c7","unresolved":false,"context_lines":[{"line_number":1159,"context_line":"                fips \u003d [f for f in external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                        if not ipv6_utils.is_auto_address_subnet("},{"line_number":1161,"context_line":"                            ext_subnets_dict[f[\u0027subnet_id\u0027]])]"},{"line_number":1162,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1163,"context_line":"                # Don\u0027t add the fixed IP to the port if it already"},{"line_number":1164,"context_line":"                # has a stateful fixed IP of the same IP version"},{"line_number":1165,"context_line":"                if num_fips \u003e 1:"}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_4b4d243d","line":1162,"updated":"2015-05-08 06:33:20.000000000","message":"\u0027fips\u0027 is often used to indicate \"floating IPs\".\nNot something that you need to address, but perhaps this might confuse people reading this code.","commit_id":"1bfd86e1ef7148370798aa99c868d7f931fcbf78"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"8793cbab2908e24630190474c92721caa12ff2dd","unresolved":false,"context_lines":[{"line_number":1159,"context_line":"                fips \u003d [f for f in external_gateway_info[\u0027external_fixed_ips\u0027]"},{"line_number":1160,"context_line":"                        if not ipv6_utils.is_auto_address_subnet("},{"line_number":1161,"context_line":"                            ext_subnets_dict[f[\u0027subnet_id\u0027]])]"},{"line_number":1162,"context_line":"                num_fips \u003d len(fips)"},{"line_number":1163,"context_line":"                # Don\u0027t add the fixed IP to the port if it already"},{"line_number":1164,"context_line":"                # has a stateful fixed IP of the same IP version"},{"line_number":1165,"context_line":"                if num_fips \u003e 1:"}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_5c2848e2","line":1162,"in_reply_to":"1aca2d91_4b4d243d","updated":"2015-05-08 08:50:43.000000000","message":"+1","commit_id":"1bfd86e1ef7148370798aa99c868d7f931fcbf78"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"fc1a7768f3554b86f4b9c812f9ee4a807a2aa8c7","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"                    continue"},{"line_number":1167,"context_line":"                if num_fips \u003d\u003d 1 and netaddr.IPAddress("},{"line_number":1168,"context_line":"                        fips[0][\u0027ip_address\u0027]).version \u003d\u003d subnet[\u0027ip_version\u0027]:"},{"line_number":1169,"context_line":"                    continue"},{"line_number":1170,"context_line":"                external_gateway_info[\u0027external_fixed_ips\u0027].append("},{"line_number":1171,"context_line":"                                             {\u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":1172,"context_line":"                info \u003d {\u0027router\u0027: {\u0027external_gateway_info\u0027:"}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_8b59fc00","line":1169,"updated":"2015-05-08 06:33:20.000000000","message":"these two ifs could be brought together, but if you prefer it this way it\u0027s not a big deal","commit_id":"1bfd86e1ef7148370798aa99c868d7f931fcbf78"}],"neutron/db/l3_db.py":[{"author":{"_account_id":6587,"name":"Patrick Amor","email":"pamor@cisco.com","username":"pamor"},"change_message_id":"667f1c170ff42d1631a43b00c74125952580a1ff","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                if v4_exists:"},{"line_number":403,"context_line":"                    return False"},{"line_number":404,"context_line":"                v4_exists \u003d True"},{"line_number":405,"context_line":"            if ip_version \u003d\u003d 6:"},{"line_number":406,"context_line":"                if v6_exists:"},{"line_number":407,"context_line":"                    return False"},{"line_number":408,"context_line":"                v6_exists \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa963961_33099496","line":405,"updated":"2015-03-26 19:28:00.000000000","message":"Just a nit: This should be eilf since you are testing the same variable and the options 4 and 6 are exclusive. It could even be an else but this could but we don\u0027t yet support ipv8!","commit_id":"6146764db322425c2c40fb5c04b25365e6fb88c9"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"65067ad645af98dc0b24a101ac44fca1e5bb8992","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                if v4_exists:"},{"line_number":403,"context_line":"                    return False"},{"line_number":404,"context_line":"                v4_exists \u003d True"},{"line_number":405,"context_line":"            if ip_version \u003d\u003d 6:"},{"line_number":406,"context_line":"                if v6_exists:"},{"line_number":407,"context_line":"                    return False"},{"line_number":408,"context_line":"                v6_exists \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa963961_268385ce","line":405,"in_reply_to":"fa963961_33099496","updated":"2015-03-27 03:43:41.000000000","message":"Oops, missed that. Good catch. I think else would be fine - not sure we\u0027ll be needing IPv8 anytime soon :)","commit_id":"6146764db322425c2c40fb5c04b25365e6fb88c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"7b4126a86727731b0feb9e4a484cea72a204f86f","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            return True"},{"line_number":384,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":385,"context_line":"            return False"},{"line_number":386,"context_line":"        v4_exists \u003d False"},{"line_number":387,"context_line":"        v6_exists \u003d False"},{"line_number":388,"context_line":"        for fip in ext_ips:"},{"line_number":389,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_5e9f7103","line":386,"updated":"2015-03-27 07:08:43.000000000","message":"you can simplify things by replacing v4/6_exists by used_versions \u003d set()","commit_id":"121e62f8299583d57c6ca5ae9c970fc5c586a4f6"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"a23c4138b20ccc82a8f2f48404369d2a13d43dbe","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            return True"},{"line_number":384,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":385,"context_line":"            return False"},{"line_number":386,"context_line":"        v4_exists \u003d False"},{"line_number":387,"context_line":"        v6_exists \u003d False"},{"line_number":388,"context_line":"        for fip in ext_ips:"},{"line_number":389,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_6a794093","line":386,"in_reply_to":"fa963961_5e9f7103","updated":"2015-03-27 14:43:57.000000000","message":"good idea","commit_id":"121e62f8299583d57c6ca5ae9c970fc5c586a4f6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"7b4126a86727731b0feb9e4a484cea72a204f86f","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                msg \u003d _(\"External fixed IPs contain neither an IP address \""},{"line_number":399,"context_line":"                        \"nor a subnet id.\")"},{"line_number":400,"context_line":"                raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":401,"context_line":"            if ip_version \u003d\u003d 4:"},{"line_number":402,"context_line":"                if v4_exists:"},{"line_number":403,"context_line":"                    return False"},{"line_number":404,"context_line":"                v4_exists \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_3ebb8d74","line":401,"updated":"2015-03-27 07:08:43.000000000","message":"it becomes:\n\n  if ip_version in used_versions:\n    return False\n  used_versions.add(ip_version)","commit_id":"121e62f8299583d57c6ca5ae9c970fc5c586a4f6"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"a23c4138b20ccc82a8f2f48404369d2a13d43dbe","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                msg \u003d _(\"External fixed IPs contain neither an IP address \""},{"line_number":399,"context_line":"                        \"nor a subnet id.\")"},{"line_number":400,"context_line":"                raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":401,"context_line":"            if ip_version \u003d\u003d 4:"},{"line_number":402,"context_line":"                if v4_exists:"},{"line_number":403,"context_line":"                    return False"},{"line_number":404,"context_line":"                v4_exists \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_ca4bac58","line":401,"in_reply_to":"fa963961_3ebb8d74","updated":"2015-03-27 14:43:57.000000000","message":"Done","commit_id":"121e62f8299583d57c6ca5ae9c970fc5c586a4f6"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _validate_router_gw_ips(self, context, ext_ips):"},{"line_number":303,"context_line":"        # Router gateway port may contain an address from up to one IPv4"},{"line_number":304,"context_line":"        # subnet and one IPv6 subnet since there may only be one default"},{"line_number":305,"context_line":"        # route per address family for Neutron routers."},{"line_number":306,"context_line":"        if not ext_ips:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_2d9eb656","line":303,"updated":"2015-04-02 14:18:19.000000000","message":"Maybe more useful as a docstring?","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _validate_router_gw_ips(self, context, ext_ips):"},{"line_number":303,"context_line":"        # Router gateway port may contain an address from up to one IPv4"},{"line_number":304,"context_line":"        # subnet and one IPv6 subnet since there may only be one default"},{"line_number":305,"context_line":"        # route per address family for Neutron routers."},{"line_number":306,"context_line":"        if not ext_ips:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_2ebfcdfc","line":303,"in_reply_to":"baa041b7_2d9eb656","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            return True"},{"line_number":309,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":310,"context_line":"            return False"},{"line_number":311,"context_line":"        used_versions \u003d set()"},{"line_number":312,"context_line":"        for fip in ext_ips:"},{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_8874909c","line":311,"updated":"2015-04-02 14:18:19.000000000","message":"Could add another short cut...\n    if len(ext_ips) !\u003d 2:\n        return True","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            return True"},{"line_number":309,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":310,"context_line":"            return False"},{"line_number":311,"context_line":"        used_versions \u003d set()"},{"line_number":312,"context_line":"        for fip in ext_ips:"},{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_0ebc0904","line":311,"in_reply_to":"baa041b7_8874909c","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":310,"context_line":"            return False"},{"line_number":311,"context_line":"        used_versions \u003d set()"},{"line_number":312,"context_line":"        for fip in ext_ips:"},{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"},{"line_number":315,"context_line":"            if ip_address:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_0873e081","line":312,"updated":"2015-04-02 14:18:19.000000000","message":"Since there are only two entries, you could have a helper function that gets version from ip address or subnet, and then just compare:\n\n    def _get_ip_version(self, fip):\n        ip_address \u003d fip.get(\u0027ip_address\u0027)\n        if ip_address:\n            return netaddr.IPAddress(ip_address).version\n        subnet \u003d fip.get(\u0027subnet_id\u0027)\n        if subnet:\n            return self.core_plugin._get_subnet(context, subnet)[\u0027ip_version\u0027]\n        msg \u003d ...\n        raise ...\n\nFor use, do like mentioned for L340 and return comparison","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":310,"context_line":"            return False"},{"line_number":311,"context_line":"        used_versions \u003d set()"},{"line_number":312,"context_line":"        for fip in ext_ips:"},{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"},{"line_number":315,"context_line":"            if ip_address:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_0e740933","line":312,"in_reply_to":"baa041b7_0873e081","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"},{"line_number":315,"context_line":"            if ip_address:"},{"line_number":316,"context_line":"                ip_version \u003d 4 if netaddr.valid_ipv4(ip_address) else 6"},{"line_number":317,"context_line":"            elif subnet:"},{"line_number":318,"context_line":"                subnet \u003d self._core_plugin._get_subnet(context,"},{"line_number":319,"context_line":"                                                       fip[\u0027subnet_id\u0027])"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_4de8da7c","line":316,"updated":"2015-04-02 14:18:19.000000000","message":"could do:\n    ip_version \u003d netaddr.IPAddress(ip_address).version","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":314,"context_line":"            subnet \u003d fip.get(\u0027subnet_id\u0027)"},{"line_number":315,"context_line":"            if ip_address:"},{"line_number":316,"context_line":"                ip_version \u003d 4 if netaddr.valid_ipv4(ip_address) else 6"},{"line_number":317,"context_line":"            elif subnet:"},{"line_number":318,"context_line":"                subnet \u003d self._core_plugin._get_subnet(context,"},{"line_number":319,"context_line":"                                                       fip[\u0027subnet_id\u0027])"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_ae999d70","line":316,"in_reply_to":"baa041b7_4de8da7c","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        return True"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def _validate_external_network(self, context, network_id):"},{"line_number":331,"context_line":"        # When creating a gw port, we need to ensure that the external"},{"line_number":332,"context_line":"        # network has no more than one IPv4 and one IPv6 subnet."},{"line_number":333,"context_line":"        subnets \u003d self._core_plugin._get_subnets_by_network(context,"},{"line_number":334,"context_line":"                                                            network_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_8d63821a","line":331,"updated":"2015-04-02 14:18:19.000000000","message":"As docstring?","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        return True"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def _validate_external_network(self, context, network_id):"},{"line_number":331,"context_line":"        # When creating a gw port, we need to ensure that the external"},{"line_number":332,"context_line":"        # network has no more than one IPv4 and one IPv6 subnet."},{"line_number":333,"context_line":"        subnets \u003d self._core_plugin._get_subnets_by_network(context,"},{"line_number":334,"context_line":"                                                            network_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_8e9ed954","line":331,"in_reply_to":"baa041b7_8d63821a","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3366c23f91e3e8ab05abe42cc21bb9f0909a6ed1","unresolved":false,"context_lines":[{"line_number":337,"context_line":"            return True"},{"line_number":338,"context_line":"        if len(subnets) \u003e 2:"},{"line_number":339,"context_line":"            return False"},{"line_number":340,"context_line":"        used_versions \u003d set()"},{"line_number":341,"context_line":"        for s in subnets:"},{"line_number":342,"context_line":"            ip_version \u003d s[\u0027ip_version\u0027]"},{"line_number":343,"context_line":"            if ip_version in used_versions:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_c834d8df","line":340,"updated":"2015-04-02 14:18:19.000000000","message":"Could do...\n    if len(subnets) !\u003d 2:\n        return True\n    return subnets[0][\u0027ip_version\u0027] !\u003d subnets[1][\u0027ip_version\u0027]","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"3710f372116578b3f7712963e206f93ff6a96a55","unresolved":false,"context_lines":[{"line_number":337,"context_line":"            return True"},{"line_number":338,"context_line":"        if len(subnets) \u003e 2:"},{"line_number":339,"context_line":"            return False"},{"line_number":340,"context_line":"        used_versions \u003d set()"},{"line_number":341,"context_line":"        for s in subnets:"},{"line_number":342,"context_line":"            ip_version \u003d s[\u0027ip_version\u0027]"},{"line_number":343,"context_line":"            if ip_version in used_versions:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baa041b7_0e1e49ba","line":340,"in_reply_to":"baa041b7_c834d8df","updated":"2015-04-02 15:14:54.000000000","message":"Done","commit_id":"c496e5de691458ffff532445efad0dc590cf22a5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7f718d364dd3942cfc55f0c07163f6b87eb79efc","unresolved":false,"context_lines":[{"line_number":324,"context_line":"            return True"},{"line_number":325,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":326,"context_line":"            return False"},{"line_number":327,"context_line":"        if len(ext_ips) !\u003d 2:"},{"line_number":328,"context_line":"            return True"},{"line_number":329,"context_line":"        return self._get_ip_version(context, ext_ips[0]) !\u003d ("},{"line_number":330,"context_line":"                self._get_ip_version(context, ext_ips[1]))"}],"source_content_type":"text/x-python","patch_set":15,"id":"9aa53dc9_251ebf0c","line":327,"updated":"2015-04-07 16:45:34.000000000","message":"This is a funny way of saying len(ext_ips) \u003d\u003d 1.  I don\u0027t like this comparison because its meaning depends on the position in the method relative to other cases.\n\nThis method might be more straight-forward if the comparisons were a little more consistent and less position dependent.\n\n  number_of_ips \u003d len(ext_ips)\n  if number_of_ips \u003d\u003d 0 or number_of_ips \u003d\u003d 1:\n     return True\n  if number_of_ips \u003d\u003d 2:\n     return self._get_ip_version(context, ext_ips[0]) !\u003d (\n                self._get_ip_version(context, ext_ips[1]))\n  return False","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"fa8c0a2fabc1b35b72a8027d7889e2f2324e7b3d","unresolved":false,"context_lines":[{"line_number":324,"context_line":"            return True"},{"line_number":325,"context_line":"        if len(ext_ips) \u003e 2:"},{"line_number":326,"context_line":"            return False"},{"line_number":327,"context_line":"        if len(ext_ips) !\u003d 2:"},{"line_number":328,"context_line":"            return True"},{"line_number":329,"context_line":"        return self._get_ip_version(context, ext_ips[0]) !\u003d ("},{"line_number":330,"context_line":"                self._get_ip_version(context, ext_ips[1]))"}],"source_content_type":"text/x-python","patch_set":15,"id":"9aa53dc9_63dea3de","line":327,"in_reply_to":"9aa53dc9_251ebf0c","updated":"2015-04-09 18:52:33.000000000","message":"I changed it a little bit. See if you like the next version better.","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7f718d364dd3942cfc55f0c07163f6b87eb79efc","unresolved":false,"context_lines":[{"line_number":345,"context_line":"        if len(subnets) \u003e 2:"},{"line_number":346,"context_line":"            return False"},{"line_number":347,"context_line":"        if len(subnets) !\u003d 2:"},{"line_number":348,"context_line":"            return True"},{"line_number":349,"context_line":"        return subnets[0][\u0027ip_version\u0027] !\u003d subnets[1][\u0027ip_version\u0027]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _validate_gw_info(self, context, gw_port, info, ext_ips):"}],"source_content_type":"text/x-python","patch_set":15,"id":"9aa53dc9_a5bf6fcf","line":348,"updated":"2015-04-07 16:45:34.000000000","message":"Comment from L327 should be applied here except that I think that this check should not be done.  So, the point is moot.  Just remove this method.","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7f718d364dd3942cfc55f0c07163f6b87eb79efc","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                                    \"gateway IP\") % ext_ip.get(\u0027ip_address\u0027)"},{"line_number":372,"context_line":"                            raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":373,"context_line":"            elif not self._validate_external_network(context, network_id):"},{"line_number":374,"context_line":"                msg \u003d _(\"Cannot have more than one IPv4 and one IPv6 \""},{"line_number":375,"context_line":"                        \"subnet on the network on which the router \""},{"line_number":376,"context_line":"                        \"gateway is set.\")"},{"line_number":377,"context_line":"                raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9aa53dc9_e5efb7ae","line":374,"updated":"2015-04-07 16:45:34.000000000","message":"This is not an invalid scenario that you\u0027re trying to prevent.  It is valid and supported to connect a router gateway to an external network with more than one ipv4 or ipv6 subnet!","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"f6ffca7b8ddd0fd471febcdfbeec974ed18dd996","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                                    \"gateway IP\") % ext_ip.get(\u0027ip_address\u0027)"},{"line_number":372,"context_line":"                            raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":373,"context_line":"            elif not self._validate_external_network(context, network_id):"},{"line_number":374,"context_line":"                msg \u003d _(\"Cannot have more than one IPv4 and one IPv6 \""},{"line_number":375,"context_line":"                        \"subnet on the network on which the router \""},{"line_number":376,"context_line":"                        \"gateway is set.\")"},{"line_number":377,"context_line":"                raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9aa53dc9_0f85d505","line":374,"in_reply_to":"9aa53dc9_e5efb7ae","updated":"2015-04-09 01:06:37.000000000","message":"Hmm, ok. I\u0027m going to have to go back and change some of the code that automatically updates the IP addresses on the router when an external subnet is created then. Without that, even if I remove _validate_external_network(), _validate_router_gw_ips() will prevent new subnets from being added because the auto-update code will try to add a new IP address to the router, which will fail, and so the subnet creation will also fail.\n\nThe router-gateway-set API is a little awkward when multiple external subnets are allowed because you cannot choose the subnet that is associated with the router port - it\u0027s essentially random (well, based on whichever subnet has the lowest random UUID). This has implications for the default route.\n\nI\u0027ll remove this restriction from the patch and fix the auto-update, but I think the router-gateway-set API should be enhanced to allow subnet-specification.","commit_id":"45c62976a93a1b730cfc65d313bd0c7770f38f6a"},{"author":{"_account_id":704,"name":"Edgar Magana","email":"emagana@gmail.com","username":"emagana"},"change_message_id":"8e834fd7bf15609a1db4b233fb9194536db5a6eb","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            context.session.add(router)"},{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_ip_version(self, context, fip):"},{"line_number":303,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":304,"context_line":"        if ip_address:"},{"line_number":305,"context_line":"            return netaddr.IPAddress(ip_address).version"}],"source_content_type":"text/x-python","patch_set":16,"id":"7aaa499b_d09c45c2","line":302,"updated":"2015-04-14 22:45:15.000000000","message":"we should get a unit test for this helper as well","commit_id":"575a55b8a6624a374edfbfe87b1c5bb7d250d720"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"95704bdc0dc04751ca6fdf5064a4122d0b6f3915","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            context.session.add(router)"},{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_ip_version(self, context, fip):"},{"line_number":303,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":304,"context_line":"        if ip_address:"},{"line_number":305,"context_line":"            return netaddr.IPAddress(ip_address).version"}],"source_content_type":"text/x-python","patch_set":16,"id":"7aaa499b_cb8eba78","line":302,"in_reply_to":"7aaa499b_d09c45c2","updated":"2015-04-15 15:52:18.000000000","message":"Done","commit_id":"575a55b8a6624a374edfbfe87b1c5bb7d250d720"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"de1f88325de471b2a67053588016589a76b29175","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                    \"nor a subnet id.\")"},{"line_number":313,"context_line":"            raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def _validate_router_gw_ips(self, context, ext_ips):"},{"line_number":316,"context_line":"        \"\"\"Ensure gateway port does not exceed IP limit"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        Router gateway port may contain an address from up to one IPv4"}],"source_content_type":"text/x-python","patch_set":16,"id":"9aa53dc9_158009bf","line":315,"updated":"2015-04-10 12:30:29.000000000","message":"Is it possible to remove/reduce the duplication here and with  _gw_port_exceeds_max_ips()?","commit_id":"575a55b8a6624a374edfbfe87b1c5bb7d250d720"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e7300c1086cac649e3a611bb094e6381e927d379","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                    \"nor a subnet id.\")"},{"line_number":313,"context_line":"            raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def _validate_router_gw_ips(self, context, ext_ips):"},{"line_number":316,"context_line":"        \"\"\"Ensure gateway port does not exceed IP limit"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        Router gateway port may contain an address from up to one IPv4"}],"source_content_type":"text/x-python","patch_set":16,"id":"9aa53dc9_6da42907","line":315,"in_reply_to":"9aa53dc9_158009bf","updated":"2015-04-10 14:26:44.000000000","message":"After looking further and discussing with Drew. I makes sense to deal with clean up as a separate commit, refactoring out common code and moving some code as well.","commit_id":"575a55b8a6624a374edfbfe87b1c5bb7d250d720"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1de98540b193b1891fcbff377b3e4cedf559c178","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            context.session.add(router)"},{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_ip_version(self, context, fip):"},{"line_number":303,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":304,"context_line":"        if ip_address:"},{"line_number":305,"context_line":"            return netaddr.IPAddress(ip_address).version"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_6ed7b44b","line":302,"updated":"2015-04-15 16:30:44.000000000","message":"This point was already raised by Paul, I really feel that we should avoid code duplication and move _get_ip_version and _validate_router_gw_ips in a common place where they can be used by both this class and by NeutronDbPluginV2 . It doesn\u0027t seem hard to do.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"9a7fd43a078b2766a34c05ad2c5f37933efff74e","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            context.session.add(router)"},{"line_number":300,"context_line":"            context.session.add(router_port)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_ip_version(self, context, fip):"},{"line_number":303,"context_line":"        ip_address \u003d fip.get(\u0027ip_address\u0027)"},{"line_number":304,"context_line":"        if ip_address:"},{"line_number":305,"context_line":"            return netaddr.IPAddress(ip_address).version"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_84f53f58","line":302,"in_reply_to":"7aaa499b_6ed7b44b","updated":"2015-04-15 19:24:05.000000000","message":"Yes, I agree. I\u0027m going to remove these methods and just call the ones in NeutronDbPluginV2.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1de98540b193b1891fcbff377b3e4cedf559c178","unresolved":false,"context_lines":[{"line_number":339,"context_line":"                    msg \u003d _(\"Routers only support up to one external IPv4 \""},{"line_number":340,"context_line":"                            \"address and one external IPv6 address\")"},{"line_number":341,"context_line":"                    raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":342,"context_line":"                subnets \u003d self._core_plugin._get_subnets_by_network(context,"},{"line_number":343,"context_line":"                                                                    network_id)"},{"line_number":344,"context_line":"                for s in subnets:"},{"line_number":345,"context_line":"                    if not s[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_0e7f1016","line":342,"updated":"2015-04-15 16:30:44.000000000","message":"if you swap this operation with \u0027self._validate_router_gw_ips\u0027 you can pass the subnets as parameter to _validate_router_gw_ips . This has 2 benefits: avoiding one extra call to the db and avoiding passing the context around when there\u0027s no need.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"9a7fd43a078b2766a34c05ad2c5f37933efff74e","unresolved":false,"context_lines":[{"line_number":339,"context_line":"                    msg \u003d _(\"Routers only support up to one external IPv4 \""},{"line_number":340,"context_line":"                            \"address and one external IPv6 address\")"},{"line_number":341,"context_line":"                    raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":342,"context_line":"                subnets \u003d self._core_plugin._get_subnets_by_network(context,"},{"line_number":343,"context_line":"                                                                    network_id)"},{"line_number":344,"context_line":"                for s in subnets:"},{"line_number":345,"context_line":"                    if not s[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5aa745d1_83af02e3","line":342,"in_reply_to":"7aaa499b_0e7f1016","updated":"2015-04-15 19:24:05.000000000","message":"Sounds like a good idea. The problem is worse in the other file as it is making a db call to get a subnet for each router. Calling _get_subnets_by_network there as well would be better.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"60f47b47adccfa3a47ca04e1d58add4994fc3314","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                        subnets_dict, ext_ips):"},{"line_number":315,"context_line":"                    msg \u003d _(\"Routers only support up to one external IPv4 \""},{"line_number":316,"context_line":"                            \"address and one external IPv6 address\")"},{"line_number":317,"context_line":"                    raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":318,"context_line":"                for s in subnets:"},{"line_number":319,"context_line":"                    if not s[\u0027gateway_ip\u0027]:"},{"line_number":320,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_483102ec","line":317,"updated":"2015-04-28 20:49:32.000000000","message":"I can\u0027t think of what case this is meant to guard against.  Can you describe the scenario that leads to this exception?","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"6f3a0c0f583054d5978c188973e6c9f920c0d4f8","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                        subnets_dict, ext_ips):"},{"line_number":315,"context_line":"                    msg \u003d _(\"Routers only support up to one external IPv4 \""},{"line_number":316,"context_line":"                            \"address and one external IPv6 address\")"},{"line_number":317,"context_line":"                    raise n_exc.BadRequest(resource\u003d\u0027router\u0027, msg\u003dmsg)"},{"line_number":318,"context_line":"                for s in subnets:"},{"line_number":319,"context_line":"                    if not s[\u0027gateway_ip\u0027]:"},{"line_number":320,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad0253b_179a0615","line":317,"in_reply_to":"5ad0253b_483102ec","updated":"2015-04-29 00:53:40.000000000","message":"Not aware of one currently - this is kind of vestigial from earlier patch sets. We would need it in the future, I think, if the gateway interface could be updated explicitly.","commit_id":"100268301712a66d28b86a0241645536fc0c5e78"}],"neutron/tests/unit/extensions/test_l3.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1de98540b193b1891fcbff377b3e4cedf559c178","unresolved":false,"context_lines":[{"line_number":1452,"context_line":"                fips \u003d (res[\u0027router\u0027][\u0027external_gateway_info\u0027]"},{"line_number":1453,"context_line":"                        [\u0027external_fixed_ips\u0027])"},{"line_number":1454,"context_line":"                fip_subnet_ids \u003d [fip[\u0027subnet_id\u0027] for fip in fips]"},{"line_number":1455,"context_line":"                self.assertIn(s1[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1456,"context_line":"                self.assertNotIn(s2[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1457,"context_line":"                self.assertIn(s3[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1458,"context_line":"                self.assertNotIn(s4[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_4eae3882","line":1455,"updated":"2015-04-15 16:30:44.000000000","message":"using one assertEqual instead of several assertIn and assertNotIn would spare some line","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"9a7fd43a078b2766a34c05ad2c5f37933efff74e","unresolved":false,"context_lines":[{"line_number":1452,"context_line":"                fips \u003d (res[\u0027router\u0027][\u0027external_gateway_info\u0027]"},{"line_number":1453,"context_line":"                        [\u0027external_fixed_ips\u0027])"},{"line_number":1454,"context_line":"                fip_subnet_ids \u003d [fip[\u0027subnet_id\u0027] for fip in fips]"},{"line_number":1455,"context_line":"                self.assertIn(s1[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1456,"context_line":"                self.assertNotIn(s2[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1457,"context_line":"                self.assertIn(s3[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"},{"line_number":1458,"context_line":"                self.assertNotIn(s4[\u0027subnet\u0027][\u0027id\u0027], fip_subnet_ids)"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_84ce5f2c","line":1455,"in_reply_to":"7aaa499b_4eae3882","updated":"2015-04-15 19:24:05.000000000","message":"Will do. I assumed the order was random (ordered by subnet uuid) which is why I used assertIn.","commit_id":"02a6a1d092650bfd643873996818df1cc67a2d31"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"2bc28c6df729756dff44ddf1e2dc28977e3c59f0","unresolved":false,"context_lines":[{"line_number":883,"context_line":"                self.assertNotEqual(fip1[\u0027subnet_id\u0027], fip2[\u0027subnet_id\u0027])"},{"line_number":884,"context_line":"                self.assertNotEqual(fip1[\u0027ip_address\u0027], fip2[\u0027ip_address\u0027])"},{"line_number":885,"context_line":""},{"line_number":886,"context_line":"    def test_router_update_gateway_upon_subnet_create_max_ips_ipv6(self):"},{"line_number":887,"context_line":"        \"\"\"Create subnet should not cause excess fixed IPs on router gw"},{"line_number":888,"context_line":""},{"line_number":889,"context_line":"        If a router gateway port has the maximum of one IPv4 and one IPv6"}],"source_content_type":"text/x-python","patch_set":28,"id":"1aca2d91_acbc136d","line":886,"updated":"2015-05-06 19:28:12.000000000","message":"Nit: The test method name has \"ipv6\" but it\u0027s a v4 and v6 test case. Should the name be changed to indicate it\u0027s testing both, or is \"ipv6\" there to as a flag to indicate that v6 is involved? (Same comment for other test method that\u0027s being added.)","commit_id":"7e235c9b5d479c9663f0472f4fadce2c0cd8bc28"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"4f2b7513ce84b5df7af65e8d5bc73584afc5e91f","unresolved":false,"context_lines":[{"line_number":883,"context_line":"                self.assertNotEqual(fip1[\u0027subnet_id\u0027], fip2[\u0027subnet_id\u0027])"},{"line_number":884,"context_line":"                self.assertNotEqual(fip1[\u0027ip_address\u0027], fip2[\u0027ip_address\u0027])"},{"line_number":885,"context_line":""},{"line_number":886,"context_line":"    def test_router_update_gateway_upon_subnet_create_max_ips_ipv6(self):"},{"line_number":887,"context_line":"        \"\"\"Create subnet should not cause excess fixed IPs on router gw"},{"line_number":888,"context_line":""},{"line_number":889,"context_line":"        If a router gateway port has the maximum of one IPv4 and one IPv6"}],"source_content_type":"text/x-python","patch_set":28,"id":"1aca2d91_5abd0c67","line":886,"in_reply_to":"1aca2d91_acbc136d","updated":"2015-05-07 14:21:58.000000000","message":"The \u0027ipv6\u0027 is there because some plugins that don\u0027t support ipv6 scan for \u0027ipv6\u0027 in the test name so that they can skip the test.","commit_id":"7e235c9b5d479c9663f0472f4fadce2c0cd8bc28"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"2bc28c6df729756dff44ddf1e2dc28977e3c59f0","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"                 self.subnet(cidr\u003d\u002710.0.1.0/24\u0027, network\u003dn)), ("},{"line_number":1415,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:1::/64\u0027, network\u003dn,"},{"line_number":1416,"context_line":"                             ip_version\u003d6)) as s3, ("},{"line_number":1417,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:2::/64\u0027, network\u003dn,"},{"line_number":1418,"context_line":"                             ip_version\u003d6)):"},{"line_number":1419,"context_line":"                self._set_net_external(n[\u0027network\u0027][\u0027id\u0027])"},{"line_number":1420,"context_line":"                self._add_external_gateway_to_router("}],"source_content_type":"text/x-python","patch_set":28,"id":"1aca2d91_200ec734","line":1417,"updated":"2015-05-06 19:28:12.000000000","message":"It probably makes sense to enhance this test by adding a couple of ipv6-stateless subnets along with the ipv4 and ipv6-stateful subnets, to test the ipv6-stateless awareness in the gateway update. (Resulting gateway should have one ipv4, one ipv6-stateful, and two ipv6-stateless fixed IPs.)","commit_id":"7e235c9b5d479c9663f0472f4fadce2c0cd8bc28"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6288634f5ab210fc6e418e515cebdaff56827e4e","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"                 self.subnet(cidr\u003d\u002710.0.1.0/24\u0027, network\u003dn)), ("},{"line_number":1415,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:1::/64\u0027, network\u003dn,"},{"line_number":1416,"context_line":"                             ip_version\u003d6)) as s3, ("},{"line_number":1417,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:2::/64\u0027, network\u003dn,"},{"line_number":1418,"context_line":"                             ip_version\u003d6)):"},{"line_number":1419,"context_line":"                self._set_net_external(n[\u0027network\u0027][\u0027id\u0027])"},{"line_number":1420,"context_line":"                self._add_external_gateway_to_router("}],"source_content_type":"text/x-python","patch_set":28,"id":"1aca2d91_04b184e1","line":1417,"in_reply_to":"1aca2d91_200ec734","updated":"2015-05-07 11:12:07.000000000","message":"+1 good idea","commit_id":"7e235c9b5d479c9663f0472f4fadce2c0cd8bc28"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"4f2b7513ce84b5df7af65e8d5bc73584afc5e91f","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"                 self.subnet(cidr\u003d\u002710.0.1.0/24\u0027, network\u003dn)), ("},{"line_number":1415,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:1::/64\u0027, network\u003dn,"},{"line_number":1416,"context_line":"                             ip_version\u003d6)) as s3, ("},{"line_number":1417,"context_line":"                 self.subnet(cidr\u003d\u00272001:db8:2::/64\u0027, network\u003dn,"},{"line_number":1418,"context_line":"                             ip_version\u003d6)):"},{"line_number":1419,"context_line":"                self._set_net_external(n[\u0027network\u0027][\u0027id\u0027])"},{"line_number":1420,"context_line":"                self._add_external_gateway_to_router("}],"source_content_type":"text/x-python","patch_set":28,"id":"1aca2d91_182afc2c","line":1417,"in_reply_to":"1aca2d91_200ec734","updated":"2015-05-07 14:21:58.000000000","message":"Done","commit_id":"7e235c9b5d479c9663f0472f4fadce2c0cd8bc28"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"ef5ec7e2747b0abb618a5a7030030f862d1211da","unresolved":false,"context_lines":[{"line_number":1406,"context_line":""},{"line_number":1407,"context_line":"        Setting the gateway of a router to an external network with more than"},{"line_number":1408,"context_line":"        one IPv4 and one IPv6 subnet should only add an address from the first"},{"line_number":1409,"context_line":"        IPv4 subnet and an address from the first IPv6 subnet."},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        \"\"\""},{"line_number":1412,"context_line":"        with self.router() as r, self.network() as n:"}],"source_content_type":"text/x-python","patch_set":29,"id":"1aca2d91_62ac9635","line":1409,"updated":"2015-05-07 18:42:18.000000000","message":"The doc string should be updated to explain that the gateway should have assigned addresses from one IPv4, one IPv6 stateful, and all IPv6 stateless subnets.","commit_id":"56188c839fd5e3f00243a1b6dc21b15e69bb553a"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"563a9607f3b1a05ee8c74e2492db158c09f3cfbf","unresolved":false,"context_lines":[{"line_number":1406,"context_line":""},{"line_number":1407,"context_line":"        Setting the gateway of a router to an external network with more than"},{"line_number":1408,"context_line":"        one IPv4 and one IPv6 subnet should only add an address from the first"},{"line_number":1409,"context_line":"        IPv4 subnet and an address from the first IPv6 subnet."},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        \"\"\""},{"line_number":1412,"context_line":"        with self.router() as r, self.network() as n:"}],"source_content_type":"text/x-python","patch_set":29,"id":"1aca2d91_59ed0cee","line":1409,"in_reply_to":"1aca2d91_62ac9635","updated":"2015-05-07 19:17:49.000000000","message":"whoops :) good catch","commit_id":"56188c839fd5e3f00243a1b6dc21b15e69bb553a"}]}