)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Don\u0027t include the rules in the SG fetch in the metadata server, since"},{"line_number":10,"context_line":"we don\u0027t need them there, and with \u003e1000 rules, it starts to get"},{"line_number":11,"context_line":"really slow, especially in Pike and later."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I7de14456d04370c842b4c35597dca3a628a826a2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bfb3d3c7_4eff3567","line":12,"updated":"2019-05-17 18:35:29.000000000","message":"Do you want to report a bug for this for backporting purposes?","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Don\u0027t include the rules in the SG fetch in the metadata server, since"},{"line_number":10,"context_line":"we don\u0027t need them there, and with \u003e1000 rules, it starts to get"},{"line_number":11,"context_line":"really slow, especially in Pike and later."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I7de14456d04370c842b4c35597dca3a628a826a2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3fa7e38b_d3c75e46","line":12,"in_reply_to":"bfb3d3c7_4eff3567","updated":"2019-11-05 20:59:54.000000000","message":"I reported bug 1851430 for this.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"}],"nova/api/metadata/base.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        secgroup_api \u003d openstack_driver.get_openstack_security_group_driver()"},{"line_number":145,"context_line":"        self.security_groups \u003d secgroup_api.get_instance_security_groups("},{"line_number":146,"context_line":"            ctxt, instance, fields\u003d[\u0027id\u0027, \u0027name\u0027])"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        self.mappings \u003d _format_instance_mapping(ctxt, instance)"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_4e34154b","line":146,"range":{"start_line":146,"start_character":37,"end_line":146,"end_character":39},"updated":"2019-05-17 18:35:29.000000000","message":"Doesn\u0027t look like we use this?\n\n(later)\n\nOK I see why it\u0027s specified:\n\nhttps://review.opendev.org/#/c/656084/3/nova/network/security_group/neutron_driver.py@372\n\nThat\u0027s a bit tightly coupled for my test so I left a suggestion in the sec group API code, essentially making fields more like \u0027additional fields\u0027.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        fixed_ips \u003d self.ip_info[\u0027fixed_ips\u0027]"},{"line_number":249,"context_line":"        fixed_ip \u003d fixed_ips and fixed_ips[0] or \u0027\u0027"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        fmt_sgroups \u003d [x[\u0027name\u0027] for x in self.security_groups]"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        meta_data \u003d {"},{"line_number":254,"context_line":"            \u0027ami-id\u0027: self.instance.ec2_ids.ami_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_8e2a0de5","line":251,"range":{"start_line":251,"start_character":8,"end_line":251,"end_character":63},"updated":"2019-05-17 18:35:29.000000000","message":"Yup, looks like id isn\u0027t even used.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":6167,"context_line":"        self._refresh_instance_security_rules(context, instances)"},{"line_number":6168,"context_line":""},{"line_number":6169,"context_line":"    def get_instance_security_groups(self, context, instance, detailed\u003dFalse,"},{"line_number":6170,"context_line":"                                     fields\u003dNone):"},{"line_number":6171,"context_line":"        if detailed:"},{"line_number":6172,"context_line":"            return self.db.security_group_get_by_instance(context,"},{"line_number":6173,"context_line":"                                                          instance.uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_2e3be138","line":6170,"updated":"2019-05-17 18:35:29.000000000","message":"OK this is nova-network so we don\u0027t care.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"}],"nova/network/security_group/neutron_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        utils.check_string_length(value, name\u003dproperty, min_length\u003d0,"},{"line_number":98,"context_line":"                                  max_length\u003d255)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def _convert_to_nova_security_group_format(self, security_group):"},{"line_number":101,"context_line":"        nova_group \u003d {}"},{"line_number":102,"context_line":"        nova_group[\u0027id\u0027] \u003d security_group[\u0027id\u0027]"},{"line_number":103,"context_line":"        nova_group[\u0027description\u0027] \u003d security_group[\u0027description\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_b37982c6","line":100,"updated":"2019-11-05 20:59:54.000000000","message":"Yeah the stuff used in here might not be in the fields list of returned keys for security groups. So we have a few options I think:\n\n1. detailed\u003dTrue and fields are mutually exclusive, specifying both is an error\n\n2. detailed\u003dTrue overrides any fields specified\n\n3. detailed\u003dTrue means we add everything needed in this method to fields if not specified\n\n--\n\nOption 1 is gross since callers would have to handle the error.\n\nOption 3 tightly couples to what\u0027s used in this method.\n\nOption 2 is likely the best since if you ask for detailed\u003dTrue you shouldn\u0027t be using fields to narrow the results.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        security_groups \u003d {}"},{"line_number":366,"context_line":"        for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS - 1):"},{"line_number":367,"context_line":"            sg_search_opts \u003d {\u0027id\u0027: sg_id_list}"},{"line_number":368,"context_line":"            if fields is not None:"},{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_ae66f150","line":368,"range":{"start_line":368,"start_character":21,"end_line":368,"end_character":33},"updated":"2019-05-17 18:35:29.000000000","message":"nit: could just drop this so you don\u0027t accidentally set fields to [] if something passed an empty list.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        security_groups \u003d {}"},{"line_number":366,"context_line":"        for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS - 1):"},{"line_number":367,"context_line":"            sg_search_opts \u003d {\u0027id\u0027: sg_id_list}"},{"line_number":368,"context_line":"            if fields is not None:"},{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_f3c93a51","line":368,"range":{"start_line":368,"start_character":21,"end_line":368,"end_character":33},"in_reply_to":"bfb3d3c7_ae66f150","updated":"2019-11-05 20:59:54.000000000","message":"Done","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"},{"line_number":372,"context_line":"                security_groups[sg[\u0027id\u0027]] \u003d sg"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        return security_groups"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_4e5bb586","line":372,"updated":"2019-05-17 18:35:29.000000000","message":"OK this is why you need \u0027id\u0027 in the fields list. We could just always hard-code fields in here to be something like:\n\nsg_fields \u003d [\u0027id\u0027]\nif fields:\n  sg_fields.extend(fields)\n  sg_fields \u003d list(set(sg_fields))  # remove duplicates\n\nBut that\u0027s a nit. It\u0027s also something you\u0027d want to do outside the for loop.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"},{"line_number":372,"context_line":"                security_groups[sg[\u0027id\u0027]] \u003d sg"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        return security_groups"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_f3f29a9b","line":372,"in_reply_to":"bfb3d3c7_217d1db1","updated":"2019-11-05 20:59:54.000000000","message":"Actually I guess there is a bug in what I proposed. If we always request at a minimum fields\u003d[\u0027id\u0027] then anything that was calling this expecting the full response will be broken.\n\nSo it\u0027s more likely better to only request fields if specified and ensure id is in the set so the caller doesn\u0027t have to worry about passing id if they don\u0027t use it.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":10980,"name":"Doug Wiegley","email":"dougwig@parkside.io","username":"dougw"},"change_message_id":"1ecb53414ea57d1747b3f408906fe46c57d9224c","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"},{"line_number":372,"context_line":"                security_groups[sg[\u0027id\u0027]] \u003d sg"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        return security_groups"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_7471a6e0","line":372,"in_reply_to":"bfb3d3c7_4e5bb586","updated":"2019-05-17 20:46:03.000000000","message":"Which way do you prefer?  As long as we don\u0027t ask for the rules and hit its JOIN, the other fields are all fast.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":10980,"name":"Doug Wiegley","email":"dougwig@parkside.io","username":"dougw"},"change_message_id":"6ecec86f9bb2946c8e10968a877ce2eed907790c","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"},{"line_number":372,"context_line":"                security_groups[sg[\u0027id\u0027]] \u003d sg"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        return security_groups"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_217d1db1","line":372,"in_reply_to":"bfb3d3c7_5af60892","updated":"2019-05-20 14:42:14.000000000","message":"That sounds quite good, I\u0027ll change it that way. Thanks.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"089647099819b45743f27ded0e8be7fcf33e8ad3","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                sg_search_opts[\u0027fields\u0027] \u003d fields"},{"line_number":370,"context_line":"            search_results \u003d neutron.list_security_groups(**sg_search_opts)"},{"line_number":371,"context_line":"            for sg in search_results.get(\u0027security_groups\u0027):"},{"line_number":372,"context_line":"                security_groups[sg[\u0027id\u0027]] \u003d sg"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        return security_groups"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_5af60892","line":372,"in_reply_to":"bfb3d3c7_7471a6e0","updated":"2019-05-18 18:40:04.000000000","message":"I was mostly (but not super) concerned about the metadata API code needing to pass \u0027id\u0027 in the fields when it\u0027s not using the id, so it\u0027s tightly coupled to the inner workings of this method. So I\u0027d prefer that this code is smart enough to always request id and treat the incoming fields as additional but id will always be returned. So the metadata API code could just cleanly ask for \u0027name\u0027 and that\u0027s all it cares about.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":394,"context_line":"                # the port has an SG that this user doesn\u0027t have access to"},{"line_number":395,"context_line":"                port_sg \u003d security_groups.get(port_sg_id)"},{"line_number":396,"context_line":"                if port_sg:"},{"line_number":397,"context_line":"                    if detailed:"},{"line_number":398,"context_line":"                        sg_entry \u003d self._convert_to_nova_security_group_format("},{"line_number":399,"context_line":"                                 port_sg)"},{"line_number":400,"context_line":"                        instances_security_group_bindings.setdefault("},{"line_number":401,"context_line":"                            port[\u0027device_id\u0027], []).append(sg_entry)"},{"line_number":402,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_93032611","line":399,"range":{"start_line":397,"start_character":20,"end_line":399,"end_character":41},"updated":"2019-11-05 20:59:54.000000000","message":"So looking at this, if detailed is True, we call _convert_to_nova_security_group_format which is going to want more fields than what might be in the fields list.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1be151b217c5bc08189c2b01d23ef175d9cdd701","unresolved":false,"context_lines":[{"line_number":414,"context_line":"    def get_instance_security_groups(self, context, instance, detailed\u003dFalse,"},{"line_number":415,"context_line":"                                     fields\u003dNone):"},{"line_number":416,"context_line":"        \"\"\"Returns the security groups that are associated with an instance."},{"line_number":417,"context_line":"        If detailed is True then it also returns the full details of the"},{"line_number":418,"context_line":"        security groups associated with an instance."},{"line_number":419,"context_line":"        Fields allows limiting the returned data, as per the neutron SG API."},{"line_number":420,"context_line":"        \"\"\""},{"line_number":421,"context_line":"        servers \u003d [{\u0027id\u0027: instance.uuid}]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_53ba8e71","line":418,"range":{"start_line":417,"start_character":8,"end_line":418,"end_character":52},"updated":"2019-11-05 20:59:54.000000000","message":"This is a bit tricky if detailed\u003dTrue and fields is not None...\n\n(later)\n\nThe more I worked on this locally, and tried to deal with detailed and fields both being specified, the more I realized that \u0027name\u0027 has to also be specified in fields because of how get_instances_security_groups_bindings works in the non-detailed case.\n\nSo at a minimum if detailed\u003dFalse, fields should contain \u0027id\u0027 and \u0027name\u0027, and if we\u0027re going to do that, we might as well just drop the fields kwarg and say if detailed is False, just hard-code fields to be [\u0027id\u0027, \u0027name\u0027] since we won\u0027t use anything else. That makes this a much simpler fix and restricts it to just this module.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b7b61035e028cf81d2b25fc26c2e335c2fc6dd25","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                if port_sg:"},{"line_number":403,"context_line":"                    if detailed:"},{"line_number":404,"context_line":"                        sg_entry \u003d self._convert_to_nova_security_group_format("},{"line_number":405,"context_line":"                                 port_sg)"},{"line_number":406,"context_line":"                        instances_security_group_bindings.setdefault("},{"line_number":407,"context_line":"                            port[\u0027device_id\u0027], []).append(sg_entry)"},{"line_number":408,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_0a736dea","line":405,"updated":"2019-11-06 19:49:28.000000000","message":"Note to self: here\u0027s the convert method call.","commit_id":"eaf16fdde59a14fb38df669b21a911a0c2d2576f"}],"nova/tests/unit/api/openstack/compute/test_neutron_security_groups.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42a49eda32288ed21983f09dd095a9685ba28e66","unresolved":false,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    @mock.patch(\u0027nova.network.security_group.neutron_driver.SecurityGroupAPI.\u0027"},{"line_number":381,"context_line":"                \u0027get_instances_security_groups_bindings\u0027)"},{"line_number":382,"context_line":"    def test_get_security_group_empty_for_instance(self, neutron_sg_bind_mock):"},{"line_number":383,"context_line":"        servers \u003d [{\u0027id\u0027: test_security_groups.FAKE_UUID1}]"},{"line_number":384,"context_line":"        neutron_sg_bind_mock.return_value \u003d {}"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb3d3c7_0e55bd50","line":382,"updated":"2019-05-17 18:35:29.000000000","message":"You should have a new test that passes in fields and asserts they are passed through to the neutron python API client code.","commit_id":"ac397d50481cc16afe12ea15c4354d17c3aa30b6"}]}