)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"bbff5de4671b8c4bf67def69cf206ab81b0b4d73","unresolved":false,"context_lines":[{"line_number":17,"context_line":"the IP allocation table to get the IP addresses. The second directly"},{"line_number":18,"context_line":"joins the address pairs table to the security groups association"},{"line_number":19,"context_line":"table to get the IP addresses of the address pairs. In local testing"},{"line_number":20,"context_line":"of the method, this sped it up by an order of magnitude."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch also eliminates several other unnecessary loads of data"},{"line_number":23,"context_line":"from the ports table in the same module."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9aa7fdbe_a781754c","line":20,"updated":"2014-10-20 01:21:03.000000000","message":"this paragraph doesn\u0027t seem to match to the patch.","commit_id":"560a473cadbac265c875e051796d4b678f73f6ed"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"38b0bb8bb10d4789605790294043bfdc831362da","unresolved":false,"context_lines":[{"line_number":17,"context_line":"the IP allocation table to get the IP addresses. The second directly"},{"line_number":18,"context_line":"joins the address pairs table to the security groups association"},{"line_number":19,"context_line":"table to get the IP addresses of the address pairs. In local testing"},{"line_number":20,"context_line":"of the method, this sped it up by an order of magnitude."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch also eliminates several other unnecessary loads of data"},{"line_number":23,"context_line":"from the ports table in the same module."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9aa7fdbe_515e9ee7","line":20,"in_reply_to":"9aa7fdbe_86028e3c","updated":"2014-10-21 05:38:47.000000000","message":"Will update.","commit_id":"560a473cadbac265c875e051796d4b678f73f6ed"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"59a4a6fdec35ffda5cf261a6d2a0e5f9eea0b391","unresolved":false,"context_lines":[{"line_number":17,"context_line":"the IP allocation table to get the IP addresses. The second directly"},{"line_number":18,"context_line":"joins the address pairs table to the security groups association"},{"line_number":19,"context_line":"table to get the IP addresses of the address pairs. In local testing"},{"line_number":20,"context_line":"of the method, this sped it up by an order of magnitude."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch also eliminates several other unnecessary loads of data"},{"line_number":23,"context_line":"from the ports table in the same module."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9aa7fdbe_86028e3c","line":20,"in_reply_to":"9aa7fdbe_a781754c","updated":"2014-10-21 05:10:08.000000000","message":"He does have a point.  I forgot to recheck this after the change.","commit_id":"560a473cadbac265c875e051796d4b678f73f6ed"}],"neutron/db/securitygroups_rpc_base.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            return ips_by_group"},{"line_number":233,"context_line":"        for remote_group_id in remote_group_ids:"},{"line_number":234,"context_line":"            ips_by_group[remote_group_id] \u003d []"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        ip_port \u003d models_v2.IPAllocation.port_id"},{"line_number":237,"context_line":"        sg_binding_port \u003d sg_db.SecurityGroupPortBinding.port_id"},{"line_number":238,"context_line":"        sg_binding_sgid \u003d sg_db.SecurityGroupPortBinding.security_group_id"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_0775ba97","side":"PARENT","line":235,"updated":"2014-09-30 18:00:22.000000000","message":"This shouldn\u0027t be changed here.","commit_id":"a49f8e9ebfd30db5c2f9796beb22de758cd139c1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                   \u0027sg_member_ips\u0027: {}}"},{"line_number":155,"context_line":"        rules_in_db \u003d self._select_rules_for_ports(context, ports)"},{"line_number":156,"context_line":"        remote_security_group_info \u003d {}"},{"line_number":157,"context_line":"        for (port_id, rule_in_db) in rules_in_db:"},{"line_number":158,"context_line":"            remote_gid \u003d rule_in_db.get(\u0027remote_group_id\u0027)"},{"line_number":159,"context_line":"            security_group_id \u003d rule_in_db.get(\u0027security_group_id\u0027)"},{"line_number":160,"context_line":"            ethertype \u003d rule_in_db[\u0027ethertype\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_8777ead3","line":157,"updated":"2014-09-30 18:00:22.000000000","message":"Does this change (along with L224 and L427) stand alone from the other changes in this file?","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        sgr_sgid \u003d sg_db.SecurityGroupRule.security_group_id"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # NOTE: This query is called very frequently. Avoid joining to the"},{"line_number":223,"context_line":"        # port table."},{"line_number":224,"context_line":"        query \u003d context.session.query(sg_binding_port,"},{"line_number":225,"context_line":"                                      sg_db.SecurityGroupRule)"},{"line_number":226,"context_line":"        query \u003d query.join(sg_db.SecurityGroupRule,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a7c78e44","line":223,"updated":"2014-09-30 18:00:22.000000000","message":"nit:  This comment makes sense in the context of the change this patch makes.  I\u0027m not sure it makes as much sense to leave it in here.  Without the patch\u0027s context, it doesn\u0027t mean much.","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d33bfcd728a1aa0b7d7edfd84a811a21a9f46d2e","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        sgr_sgid \u003d sg_db.SecurityGroupRule.security_group_id"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # NOTE: This query is called very frequently. Avoid joining to the"},{"line_number":223,"context_line":"        # port table."},{"line_number":224,"context_line":"        query \u003d context.session.query(sg_binding_port,"},{"line_number":225,"context_line":"                                      sg_db.SecurityGroupRule)"},{"line_number":226,"context_line":"        query \u003d query.join(sg_db.SecurityGroupRule,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_c9488ece","line":223,"in_reply_to":"baa201ad_2cdc0146","updated":"2014-10-07 21:13:45.000000000","message":"The best way to prevent regression is with a test.  A comment might help.  If you decide to go with a comment maybe word it a bit differently to show that there is some history and be more explicit.  Maybe something like this:\n\n NOTE: This query is called very frequently.  An optimization was made to avoid inadvertently joining the port table by querying only SecurityGroupPortBinding.port_id instead of the entire table.  Don\u0027t undo this optimization.","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"dcf881c6a7e47ea47829e616e28dfaf56785a904","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        sgr_sgid \u003d sg_db.SecurityGroupRule.security_group_id"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # NOTE: This query is called very frequently. Avoid joining to the"},{"line_number":223,"context_line":"        # port table."},{"line_number":224,"context_line":"        query \u003d context.session.query(sg_binding_port,"},{"line_number":225,"context_line":"                                      sg_db.SecurityGroupRule)"},{"line_number":226,"context_line":"        query \u003d query.join(sg_db.SecurityGroupRule,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_2cdc0146","line":223,"in_reply_to":"baa201ad_a7c78e44","updated":"2014-10-05 09:46:49.000000000","message":"It\u0027s a note to prevent future developers from undoing this in a refactor. How do I do that without a comment?","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"dfdf2ab004b6c81d301eb48631ff91f2eb7c6d35","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        sgr_sgid \u003d sg_db.SecurityGroupRule.security_group_id"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # NOTE: This query is called very frequently. Avoid joining to the"},{"line_number":223,"context_line":"        # port table."},{"line_number":224,"context_line":"        query \u003d context.session.query(sg_binding_port,"},{"line_number":225,"context_line":"                                      sg_db.SecurityGroupRule)"},{"line_number":226,"context_line":"        query \u003d query.join(sg_db.SecurityGroupRule,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_8dbe8fac","line":223,"in_reply_to":"baa201ad_c9488ece","updated":"2014-10-07 21:44:41.000000000","message":"The test will be sort of ridiculous by asserting that specific tables are not passed into a query. Is that something we want in the code?","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        sg_binding_sgid \u003d sg_db.SecurityGroupPortBinding.security_group_id"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        # Join the security group binding table directly to the IP allocation"},{"line_number":242,"context_line":"        # table instead of via the Port table skip an unnecessary intermediary"},{"line_number":243,"context_line":"        query \u003d context.session.query(sg_binding_sgid, sg_binding_port,"},{"line_number":244,"context_line":"                                      models_v2.IPAllocation.ip_address)"},{"line_number":245,"context_line":"        query \u003d query.join(models_v2.IPAllocation,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_87b0ca8c","line":242,"updated":"2014-09-30 18:00:22.000000000","message":"ditto (L223)","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            addr_pair.AllowedAddressPair.ip_address)"},{"line_number":259,"context_line":"        query \u003d query.filter("},{"line_number":260,"context_line":"            addr_pair.AllowedAddressPair.port_id.in_(port_to_sg.keys()))"},{"line_number":261,"context_line":"        for port_id, ip_address in query:"},{"line_number":262,"context_line":"            ips_by_group[port_to_sg[port_id]].append(ip_address)"},{"line_number":263,"context_line":"        return ips_by_group"},{"line_number":264,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_b2b98a30","line":261,"updated":"2014-09-30 18:00:22.000000000","message":"The changes to this method seem to trade one query for two less complex queries.  Is the trade-off here well-understood?  I have a hard time thinking that the two queries will perform better than the one query they replace.  Could the two new queries be combined with a join (still avoiding models_v2.Port as intermediary)?","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d33bfcd728a1aa0b7d7edfd84a811a21a9f46d2e","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            addr_pair.AllowedAddressPair.ip_address)"},{"line_number":259,"context_line":"        query \u003d query.filter("},{"line_number":260,"context_line":"            addr_pair.AllowedAddressPair.port_id.in_(port_to_sg.keys()))"},{"line_number":261,"context_line":"        for port_id, ip_address in query:"},{"line_number":262,"context_line":"            ips_by_group[port_to_sg[port_id]].append(ip_address)"},{"line_number":263,"context_line":"        return ips_by_group"},{"line_number":264,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_4983becb","line":261,"in_reply_to":"baa201ad_4c13050d","updated":"2014-10-07 21:13:45.000000000","message":"Okay, fair enough.  Thanks for pointing out where the performance gain was measured.  That helps.\n\nI\u0027ll be interested in seeing if you end up with a joined address pairs table lookup.","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"dcf881c6a7e47ea47829e616e28dfaf56785a904","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            addr_pair.AllowedAddressPair.ip_address)"},{"line_number":259,"context_line":"        query \u003d query.filter("},{"line_number":260,"context_line":"            addr_pair.AllowedAddressPair.port_id.in_(port_to_sg.keys()))"},{"line_number":261,"context_line":"        for port_id, ip_address in query:"},{"line_number":262,"context_line":"            ips_by_group[port_to_sg[port_id]].append(ip_address)"},{"line_number":263,"context_line":"        return ips_by_group"},{"line_number":264,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_4c13050d","line":261,"in_reply_to":"baa201ad_b2b98a30","updated":"2014-10-05 09:46:49.000000000","message":"In addition to a multi-table join, we were asking the database for everything from the port table and then just throwing it away.This is the method that I mentioned in the commit message being an order of magnitude faster now.\n\n\nJoining to the address pairs table should work and will probably be a little faster. I will test and update.","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        if not network_ids:"},{"line_number":279,"context_line":"            return {}"},{"line_number":280,"context_line":"        query \u003d context.session.query(models_v2.Port.mac_address,"},{"line_number":281,"context_line":"                                      models_v2.Port.network_id,"},{"line_number":282,"context_line":"                                      models_v2.IPAllocation.ip_address)"},{"line_number":283,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":284,"context_line":"        query \u003d query.filter(models_v2.Port.network_id.in_(network_ids))"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_074bfa87","line":281,"updated":"2014-09-30 18:00:22.000000000","message":"Is this yet another change that could stand alone from other changes in this file?","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e75a42fc5e2df6691a891627e3c4aec5a108c135","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        return ips"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def _get_lla_gateway_ip_for_subnet(self, context, subnet):"},{"line_number":341,"context_line":"        query \u003d context.session.query(models_v2.Port.mac_address)"},{"line_number":342,"context_line":"        query \u003d query.join(models_v2.IPAllocation)"},{"line_number":343,"context_line":"        query \u003d query.filter("},{"line_number":344,"context_line":"            models_v2.IPAllocation.subnet_id \u003d\u003d subnet[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_b21d0a69","line":341,"updated":"2014-09-30 18:00:22.000000000","message":"ditto (L281)","commit_id":"7bfc356fb9375a7a6f8a7e9d218b74a7f91aa407"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4d874e982d9b1132f5502a232ade61bbba636a96","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            return ips_by_group"},{"line_number":233,"context_line":"        for remote_group_id in remote_group_ids:"},{"line_number":234,"context_line":"            ips_by_group[remote_group_id] \u003d []"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        ip_port \u003d models_v2.IPAllocation.port_id"},{"line_number":237,"context_line":"        sg_binding_port \u003d sg_db.SecurityGroupPortBinding.port_id"},{"line_number":238,"context_line":"        sg_binding_sgid \u003d sg_db.SecurityGroupPortBinding.security_group_id"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_9d522b11","side":"PARENT","line":235,"updated":"2014-10-17 20:41:55.000000000","message":":\u0027(","commit_id":"79586d3a26484ab9f62caa69a62b76e60bc3a206"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4d874e982d9b1132f5502a232ade61bbba636a96","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        ips_by_group \u003d {}"},{"line_number":232,"context_line":"        if not remote_group_ids:"},{"line_number":233,"context_line":"            return ips_by_group"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for remote_group_id in remote_group_ids:"},{"line_number":236,"context_line":"            ips_by_group[remote_group_id] \u003d []"},{"line_number":237,"context_line":"        ip_port \u003d models_v2.IPAllocation.port_id"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_7d4f476b","line":234,"updated":"2014-10-17 20:41:55.000000000","message":":\u0027(","commit_id":"4adfc37ba2a53c137ae7def7028b1298e43d6770"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"71eec46b9756f7060cae2288299c55c261d95c28","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        ips_by_group \u003d {}"},{"line_number":232,"context_line":"        if not remote_group_ids:"},{"line_number":233,"context_line":"            return ips_by_group"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for remote_group_id in remote_group_ids:"},{"line_number":236,"context_line":"            ips_by_group[remote_group_id] \u003d []"},{"line_number":237,"context_line":"        ip_port \u003d models_v2.IPAllocation.port_id"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_9b2bee2f","line":234,"in_reply_to":"baa201ad_7d4f476b","updated":"2014-10-19 09:50:52.000000000","message":"This is an emotional patch! :)","commit_id":"4adfc37ba2a53c137ae7def7028b1298e43d6770"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4d874e982d9b1132f5502a232ade61bbba636a96","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            # each allowed address pair IP record for a port beyond the 1st"},{"line_number":256,"context_line":"            # will have a duplicate regular IP in the query response since"},{"line_number":257,"context_line":"            # the relationship is 1-to-many. Check if in list to mitigate."},{"line_number":258,"context_line":"            if ip_address not in ips_by_group[security_group_id]:"},{"line_number":259,"context_line":"                ips_by_group[security_group_id].append(ip_address)"},{"line_number":260,"context_line":"            if allowed_addr_ip:"},{"line_number":261,"context_line":"                ips_by_group[security_group_id].append(allowed_addr_ip)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_5df5234f","line":258,"updated":"2014-10-17 20:41:55.000000000","message":"nit:  This isn\u0027t the best from an algorithm complexity standpoint but I don\u0027t see yet how this list could grow very large so maybe it doesn\u0027t matter.\n\nIf you look at the calling context the only thing expected of this list is that it is iterable.  Order doesn\u0027t seem to matter at first glance.  So, a set could be dropped in here in place of the list.  Of course, by drop in, I mean it would work in the calling context, you\u0027d still have to change the calls to append below.  You might consider it.","commit_id":"4adfc37ba2a53c137ae7def7028b1298e43d6770"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"71eec46b9756f7060cae2288299c55c261d95c28","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            # each allowed address pair IP record for a port beyond the 1st"},{"line_number":256,"context_line":"            # will have a duplicate regular IP in the query response since"},{"line_number":257,"context_line":"            # the relationship is 1-to-many. Check if in list to mitigate."},{"line_number":258,"context_line":"            if ip_address not in ips_by_group[security_group_id]:"},{"line_number":259,"context_line":"                ips_by_group[security_group_id].append(ip_address)"},{"line_number":260,"context_line":"            if allowed_addr_ip:"},{"line_number":261,"context_line":"                ips_by_group[security_group_id].append(allowed_addr_ip)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_fbf07acb","line":258,"in_reply_to":"baa201ad_5df5234f","updated":"2014-10-19 09:50:52.000000000","message":"Done","commit_id":"4adfc37ba2a53c137ae7def7028b1298e43d6770"}]}