)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"52b3d323a0e7f5c442e795efd167c48b3ad031b7","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"    model_query(context, models.BlockDeviceMapping).\\"},{"line_number":1836,"context_line":"            filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":1837,"context_line":"            soft_delete()"},{"line_number":1838,"context_line":"    model_query(context, models.Console).filter_by("},{"line_number":1839,"context_line":"            instance_uuid\u003dinstance_uuid).soft_delete()"},{"line_number":1840,"context_line":"    # NOTE(snikitin): We can\u0027t use model_query here, because there is no"},{"line_number":1841,"context_line":"    # column \u0027deleted\u0027 in \u0027tags\u0027 table."}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_bb7d431e","line":1838,"updated":"2016-08-04 13:10:50.000000000","message":"Hmm, what about the upgrade case? This code will only soft delete the associated Console entries for instances deleted *after* this code was deployed, but you may *already* have soft-deleted but not archived instances rows with Console\u0027s.\n\nWouldn\u0027t archiving of instances fail in that case?\n\nThis change is fine, but I think we\u0027ll also need something like https://review.openstack.org/#/c/323684/17/nova/db/sqlalchemy/api.py in _archive_deleted_rows_for_table() to ensure archiving succeeds.","commit_id":"b03321001e5583a9673bd2ca2c564e5c17d9ea81"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"32c242f17ab3b7c3344d59a6756e7d212f229350","unresolved":false,"context_lines":[{"line_number":6317,"context_line":"    # have to be manage differently so we soft-delete them here to let"},{"line_number":6318,"context_line":"    # the archive work the same for all tables"},{"line_number":6319,"context_line":"    # TODO(bhagyashris): \u0027consoles\u0027 entry should be removed in future"},{"line_number":6320,"context_line":"    # onceit is ensured all consoles are marked as soft-deleted"},{"line_number":6321,"context_line":"    # for deleted instances."},{"line_number":6322,"context_line":"    if tablename in (\"instance_actions\", \"consoles\"):"},{"line_number":6323,"context_line":"        instances \u003d models.BASE.metadata.tables[\"instances\"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_cb28e035","line":6320,"updated":"2016-08-11 12:42:30.000000000","message":"typo: once it is ensured","commit_id":"3aba2d23ac82ed1a593431a9e353cbf8e1fde0f0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"463506874a6f922130ef39483a0b02af146168a5","unresolved":false,"context_lines":[{"line_number":1855,"context_line":"    model_query(context, models.BlockDeviceMapping).\\"},{"line_number":1856,"context_line":"            filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":1857,"context_line":"            soft_delete()"},{"line_number":1858,"context_line":"    model_query(context, models.Console).filter_by("},{"line_number":1859,"context_line":"            instance_uuid\u003dinstance_uuid).soft_delete()"},{"line_number":1860,"context_line":"    # NOTE(snikitin): We can\u0027t use model_query here, because there is no"},{"line_number":1861,"context_line":"    # column \u0027deleted\u0027 in \u0027tags\u0027 table."}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_616eb797","line":1858,"updated":"2016-09-15 01:24:48.000000000","message":"Since we don\u0027t have cascading deletes and we just keep hard-coding these in here, I feel like at some point we\u0027re going to have to refactor this code to look up the related tables to instances and go through and soft delete those entries programmatically, but that doesn\u0027t need to happen in this change.","commit_id":"d3404847d443ed08dc5437a8f9b11f8ac46b1154"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5e43ceba1e8e74b8ac0e7c42ac5364b498124f26","unresolved":false,"context_lines":[{"line_number":6329,"context_line":"    # TODO(bhagyashris): \u0027consoles\u0027 entry should be removed in future"},{"line_number":6330,"context_line":"    # once it is ensured all consoles are marked as soft-deleted"},{"line_number":6331,"context_line":"    # for deleted instances."},{"line_number":6332,"context_line":"    if tablename in (\"instance_actions\", \"consoles\"):"},{"line_number":6333,"context_line":"        instances \u003d models.BASE.metadata.tables[\"instances\"]"},{"line_number":6334,"context_line":"        deleted_instances \u003d sql.select([instances.c.uuid]).\\"},{"line_number":6335,"context_line":"            where(instances.c.deleted !\u003d instances.c.deleted.default.arg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_cf55cd1e","line":6332,"updated":"2016-09-12 14:36:55.000000000","message":"This kind of grosses me out, but I\u0027m not having any bright ideas on this otherwise.\n\nWe could have a nova-manage command to delete these, but that\u0027s what archive already is.\n\nWe could have a database migration that did this though too, but I\u0027m not sure if that\u0027s a great solution either.","commit_id":"d3404847d443ed08dc5437a8f9b11f8ac46b1154"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"463506874a6f922130ef39483a0b02af146168a5","unresolved":false,"context_lines":[{"line_number":6329,"context_line":"    # TODO(bhagyashris): \u0027consoles\u0027 entry should be removed in future"},{"line_number":6330,"context_line":"    # once it is ensured all consoles are marked as soft-deleted"},{"line_number":6331,"context_line":"    # for deleted instances."},{"line_number":6332,"context_line":"    if tablename in (\"instance_actions\", \"consoles\"):"},{"line_number":6333,"context_line":"        instances \u003d models.BASE.metadata.tables[\"instances\"]"},{"line_number":6334,"context_line":"        deleted_instances \u003d sql.select([instances.c.uuid]).\\"},{"line_number":6335,"context_line":"            where(instances.c.deleted !\u003d instances.c.deleted.default.arg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_a1727f47","line":6332,"in_reply_to":"9a89bdaa_cf55cd1e","updated":"2016-09-15 01:24:48.000000000","message":"We could also potentially do the console cleanup in the nova-manage db online_data_migrations command which gets run on upgrades.\n\nI\u0027ve posted to the dev list about this to see if there is other input:\n\nhttp://lists.openstack.org/pipermail/openstack-dev/2016-September/103709.html\n\nSince something like that wouldn\u0027t happen until Ocata, I\u0027m OK with just getting this in for now as a workaround and remove it after we have some kind of data migration in place.\n\nHowever, I\u0027m -1 on this change for now since this change in the archive code isn\u0027t tested.","commit_id":"d3404847d443ed08dc5437a8f9b11f8ac46b1154"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"4913ef65840377f3d69230d299f518adad75c43d","unresolved":false,"context_lines":[{"line_number":2847,"context_line":""},{"line_number":2848,"context_line":"    def test_delete_console_on_instance_destroy(self):"},{"line_number":2849,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":2850,"context_line":"        uuid \u003d str(stdlib_uuid.uuid4())"},{"line_number":2851,"context_line":"        # Create console"},{"line_number":2852,"context_line":"        db.instance_create(ctxt, {\u0027uuid\u0027: uuid})"},{"line_number":2853,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_e949e8d1","line":2850,"range":{"start_line":2850,"start_character":15,"end_line":2850,"end_character":39},"updated":"2016-07-25 06:52:55.000000000","message":"Use uuidsentinel.","commit_id":"a3226e024d980a2945d9ee9faaad6b8a08a55e8f"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"52b3d323a0e7f5c442e795efd167c48b3ad031b7","unresolved":false,"context_lines":[{"line_number":2864,"context_line":"        consoles \u003d db.console_get_all_by_instance(ctxt, uuid)"},{"line_number":2865,"context_line":"        # Console record is retrieved because \"read_deleted\" flag in"},{"line_number":2866,"context_line":"        # db.console_get_all_by_instance() is set to \"yes\"."},{"line_number":2867,"context_line":"        self.assertEqual(1, len(consoles))"},{"line_number":2868,"context_line":""},{"line_number":2869,"context_line":"    def test_instance_update_and_get_original(self):"},{"line_number":2870,"context_line":"        instance \u003d self.create_instance_with_args(vm_state\u003d\u0027building\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_3b61533a","line":2867,"updated":"2016-08-04 13:10:50.000000000","message":"Can we check attribute `deleted` of the returned instance?","commit_id":"b03321001e5583a9673bd2ca2c564e5c17d9ea81"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"32c242f17ab3b7c3344d59a6756e7d212f229350","unresolved":false,"context_lines":[{"line_number":2892,"context_line":"        consoles \u003d db.console_get_all_by_instance(ctxt, uuid)"},{"line_number":2893,"context_line":"        # Console record is retrieved because \"read_deleted\" flag in"},{"line_number":2894,"context_line":"        # db.console_get_all_by_instance() is set to \"yes\"."},{"line_number":2895,"context_line":"        self.assertEqual(1, instance.deleted)"},{"line_number":2896,"context_line":"        self.assertEqual(1, len(consoles))"},{"line_number":2897,"context_line":""},{"line_number":2898,"context_line":"    def test_instance_update_and_get_original(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_eb57a4c7","line":2895,"updated":"2016-08-11 12:42:30.000000000","message":"Why not check  the value of attribute deleted of the console object instead? As that\u0027s what your change is about. IMO, this should be:\n\n  self.assertEqual(1, len(consoles))\n  self.assertTrue(consoles[0].deleted)","commit_id":"3aba2d23ac82ed1a593431a9e353cbf8e1fde0f0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"463506874a6f922130ef39483a0b02af146168a5","unresolved":false,"context_lines":[{"line_number":9006,"context_line":"            \u0027shadow_dns_domains\u0027,"},{"line_number":9007,"context_line":"        )"},{"line_number":9008,"context_line":""},{"line_number":9009,"context_line":"    def test_archive_deleted_rows_fk_constraint(self):"},{"line_number":9010,"context_line":"        # consoles.pool_id depends on console_pools.id"},{"line_number":9011,"context_line":"        # SQLite doesn\u0027t enforce foreign key constraints without a pragma."},{"line_number":9012,"context_line":"        dialect \u003d self.engine.url.get_dialect()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_619f9750","line":9009,"updated":"2016-09-15 01:24:48.000000000","message":"Seems like you should have a test like this but with consoles entries, like in this similar change for migrations:\n\nhttps://review.openstack.org/#/c/323684/28/nova/tests/unit/db/test_db_api.py@9113","commit_id":"d3404847d443ed08dc5437a8f9b11f8ac46b1154"}]}