)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":35,"context_line":"intentionally orphans the instance record to avoid triggering"},{"line_number":36,"context_line":"lazy-loads while it attempts to populate instance.flavor,"},{"line_number":37,"context_line":"instance.new_flavor, and instance.old_flavor and then another lazy-load"},{"line_number":38,"context_line":"is triggered (because instance.flavor is still not populateds) and"},{"line_number":39,"context_line":"fails with OrphanedObjectError."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"One way to solve this problem is to make it impossible for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"57e7947b_9852a1e6","line":38,"range":{"start_line":38,"start_character":51,"end_line":38,"end_character":61},"updated":"2021-03-09 18:12:32.000000000","message":"populated","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":false,"context_lines":[{"line_number":47,"context_line":"together in a single database transaction to archive all related"},{"line_number":48,"context_line":"records \"atomically\". The idea is that if anything were to interrupt"},{"line_number":49,"context_line":"the transaction (errors or other) it would roll back and keep all the"},{"line_number":50,"context_line":"related records together. Either all or archived or none are archived."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"This changes the logic of the per table archive to discover tables that"},{"line_number":53,"context_line":"refer to the table by foreign keys and generates insert/delete query"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"612f5e84_4bcc0ac2","line":50,"updated":"2021-03-09 18:12:32.000000000","message":"Well that\u0027s very sensible","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":4059,"context_line":"    return tables"},{"line_number":4060,"context_line":""},{"line_number":4061,"context_line":""},{"line_number":4062,"context_line":"def _collect_fk_inserts_and_deletes(metadata, conn, table, column, records,"},{"line_number":4063,"context_line":"                                    inserts, deletes):"},{"line_number":4064,"context_line":"    \"\"\"Find records related to this table by foreign key (FK) and create and"},{"line_number":4065,"context_line":"    collect insert/delete statements for them."}],"source_content_type":"text/x-python","patch_set":4,"id":"360911fb_11c54dfb","line":4062,"range":{"start_line":4062,"start_character":4,"end_line":4062,"end_character":35},"updated":"2021-03-09 18:12:32.000000000","message":"nit: This is very descriptive, but it does make calling it a bit long-winded. Would \u0027_collect_fk_ops\u0027 be appropriate?","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d71d64611cfe88fcfb862529c0be6e41b74d04f","unresolved":true,"context_lines":[{"line_number":4059,"context_line":"    return tables"},{"line_number":4060,"context_line":""},{"line_number":4061,"context_line":""},{"line_number":4062,"context_line":"def _collect_fk_inserts_and_deletes(metadata, conn, table, column, records,"},{"line_number":4063,"context_line":"                                    inserts, deletes):"},{"line_number":4064,"context_line":"    \"\"\"Find records related to this table by foreign key (FK) and create and"},{"line_number":4065,"context_line":"    collect insert/delete statements for them."}],"source_content_type":"text/x-python","patch_set":4,"id":"bc1bf2f0_8035f2c9","line":4062,"range":{"start_line":4062,"start_character":4,"end_line":4062,"end_character":35},"in_reply_to":"360911fb_11c54dfb","updated":"2021-03-09 23:44:32.000000000","message":"Agreed it\u0027s long. I wanted it to be clear that it generates sql statements so maybe like _collect_fk_stmts? Actually, I used the word \"collect\" because of the modification of args, so when I change it to not, I\u0027d likely  want to call it _get_fk_statements instead.","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":4074,"context_line":"    :param conn: Connection object to use to select records related by FK"},{"line_number":4075,"context_line":"    :param table: Table object (parent) for which to find references by FK"},{"line_number":4076,"context_line":"    :param column: Column object (parent) to use to select records related by"},{"line_number":4077,"context_line":"                   FK"},{"line_number":4078,"context_line":"    :param records: A list of records (column values) to use to select records"},{"line_number":4079,"context_line":"                    related by FK"},{"line_number":4080,"context_line":"    :param inserts: A collections.deque of insert query statements. Shadow"}],"source_content_type":"text/x-python","patch_set":4,"id":"45216b8c_d51626ec","line":4077,"range":{"start_line":4077,"start_character":9,"end_line":4077,"end_character":19},"updated":"2021-03-09 18:12:32.000000000","message":"nit (and below)","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d71d64611cfe88fcfb862529c0be6e41b74d04f","unresolved":true,"context_lines":[{"line_number":4074,"context_line":"    :param conn: Connection object to use to select records related by FK"},{"line_number":4075,"context_line":"    :param table: Table object (parent) for which to find references by FK"},{"line_number":4076,"context_line":"    :param column: Column object (parent) to use to select records related by"},{"line_number":4077,"context_line":"                   FK"},{"line_number":4078,"context_line":"    :param records: A list of records (column values) to use to select records"},{"line_number":4079,"context_line":"                    related by FK"},{"line_number":4080,"context_line":"    :param inserts: A collections.deque of insert query statements. Shadow"}],"source_content_type":"text/x-python","patch_set":4,"id":"15382623_bb3ff061","line":4077,"range":{"start_line":4077,"start_character":9,"end_line":4077,"end_character":19},"in_reply_to":"45216b8c_d51626ec","updated":"2021-03-09 23:44:32.000000000","message":"OK","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":4096,"context_line":"                                    autoload\u003dTrue)"},{"line_number":4097,"context_line":"        except NoSuchTableError:"},{"line_number":4098,"context_line":"            # No corresponding shadow table; skip it."},{"line_number":4099,"context_line":"            continue"},{"line_number":4100,"context_line":"        # TODO(stephenfin): Drop this when we drop the table"},{"line_number":4101,"context_line":"        if fk_table.name \u003d\u003d \"dns_domains\":"},{"line_number":4102,"context_line":"            # We have one table (dns_domains) where the key is called"}],"source_content_type":"text/x-python","patch_set":4,"id":"dde99614_65751961","line":4099,"updated":"2021-03-09 18:12:32.000000000","message":"style nit: could you add a newline after this to help visually group things? This is a lot to absorb visually","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d71d64611cfe88fcfb862529c0be6e41b74d04f","unresolved":true,"context_lines":[{"line_number":4096,"context_line":"                                    autoload\u003dTrue)"},{"line_number":4097,"context_line":"        except NoSuchTableError:"},{"line_number":4098,"context_line":"            # No corresponding shadow table; skip it."},{"line_number":4099,"context_line":"            continue"},{"line_number":4100,"context_line":"        # TODO(stephenfin): Drop this when we drop the table"},{"line_number":4101,"context_line":"        if fk_table.name \u003d\u003d \"dns_domains\":"},{"line_number":4102,"context_line":"            # We have one table (dns_domains) where the key is called"}],"source_content_type":"text/x-python","patch_set":4,"id":"c86a704c_517a4b39","line":4099,"in_reply_to":"dde99614_65751961","updated":"2021-03-09 23:44:32.000000000","message":"Can do.","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":4103,"context_line":"            # \"domain\" rather than \"id\""},{"line_number":4104,"context_line":"            fk_column \u003d fk_table.c.domain"},{"line_number":4105,"context_line":"        else:"},{"line_number":4106,"context_line":"            fk_column \u003d fk_table.c.id"},{"line_number":4107,"context_line":"        for fk in fk_table.foreign_keys:"},{"line_number":4108,"context_line":"            # We need to find the records in the referring (child) table that"},{"line_number":4109,"context_line":"            # correspond to the records in our (parent) table so we can archive"}],"source_content_type":"text/x-python","patch_set":4,"id":"65cd65d8_94ee52e5","line":4106,"updated":"2021-03-09 18:12:32.000000000","message":"ditto","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e56a70e4fc03edc6e9debdd54d77edc230c816b4","unresolved":true,"context_lines":[{"line_number":4153,"context_line":"                deletes.appendleft(fk_delete)"},{"line_number":4154,"context_line":"        # Repeat for any possible nested child tables."},{"line_number":4155,"context_line":"        _collect_fk_inserts_and_deletes("},{"line_number":4156,"context_line":"            metadata, conn, fk_table, fk_column, fk_records, inserts, deletes)"},{"line_number":4157,"context_line":""},{"line_number":4158,"context_line":""},{"line_number":4159,"context_line":"def _archive_deleted_rows_for_table(metadata, tablename, max_rows, before):"}],"source_content_type":"text/x-python","patch_set":4,"id":"8555d98e_78b40cab","line":4156,"updated":"2021-03-09 18:12:32.000000000","message":"The use of side-effects here works, but it\u0027s a little confusing and not something commonly done in Python. Rather than doing this, could we make use of return values? Something as simple as the below ought to do the trick?\n\n  inserts \u003d []\n  deletes \u003d []\n  for fk in fk_table.foreign_keys:\n      ...\n      if fk_records:\n          ...\n          inserts.insert(0, fk_insert)\n          ...\n          deletes.insert(0, fk_delete)\n\n      result \u003d _collect_fk_inserts_and_deletes(\n          metadata, conn, fk_table, fk_column, fk_records)\n      inserts \u003d result[0] + inserts\n      deletes \u003d result[1] + deletes\n\n...or equivalent with deque - I\u0027m not familiar with the API for that /o\\ \n\nYou could also make use of yield and just switch the order around so we go depth-first:\n\n  for result in _collect_fk_inserts_and_deletes(\n      metadata, conn, fk_table, fk_column, fk_records,\n  ):\n      yield result\n\n  for fk in fk_table.foreign_keys:\n      ...\n      if fk_records:\n          ...\n          yield (insert, delete)\n\nThat makes the caller a bit weird since you\u0027d now be iterating over the operations on a per-table basis rather than a per-operation type basis, but it\u0027s still readable:\n\n  fk_ops \u003d _collect_fk_inserts_and_deletes(\n      metadata, conn, table, column, records)\n  \n  for insert_op, delete_op in fk_ops:\n      ...\n\nIn any case, I think it\u0027s fair to say we do not need to rely on side effects here 😅","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d71d64611cfe88fcfb862529c0be6e41b74d04f","unresolved":true,"context_lines":[{"line_number":4153,"context_line":"                deletes.appendleft(fk_delete)"},{"line_number":4154,"context_line":"        # Repeat for any possible nested child tables."},{"line_number":4155,"context_line":"        _collect_fk_inserts_and_deletes("},{"line_number":4156,"context_line":"            metadata, conn, fk_table, fk_column, fk_records, inserts, deletes)"},{"line_number":4157,"context_line":""},{"line_number":4158,"context_line":""},{"line_number":4159,"context_line":"def _archive_deleted_rows_for_table(metadata, tablename, max_rows, before):"}],"source_content_type":"text/x-python","patch_set":4,"id":"422afe6c_d15df28e","line":4156,"in_reply_to":"8555d98e_78b40cab","updated":"2021-03-09 23:44:32.000000000","message":"That\u0027s fair enough about the side effects. It took me awhile to get this working right 😆 and I\u0027m not attached to the idea of modifying the args. Will change it.","commit_id":"eea366e39593598f01858c7d3802a4e9ea3f247c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6c578e54fe0d494cca9688954a8a824846f52a24","unresolved":true,"context_lines":[{"line_number":4225,"context_line":"    # following FK relationships. Since we are iterating over the sorted_tables"},{"line_number":4226,"context_line":"    # (list of Table objects sorted in order of foreign key dependency), we"},{"line_number":4227,"context_line":"    # will add new deletes (\"leaves\") to the front of the deque."},{"line_number":4228,"context_line":"    fk_deletes \u003d collections.deque()"},{"line_number":4229,"context_line":"    # Keep track of any extra tablenames to number of rows that we archive by"},{"line_number":4230,"context_line":"    # following FK relationships."},{"line_number":4231,"context_line":"    # {tablename: extra_rows_archived}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f9ce012_2d1b0462","line":4228,"updated":"2021-03-11 09:32:31.000000000","message":"You can remove these now. They\u0027re never accessed outside of the scope of the below if-statement where they\u0027re created","commit_id":"6d6093420e36fc85fd45f3cd73d6ef7f1eca64ac"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"92b817cbd93c0a5c77e5113623f20a79505fd46e","unresolved":true,"context_lines":[{"line_number":4225,"context_line":"    # following FK relationships. Since we are iterating over the sorted_tables"},{"line_number":4226,"context_line":"    # (list of Table objects sorted in order of foreign key dependency), we"},{"line_number":4227,"context_line":"    # will add new deletes (\"leaves\") to the front of the deque."},{"line_number":4228,"context_line":"    fk_deletes \u003d collections.deque()"},{"line_number":4229,"context_line":"    # Keep track of any extra tablenames to number of rows that we archive by"},{"line_number":4230,"context_line":"    # following FK relationships."},{"line_number":4231,"context_line":"    # {tablename: extra_rows_archived}"}],"source_content_type":"text/x-python","patch_set":5,"id":"19bb9c42_a70b2c35","line":4228,"in_reply_to":"3f9ce012_2d1b0462","updated":"2021-03-11 19:48:18.000000000","message":"Oh whoops, thanks, will remove.","commit_id":"6d6093420e36fc85fd45f3cd73d6ef7f1eca64ac"}],"nova/tests/functional/db/test_archive.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"86190f1fe8606f985a104a1841f34111fa108224","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        \"\"\"This tests a scenario where archive_deleted_rows is run with"},{"line_number":133,"context_line":"        --max_rows and does not run to completion."},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        That is, the archive is stopped before all archivable records have been"},{"line_number":136,"context_line":"        archived. Specifically, the problematic state is when a single instance"},{"line_number":137,"context_line":"        becomes partially archived (example: \u0027instance_extra\u0027 record for one"},{"line_number":138,"context_line":"        instance has been archived while its \u0027instances\u0027 record remains). Any"},{"line_number":139,"context_line":"        access of the instance (example: listing deleted instances) that"},{"line_number":140,"context_line":"        triggers the retrieval of a dependent record that has been archived"},{"line_number":141,"context_line":"        away, results in undefined behavior that may raise an error."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        We will force the system into a state where a single deleted instance"},{"line_number":144,"context_line":"        is partially archived. We want to verify that we can, for example,"}],"source_content_type":"text/x-python","patch_set":6,"id":"0b3541d9_63c27fb4","line":141,"range":{"start_line":135,"start_character":0,"end_line":141,"end_character":68},"updated":"2021-03-23 11:05:13.000000000","message":"Did you want to rewrite this now that it isn\u0027t possible to get into this partial state with everything deleted atomically?","commit_id":"becb94ae643ab4863daa564783646921b4a2b372"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d192dc2c4a318612dae3a42c3e7c39ca18059021","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        \"\"\"This tests a scenario where archive_deleted_rows is run with"},{"line_number":133,"context_line":"        --max_rows and does not run to completion."},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        That is, the archive is stopped before all archivable records have been"},{"line_number":136,"context_line":"        archived. Specifically, the problematic state is when a single instance"},{"line_number":137,"context_line":"        becomes partially archived (example: \u0027instance_extra\u0027 record for one"},{"line_number":138,"context_line":"        instance has been archived while its \u0027instances\u0027 record remains). Any"},{"line_number":139,"context_line":"        access of the instance (example: listing deleted instances) that"},{"line_number":140,"context_line":"        triggers the retrieval of a dependent record that has been archived"},{"line_number":141,"context_line":"        away, results in undefined behavior that may raise an error."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        We will force the system into a state where a single deleted instance"},{"line_number":144,"context_line":"        is partially archived. We want to verify that we can, for example,"}],"source_content_type":"text/x-python","patch_set":6,"id":"eda10b87_e3c91286","line":141,"range":{"start_line":135,"start_character":0,"end_line":141,"end_character":68},"in_reply_to":"0b3541d9_63c27fb4","updated":"2021-03-23 20:42:22.000000000","message":"Oh, yeah good point. I will propose a follow up to adjust this. Thanks!","commit_id":"becb94ae643ab4863daa564783646921b4a2b372"}]}