)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e88623a9415f75db7319eb2719d3fc0d96a643ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d5195596_5817fe89","updated":"2021-10-27 18:09:35.000000000","message":"-1 simply because I think you\u0027ve put a fix in here that should be in the previous patch. If it\u0027s not possible to do things before, let me know","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"70a37a201bfcbc49641e02b0048016979f455075","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a5dce99d_44681da1","updated":"2021-10-28 00:09:46.000000000","message":"I *think* there is an unnecessary change wrt the per connection database switching.\n\nAside: it doesn\u0027t look like Database(connection\u003d...) is being used anywhere, it was superseded by the CellDatabases fixture.","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3780eaa5568ac383bfea01c85eb1cdea121ad3d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a15139a5_3f3d83d0","in_reply_to":"a5dce99d_44681da1","updated":"2021-11-04 16:27:40.000000000","message":"we do use Database(connection\u003d) in a single test[1]. But I don\u0027t see an easy way to remove it from there.\n\n[1] https://github.com/openstack/nova/blob/ee91d92f9105aa7e7b1e45a47f70b8989b458a68/nova/tests/functional/db/test_connection_switch.py#L38","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3780eaa5568ac383bfea01c85eb1cdea121ad3d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d8182a6b_de587695","in_reply_to":"d5195596_5817fe89","updated":"2021-11-04 16:27:40.000000000","message":"Based on Melanie\u0027s comment we don\u0027t need that fix at all.","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0eea1deab798b2f48beb5aff8ffe4172a402f996","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"90330cf3_919e6178","updated":"2021-11-08 13:49:29.000000000","message":"Eventually accepting given gibi replied about my question.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e13a7b8c59b77eda6eeccd69828ea17bcc11fdc5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"794aa6ba_a5f313aa","updated":"2021-11-08 11:31:39.000000000","message":"LGTM makes sense.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"70a51391ec1739714af449c4f8e7ab6bb7853e4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"452b76ef_3444de8c","updated":"2021-11-05 14:57:33.000000000","message":"This looks to me legit, but I wonder about something","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bbdd9846e1c9182497b5d7dd4dd6d74a1c9747fe","unresolved":false,"context_lines":[{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        assert database in {\u0027main\u0027, \u0027api\u0027}, f\u0027Unrecognized database {database}\u0027"},{"line_number":612,"context_line":"        if database \u003d\u003d \u0027api\u0027:"},{"line_number":613,"context_line":"            assert connection is None, \u0027Not supported for the API database\u0027"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"        self.database \u003d database"},{"line_number":616,"context_line":"        self.version \u003d version"}],"source_content_type":"text/x-python","patch_set":1,"id":"366cca8d_8f96220b","line":613,"updated":"2021-10-27 16:13:27.000000000","message":"yeah, this is a better place for this to live","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e88623a9415f75db7319eb2719d3fc0d96a643ba","unresolved":true,"context_lines":[{"line_number":617,"context_line":"        self.connection \u003d connection"},{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    def setUp(self):"},{"line_number":620,"context_line":"        super(Database, self).setUp()"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":623,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d54f609b_1b63e855","line":620,"range":{"start_line":620,"start_character":14,"end_line":620,"end_character":28},"updated":"2021-10-27 18:09:35.000000000","message":"nit: we can drop this in Python 3","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3780eaa5568ac383bfea01c85eb1cdea121ad3d9","unresolved":false,"context_lines":[{"line_number":617,"context_line":"        self.connection \u003d connection"},{"line_number":618,"context_line":""},{"line_number":619,"context_line":"    def setUp(self):"},{"line_number":620,"context_line":"        super(Database, self).setUp()"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":623,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"37697270_f0e75f35","line":620,"range":{"start_line":620,"start_character":14,"end_line":620,"end_character":28},"in_reply_to":"d54f609b_1b63e855","updated":"2021-11-04 16:27:40.000000000","message":"Done","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"70a37a201bfcbc49641e02b0048016979f455075","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":623,"context_line":""},{"line_number":624,"context_line":"            if self.connection is not None:"},{"line_number":625,"context_line":"                ctxt_mgr \u003d enginefacade.transaction_context()"},{"line_number":626,"context_line":"                self.addCleanup(ctxt_mgr.patch_factory("},{"line_number":627,"context_line":"                    enginefacade._TransactionFactory()))"},{"line_number":628,"context_line":"                ctxt_mgr.configure("},{"line_number":629,"context_line":"                    **main_db_api._get_db_conf("},{"line_number":630,"context_line":"                        CONF.database, connection\u003dself.connection))"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"                self.get_engine \u003d ctxt_mgr.writer.get_engine"},{"line_number":633,"context_line":"            else:"},{"line_number":634,"context_line":"                # NOTE(gibi): this inject a new factory for each test and"}],"source_content_type":"text/x-python","patch_set":2,"id":"e5f08cad_342149e6","line":631,"range":{"start_line":625,"start_character":16,"end_line":631,"end_character":0},"updated":"2021-10-28 00:09:46.000000000","message":"I don\u0027t think you need (or want) to do this ... the call:\n\n ctxt_mgr \u003d main_db_api.create_context_manager(connection\u003dself.connection)\n\nwill create a new transaction context manager and call configure on it with self.connection. A new transaction context manager will have a new engine and thus a new factory each time.\n\nSo this part should be able to stay the same as before.","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3780eaa5568ac383bfea01c85eb1cdea121ad3d9","unresolved":false,"context_lines":[{"line_number":622,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":623,"context_line":""},{"line_number":624,"context_line":"            if self.connection is not None:"},{"line_number":625,"context_line":"                ctxt_mgr \u003d enginefacade.transaction_context()"},{"line_number":626,"context_line":"                self.addCleanup(ctxt_mgr.patch_factory("},{"line_number":627,"context_line":"                    enginefacade._TransactionFactory()))"},{"line_number":628,"context_line":"                ctxt_mgr.configure("},{"line_number":629,"context_line":"                    **main_db_api._get_db_conf("},{"line_number":630,"context_line":"                        CONF.database, connection\u003dself.connection))"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"                self.get_engine \u003d ctxt_mgr.writer.get_engine"},{"line_number":633,"context_line":"            else:"},{"line_number":634,"context_line":"                # NOTE(gibi): this inject a new factory for each test and"}],"source_content_type":"text/x-python","patch_set":2,"id":"7598dde0_af72d8f0","line":631,"range":{"start_line":625,"start_character":16,"end_line":631,"end_character":0},"in_reply_to":"e5f08cad_342149e6","updated":"2021-11-04 16:27:40.000000000","message":"Done","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e0230653123a8b5d4bdb0653dc1d14a524e3dc72","unresolved":true,"context_lines":[{"line_number":623,"context_line":"        super().setUp()"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":626,"context_line":""},{"line_number":627,"context_line":"            if self.connection is not None:"},{"line_number":628,"context_line":"                ctxt_mgr \u003d main_db_api.create_context_manager("},{"line_number":629,"context_line":"                    connection\u003dself.connection)"}],"source_content_type":"text/x-python","patch_set":3,"id":"09961add_5e626488","line":626,"updated":"2021-11-08 11:32:46.000000000","message":"nit - whitespace, somehow missed this first time in the diff. Not a massive thing obviously.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5ec3d938f27c48e9ff1e2abc0a340b46c8ba24d","unresolved":true,"context_lines":[{"line_number":623,"context_line":"        super().setUp()"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"        if self.database \u003d\u003d \u0027main\u0027:"},{"line_number":626,"context_line":""},{"line_number":627,"context_line":"            if self.connection is not None:"},{"line_number":628,"context_line":"                ctxt_mgr \u003d main_db_api.create_context_manager("},{"line_number":629,"context_line":"                    connection\u003dself.connection)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bb28f5bc_ff437ec3","line":626,"in_reply_to":"09961add_5e626488","updated":"2021-11-08 12:21:57.000000000","message":"If I have to respin then I will fix.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"70a51391ec1739714af449c4f8e7ab6bb7853e4a","unresolved":true,"context_lines":[{"line_number":635,"context_line":"                # factory so we can avoid having a global flag guarding against"},{"line_number":636,"context_line":"                # factory re-configuration"},{"line_number":637,"context_line":"                self.addCleanup(main_db_api.context_manager.patch_factory("},{"line_number":638,"context_line":"                    enginefacade._TransactionFactory()))"},{"line_number":639,"context_line":"                main_db_api.configure(CONF)"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":"                self.get_engine \u003d main_db_api.get_engine"}],"source_content_type":"text/x-python","patch_set":3,"id":"764e7a47_1cdc2d2d","line":638,"updated":"2021-11-05 14:57:33.000000000","message":"Technically, you now call it *after* running the tests while previously, it was done *before* running the tests.\nIt looks to me a non-issue.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5ec3d938f27c48e9ff1e2abc0a340b46c8ba24d","unresolved":false,"context_lines":[{"line_number":635,"context_line":"                # factory so we can avoid having a global flag guarding against"},{"line_number":636,"context_line":"                # factory re-configuration"},{"line_number":637,"context_line":"                self.addCleanup(main_db_api.context_manager.patch_factory("},{"line_number":638,"context_line":"                    enginefacade._TransactionFactory()))"},{"line_number":639,"context_line":"                main_db_api.configure(CONF)"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":"                self.get_engine \u003d main_db_api.get_engine"}],"source_content_type":"text/x-python","patch_set":3,"id":"8fa87c84_b7092d2c","line":638,"in_reply_to":"764e7a47_1cdc2d2d","updated":"2021-11-08 12:21:57.000000000","message":"This change does not change when the patching and cleanup happens.\nIn both the base and the new case the call\n\n  patch_factory(enginefacade._TransactionFactory())\n\ndoes the patching and returns a callable that will do the cleanup when called.\n\nIn the base I stored that callable in factory_reset and called during cleanup() manually as the patcher code was called from __init__ where I cannot use addCleanup. In the new code the setup code moved from __init__ to setUp() so I can now use addCleanup to let the fixture schedule the cleanup call at the end of the testase.","commit_id":"c7ccbfe403f100b5cb420fb12ac6244c4a7ae381"}],"nova/tests/functional/db/test_connection_switch.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bbdd9846e1c9182497b5d7dd4dd6d74a1c9747fe","unresolved":true,"context_lines":[{"line_number":36,"context_line":"        # Use a file-based sqlite database so data will persist across new"},{"line_number":37,"context_line":"        # connections"},{"line_number":38,"context_line":"        # The \u0027main\u0027 database connection will stay open, so in-memory is fine"},{"line_number":39,"context_line":"        self.useFixture(nova_fixtures.Database(connection\u003dself.fake_conn))"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def cleanup(self):"},{"line_number":42,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"35444584_6311d6a8","line":39,"updated":"2021-10-27 16:13:27.000000000","message":"This is weird. We create...three database fixtures?\n\nLater: oh, I guess one of the API database, one is cell0 and this one is \"cell1\"? Not your problem really, but a comment here would be superb","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"356bc118c5f67dcb92b1975bd3f4381bc6fe87f8","unresolved":true,"context_lines":[{"line_number":36,"context_line":"        # Use a file-based sqlite database so data will persist across new"},{"line_number":37,"context_line":"        # connections"},{"line_number":38,"context_line":"        # The \u0027main\u0027 database connection will stay open, so in-memory is fine"},{"line_number":39,"context_line":"        self.useFixture(nova_fixtures.Database(connection\u003dself.fake_conn))"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def cleanup(self):"},{"line_number":42,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"66de2492_f3651969","line":39,"in_reply_to":"35444584_6311d6a8","updated":"2021-10-27 16:23:17.000000000","message":"yeah, probably you are right but I\u0027m not sure after looking at your below comment.","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bbdd9846e1c9182497b5d7dd4dd6d74a1c9747fe","unresolved":true,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        # Verify the instance isn\u0027t found in the api database"},{"line_number":70,"context_line":"        # We don\u0027t even have the instances table in the api database"},{"line_number":71,"context_line":"        self.assertRaises(oslo_db_exception.DBNonExistentTable,"},{"line_number":72,"context_line":"                          objects.Instance.get_by_uuid, ctxt, uuid)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f743e4d8_22fc8e3f","line":71,"updated":"2021-10-27 16:13:27.000000000","message":"I\u0027ve read this a few times and still don\u0027t get it. We have the following call order\n\n  objects.Instance.get_by_uuid -\u003e\n    objects.instance._db_instance_get_by_uuid -\u003e\n      db.main.api.instance_get_by_uuid\n\nso where are we talking to the API database, and why is this exception changing now?","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"356bc118c5f67dcb92b1975bd3f4381bc6fe87f8","unresolved":true,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        # Verify the instance isn\u0027t found in the api database"},{"line_number":70,"context_line":"        # We don\u0027t even have the instances table in the api database"},{"line_number":71,"context_line":"        self.assertRaises(oslo_db_exception.DBNonExistentTable,"},{"line_number":72,"context_line":"                          objects.Instance.get_by_uuid, ctxt, uuid)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fed75343_92138575","line":71,"in_reply_to":"f743e4d8_22fc8e3f","updated":"2021-10-27 16:23:17.000000000","message":"The context in this test is not cell targeted at first. Then we cell target it to a new cell. Then the context manager exits so the cell targeting is removed...\n\n...\n\nOK you caught me, I don\u0027t know why I said it is looking into the api db. And then I don\u0027t know why it ends up saying that the instances table is not found.","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4869ccf32edf5531b154fa48d16a04fc09ecbe33","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        # Verify the instance isn\u0027t found in the api database"},{"line_number":70,"context_line":"        # We don\u0027t even have the instances table in the api database"},{"line_number":71,"context_line":"        self.assertRaises(oslo_db_exception.DBNonExistentTable,"},{"line_number":72,"context_line":"                          objects.Instance.get_by_uuid, ctxt, uuid)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"11fef48b_25ffebc0","line":71,"in_reply_to":"fed75343_92138575","updated":"2021-10-27 17:19:36.000000000","message":"OK, I missed the case int he Database fixture when a connection string is directly provided and there I still patched the global factory instead of patching the locally created factory. Now this test passes without modification","commit_id":"5b60083e2eb297ec52463190c22661de8cdba3ef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3780eaa5568ac383bfea01c85eb1cdea121ad3d9","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import os"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from oslo_db import exception as oslo_db_exception"},{"line_number":16,"context_line":"from oslo_utils.fixture import uuidsentinel as uuids"},{"line_number":17,"context_line":"from oslo_utils import uuidutils"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fb487f1_b3e366a0","line":15,"in_reply_to":"a7057c73_dc173d13","updated":"2021-11-04 16:27:40.000000000","message":"\u003e pep8: F401 \u0027oslo_db.exception as oslo_db_exception\u0027 imported but unused\n\nDone.","commit_id":"5538e2ceed0472f75c75ed5f5e38191523acc6e8"}]}