)]}'
{"nova/cmd/manage.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"e2e5abeee593d1d239775477a55c2e6b1595eef5","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        flavor_obj.migrate_flavors,"},{"line_number":728,"context_line":"        flavor_obj.migrate_flavor_reset_autoincrement,"},{"line_number":729,"context_line":"        instance_obj.migrate_instance_keypairs,"},{"line_number":730,"context_line":"        keypair_obj.migrate_keypairs,"},{"line_number":731,"context_line":"    )"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_91dfa60b","line":730,"updated":"2016-05-13 14:50:20.000000000","message":"Nit, but I think these should be slightly more descriptive. Like migrate_keypairs_to_api_db.","commit_id":"787404c30de68e088307beca7c8eb69b5f29a0a9"}],"nova/objects/keypair.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dbfe43ea3f3c42c60dabcbbbf6cd9ae9f56b3347","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        kp.create()"},{"line_number":241,"context_line":"        done +\u003d 1"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    return len(main_keypairs), done"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_0e2c1eaa","line":243,"updated":"2016-05-09 16:25:41.000000000","message":"++ Above cannot be done in a transactional manner because it\u0027s across DBs. THe objects.KeyPair().create() call uses the API DB and the db_api.key_pair_destroy() uses the main/cell DB.","commit_id":"ad5745ff61437d06ca2046bef82c98cb3ca5e0f8"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"e2e5abeee593d1d239775477a55c2e6b1595eef5","unresolved":false,"context_lines":[{"line_number":207,"context_line":"@db_api.main_context_manager.reader"},{"line_number":208,"context_line":"def _count_unmigrated_instances(context):"},{"line_number":209,"context_line":"    return context.session.query(main_models.InstanceExtra).\\"},{"line_number":210,"context_line":"        filter_by(keypairs\u003dNone).\\"},{"line_number":211,"context_line":"        count()"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_1168d620","line":210,"updated":"2016-05-13 14:50:20.000000000","message":"Should this be filtering on deleted\u003d0? I didn\u0027t think the previous migration handled deleted instances so this could end up getting hit even if all of the instances that matter have migrated.","commit_id":"787404c30de68e088307beca7c8eb69b5f29a0a9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71a8766a268b414c03acd7e02eb3e922d805418a","unresolved":false,"context_lines":[{"line_number":207,"context_line":"@db_api.main_context_manager.reader"},{"line_number":208,"context_line":"def _count_unmigrated_instances(context):"},{"line_number":209,"context_line":"    return context.session.query(main_models.InstanceExtra).\\"},{"line_number":210,"context_line":"        filter_by(keypairs\u003dNone).\\"},{"line_number":211,"context_line":"        count()"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_2f464a21","line":210,"in_reply_to":"dab17558_1168d620","updated":"2016-05-13 16:35:06.000000000","message":"Yeah, good catch","commit_id":"787404c30de68e088307beca7c8eb69b5f29a0a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5484f821fe005341b5c4b82c85826112ce347ef7","unresolved":false,"context_lines":[{"line_number":205,"context_line":""},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"@db_api.main_context_manager.reader"},{"line_number":208,"context_line":"def _count_unmigrated_instances(context):"},{"line_number":209,"context_line":"    return context.session.query(main_models.InstanceExtra).\\"},{"line_number":210,"context_line":"        filter_by(keypairs\u003dNone).\\"},{"line_number":211,"context_line":"        filter_by(deleted\u003d0).\\"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_6a45cd15","line":208,"updated":"2016-05-19 18:34:02.000000000","message":"Ah it\u0027s useful to have this in my head when reading this because I was wondering where we migrated the instances, which happens before this in the nova-manage command and calls:\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/instance.py#L1269\n\nWhich just stores a serialized empty list if the instance didn\u0027t have a keypair, and a serialized KeyPairList object if it did. I was worried about the case of an instance without a keypair, but that migration handles it.","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fb8ad035dfcf25e2221ed38845672c69b44c4307","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        LOG.error(_LE(\u0027Some instances are still missing keypair \u0027"},{"line_number":227,"context_line":"                      \u0027information. Unable to run keypair migration \u0027"},{"line_number":228,"context_line":"                      \u0027at this time.\u0027))"},{"line_number":229,"context_line":"        return 0, 0"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    main_keypairs \u003d _get_main_keypairs(context, count)"},{"line_number":232,"context_line":"    done \u003d 0"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_1c31ef05","line":229,"updated":"2016-05-19 18:04:35.000000000","message":"This is probably being done in other places, but this doesn\u0027t allow for distinguishing between a migration that won\u0027t run and one that has completed right?","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a55d4c6eb33c5dfc5176256fdf70f4e5af91c1e3","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        LOG.error(_LE(\u0027Some instances are still missing keypair \u0027"},{"line_number":227,"context_line":"                      \u0027information. Unable to run keypair migration \u0027"},{"line_number":228,"context_line":"                      \u0027at this time.\u0027))"},{"line_number":229,"context_line":"        return 0, 0"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    main_keypairs \u003d _get_main_keypairs(context, count)"},{"line_number":232,"context_line":"    done \u003d 0"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_1fe9e114","line":229,"in_reply_to":"bab6814e_1c31ef05","updated":"2016-05-19 18:10:12.000000000","message":"We don\u0027t want to distinguish from the perspective of not wanting to block the looping code above us from moving on to something later in the list that it can do. And yes, this is the pattern used elsewhere (see the PCI one for example).\n\nFair point, but you could argue that we should return 0 from the CLI when there is nothing more we can do at the moment (think telling puppet to continue when we return nonzero so we move to the next phase where maybe some work can be done later).","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5484f821fe005341b5c4b82c85826112ce347ef7","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        LOG.error(_LE(\u0027Some instances are still missing keypair \u0027"},{"line_number":227,"context_line":"                      \u0027information. Unable to run keypair migration \u0027"},{"line_number":228,"context_line":"                      \u0027at this time.\u0027))"},{"line_number":229,"context_line":"        return 0, 0"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    main_keypairs \u003d _get_main_keypairs(context, count)"},{"line_number":232,"context_line":"    done \u003d 0"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_0a0521bb","line":229,"in_reply_to":"bab6814e_1fe9e114","updated":"2016-05-19 18:34:02.000000000","message":"And I guess our blocker really is the DB migration in Ocata that checks for these and fails if you haven\u0027t completed them all with 0 returns. Because if we hit this, it\u0027s because https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L1269 isn\u0027t done, which would have return \u003e0 for that migration, which means you\u0027re not done. And the DB migration for Ocata will have to block on that until they are all done online.","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fb8ad035dfcf25e2221ed38845672c69b44c4307","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                             public_key\u003ddb_keypair.public_key,"},{"line_number":239,"context_line":"                             type\u003ddb_keypair.type)"},{"line_number":240,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":241,"context_line":"        kp.create()"},{"line_number":242,"context_line":"        done +\u003d 1"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    return len(main_keypairs), done"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_7c48fb87","line":241,"updated":"2016-05-19 18:04:35.000000000","message":"These should be reversed just to be safe. Ensure that the new keypair has been written before deleting the old.","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a55d4c6eb33c5dfc5176256fdf70f4e5af91c1e3","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                             public_key\u003ddb_keypair.public_key,"},{"line_number":239,"context_line":"                             type\u003ddb_keypair.type)"},{"line_number":240,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":241,"context_line":"        kp.create()"},{"line_number":242,"context_line":"        done +\u003d 1"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    return len(main_keypairs), done"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_9fa6710e","line":241,"in_reply_to":"bab6814e_7c48fb87","updated":"2016-05-19 18:10:12.000000000","message":"well, the deal is, we can\u0027t because of the consistency check in keypair.create() that ensures we\u0027re not creating one that already exists in the other database (which it would until we delete it).\n\nNow, I could replicate the delete code here and try to abort the transaction on the source if we raise during create if you think that\u0027s better and worth it.","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"debeb8381d30dbbd73b3ad243215dd4335bb1b94","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                             public_key\u003ddb_keypair.public_key,"},{"line_number":239,"context_line":"                             type\u003ddb_keypair.type)"},{"line_number":240,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":241,"context_line":"        kp.create()"},{"line_number":242,"context_line":"        done +\u003d 1"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    return len(main_keypairs), done"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_9f131166","line":241,"in_reply_to":"bab6814e_9fa6710e","updated":"2016-05-19 18:25:26.000000000","message":"Probably a good idea to abort the transaction if creating in the API DB fails.","commit_id":"4d8a11fbd2e5ccfc204f054e66c468c70038a3e6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80ce19b186d56e9f962ee44f5cebd439dfbef77c","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        try:"},{"line_number":244,"context_line":"            kp._create()"},{"line_number":245,"context_line":"        except exception.KeyPairExists:"},{"line_number":246,"context_line":"            pass"},{"line_number":247,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":248,"context_line":"        done +\u003d 1"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_25be56d4","line":246,"updated":"2016-05-19 18:54:56.000000000","message":"Useful to have a comment in here about why we pass on this.","commit_id":"f0ad988d34c4602e918a6bfd250008d8ec14beb5"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"a0e95674b752b81ba089a11610c08d5ef3d3ccfe","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        except exception.KeyPairExists:"},{"line_number":246,"context_line":"            # NOTE(danms): If this got created somehow in the API DB,"},{"line_number":247,"context_line":"            # then it\u0027s newer and we just continue on to destroy the"},{"line_number":248,"context_line":"            # old one in the cell DB."},{"line_number":249,"context_line":"            pass"},{"line_number":250,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":251,"context_line":"        done +\u003d 1"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_251c9676","line":248,"updated":"2016-05-19 19:05:08.000000000","message":"The only way I see it happening is if there\u0027s a failure right after a kp._create here and then the migration is re-run.","commit_id":"b8aac794d4620aca341b269c6db71ea9e70d2210"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9e7c0df089d62396c5f7a717297bc2a68218cd00","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        except exception.KeyPairExists:"},{"line_number":246,"context_line":"            # NOTE(danms): If this got created somehow in the API DB,"},{"line_number":247,"context_line":"            # then it\u0027s newer and we just continue on to destroy the"},{"line_number":248,"context_line":"            # old one in the cell DB."},{"line_number":249,"context_line":"            pass"},{"line_number":250,"context_line":"        db_api.key_pair_destroy(context, db_keypair.user_id, db_keypair.name)"},{"line_number":251,"context_line":"        done +\u003d 1"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_8bf473f2","line":248,"in_reply_to":"bab6814e_251c9676","updated":"2016-05-19 19:32:13.000000000","message":"Yep, guess I should have said that.","commit_id":"b8aac794d4620aca341b269c6db71ea9e70d2210"}],"nova/tests/functional/db/test_keypair.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dbfe43ea3f3c42c60dabcbbbf6cd9ae9f56b3347","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        # NOTE(danms): This only fetches from the main DB"},{"line_number":153,"context_line":"        main_keys \u003d db_api.key_pair_get_all_by_user(self.context,"},{"line_number":154,"context_line":"                                                    self.context.user_id)"},{"line_number":155,"context_line":"        self.assertEqual(1, len(main_keys))"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def test_migrate_keypairs_bails_on_unmigrated_instances(self):"},{"line_number":158,"context_line":"        objects.Instance(context\u003dself.context, user_id\u003dself.context.user_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_6e196a88","line":155,"updated":"2016-05-09 16:25:41.000000000","message":"Might be nice to run keypair.migrate_keypairs(self.context, 2) again and verify the remaining keypair is migrated?","commit_id":"ad5745ff61437d06ca2046bef82c98cb3ca5e0f8"}]}