)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Return security groups by name"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If we query the security groups by names, in case"},{"line_number":10,"context_line":"of severals occurences by name appear, it will raise"},{"line_number":11,"context_line":"a TypeError."},{"line_number":12,"context_line":"Simple return all the sg by name will avoid to raise"},{"line_number":13,"context_line":"and exception and let the caller handle the result"},{"line_number":14,"context_line":"like here:"},{"line_number":15,"context_line":"https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4105"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I84107c8f956f911ecf8c4e33af4ae17c8a1c6878"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5faad753_142e0b43","line":14,"range":{"start_line":9,"start_character":0,"end_line":14,"end_character":10},"updated":"2019-09-09 21:43:35.000000000","message":"This is kind of hard to follow, it doesn\u0027t really explain the problem or how this fixes it. I mean I know it says there is a TypeError and somehow this fixes it,","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Simple return all the sg by name will avoid to raise"},{"line_number":13,"context_line":"and exception and let the caller handle the result"},{"line_number":14,"context_line":"like here:"},{"line_number":15,"context_line":"https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4105"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I84107c8f956f911ecf8c4e33af4ae17c8a1c6878"},{"line_number":18,"context_line":"Related-Bug: #1824435"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5faad753_f4132f0c","line":15,"range":{"start_line":15,"start_character":39,"end_line":15,"end_character":45},"updated":"2019-09-09 21:43:35.000000000","message":"This should be a tag or commit hash because the line will change over time.","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":15,"context_line":"https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4105"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I84107c8f956f911ecf8c4e33af4ae17c8a1c6878"},{"line_number":18,"context_line":"Related-Bug: #1824435"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5faad753_14d36b3a","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":7},"updated":"2019-09-09 21:43:35.000000000","message":"Is it related or does it close the bug, because if it fixes the bug, this should be \"Closes-Bug\".","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":3953,"context_line":"            filter_by(project_id\u003dcontext.project_id).\\"},{"line_number":3954,"context_line":"            filter(models.SecurityGroup.name.in_(group_names))"},{"line_number":3955,"context_line":"    sg_models \u003d query.all()"},{"line_number":3956,"context_line":"    if len(sg_models) \u003d\u003d len(group_names):"},{"line_number":3957,"context_line":"        return sg_models"},{"line_number":3958,"context_line":"    # Find the first one missing and raise"},{"line_number":3959,"context_line":"    group_names_from_models \u003d [x.name for x in sg_models]"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_b41e170a","side":"PARENT","line":3956,"updated":"2019-09-09 21:43:35.000000000","message":"Why is this removed? Is there a case where this condition is True because both are empty?","commit_id":"90401df566b0925b5e96bcec38296de606c14ba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8c8f676408f64a08ccfff461ce096f0d6ee21eb5","unresolved":false,"context_lines":[{"line_number":3953,"context_line":"            filter_by(project_id\u003dcontext.project_id).\\"},{"line_number":3954,"context_line":"            filter(models.SecurityGroup.name.in_(group_names))"},{"line_number":3955,"context_line":"    sg_models \u003d query.all()"},{"line_number":3956,"context_line":"    if len(sg_models) \u003d\u003d len(group_names):"},{"line_number":3957,"context_line":"        return sg_models"},{"line_number":3958,"context_line":"    # Find the first one missing and raise"},{"line_number":3959,"context_line":"    group_names_from_models \u003d [x.name for x in sg_models]"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_ed54bd23","side":"PARENT","line":3956,"in_reply_to":"5faad753_b41e170a","updated":"2019-09-10 04:01:55.000000000","message":"From the bug report somehow the online data migration resulted in two \u0027default\u0027 legacy security group records being created for project_id\u003dNULL (bc of anonymous get_admin_context) and when queried here, two results were returned instead of the expected single result, causing this comparison to fail and the ironic reaching of \"# Not Reached\" and a TypeError when this method returned None instead of a list of models.\n\nFrom code inspection, I could *not* find how a single online data migration created two \u0027default\u0027 legacy security groups with project_id\u003dNULL, so I will need to do some local testing with devstack to figure that out. From the recreates described on the bug, the second duplicate security group is created from the first run of the online data migration, and it doesn\u0027t make sense bc the code is supposed to do a \"get and if not exist, create\". So I don\u0027t yet understand how a duplicate gets created.","commit_id":"90401df566b0925b5e96bcec38296de606c14ba1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6ca9a44fbd810d09a277507c9457b000511cc52c","unresolved":false,"context_lines":[{"line_number":3953,"context_line":"            filter_by(project_id\u003dcontext.project_id).\\"},{"line_number":3954,"context_line":"            filter(models.SecurityGroup.name.in_(group_names))"},{"line_number":3955,"context_line":"    sg_models \u003d query.all()"},{"line_number":3956,"context_line":"    if len(sg_models) \u003d\u003d len(group_names):"},{"line_number":3957,"context_line":"        return sg_models"},{"line_number":3958,"context_line":"    # Find the first one missing and raise"},{"line_number":3959,"context_line":"    group_names_from_models \u003d [x.name for x in sg_models]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4308bd90","side":"PARENT","line":3956,"in_reply_to":"5faad753_ed54bd23","updated":"2019-10-14 15:56:01.000000000","message":"Update: I investigated this and found (tl;dr) that two \u0027default\u0027 security groups with project_id\u003dNULL get created because we create \u0027default\u0027 group in a separate database transaction (using the \u0027independent\u0027 oslo.db decorator) and a subsequent call to \u0027get\u0027 the \u0027default\u0027 group back (in the instance_create method) will not \"see\" the group that was created earlier in the instance_create method because of it.\n\nI have proposed a different fix here to use the same \u0027get or create\u0027 method to \u0027get\u0027 the group in order to similarly use a separate transaction for getting it as was used to create it. This way, the \u0027get\u0027 will use a fresh view of the database and see the existing record, and not create a duplicate:\n\nhttps://review.opendev.org/688206","commit_id":"90401df566b0925b5e96bcec38296de606c14ba1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":1783,"context_line":""},{"line_number":1784,"context_line":"    def _get_sec_group_models(security_groups):"},{"line_number":1785,"context_line":"        models \u003d []"},{"line_number":1786,"context_line":"        default_group \u003d _security_group_ensure_default(context)"},{"line_number":1787,"context_line":"        if \u0027default\u0027 in security_groups:"},{"line_number":1788,"context_line":"            models.append(default_group)"},{"line_number":1789,"context_line":"            # Generate a new list, so we don\u0027t modify the original"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_f42c8f48","line":1786,"range":{"start_line":1786,"start_character":8,"end_line":1786,"end_character":21},"updated":"2019-09-09 21:43:35.000000000","message":"And then this is None and we blow up below?","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3a276feaf201dc1ef3712b3bfb9a5b830b582cae","unresolved":false,"context_lines":[{"line_number":3960,"context_line":"            raise exception.SecurityGroupNotFoundForProject("},{"line_number":3961,"context_line":"                project_id\u003dcontext.project_id, security_group_id\u003dgroup_name)"},{"line_number":3962,"context_line":"    # Return the security groups"},{"line_number":3963,"context_line":"    return sg_models"},{"line_number":3964,"context_line":""},{"line_number":3965,"context_line":""},{"line_number":3966,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_19b19be2","line":3963,"updated":"2019-09-09 21:09:31.000000000","message":"Note that a different workaround for this bug had been proposed before in this past:\n\nhttps://review.opendev.org/653065","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d3fbcf8df6b2a4404ac7e955ac96187319b173","unresolved":false,"context_lines":[{"line_number":4101,"context_line":"@pick_context_manager_writer"},{"line_number":4102,"context_line":"def _security_group_ensure_default(context):"},{"line_number":4103,"context_line":"    try:"},{"line_number":4104,"context_line":"        default_group \u003d _security_group_get_by_names(context, [\u0027default\u0027])[0]"},{"line_number":4105,"context_line":"    except exception.NotFound:"},{"line_number":4106,"context_line":"        values \u003d {\u0027name\u0027: \u0027default\u0027,"},{"line_number":4107,"context_line":"                  \u0027description\u0027: \u0027default\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_343307ae","line":4104,"updated":"2019-09-09 21:43:35.000000000","message":"So the bug is that _security_group_get_by_names returns None here, correct?","commit_id":"1679b4bb22e66886c8fa985db077774e8303e59b"}]}