)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ad3c0df7adb7eaa52d1f24d74a9c395dfad60227","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3018e9d6_51d1732e","updated":"2021-10-27 15:49:27.000000000","message":"Oooh, snazzy \u003c3 Assuming the CI is happy, so am I","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1d2c3bcfbe3a10f123ea3332e42b57cecef21a56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0c8061f1_7fa35de2","updated":"2021-10-27 18:10:03.000000000","message":"Switching to -1 simply because I think you\u0027ve put a fix in a later patch that should be in here now instead. If it\u0027s not possible to do things before, let me know","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d286bcc863acf74bf59920097885894435ff5933","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1f41369a_d3852b06","updated":"2021-11-08 11:29:29.000000000","message":"LGTM, not approving to allow Mel to come back and ACK migi\u0027s comments.","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"22876143fd426e82e63816bb13e08368c9fd64e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cada2a12_ba249d22","updated":"2021-11-09 08:39:52.000000000","message":"Thanks gibi, LGTM","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"c862c385f8deb2748d585686d69fd8012a2597e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d0ca964b_168e2e6f","in_reply_to":"1f41369a_d3852b06","updated":"2021-11-08 11:29:48.000000000","message":"gibi*","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ad3c0df7adb7eaa52d1f24d74a9c395dfad60227","unresolved":true,"context_lines":[{"line_number":614,"context_line":"        self.version \u003d version"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"        if database \u003d\u003d \u0027main\u0027:"},{"line_number":617,"context_line":"            # NOTE(gibi): this inject a new factor for each test and"},{"line_number":618,"context_line":"            # self.factory_reset is used to clean up the factory at the end"},{"line_number":619,"context_line":"            # of the test case. This way we can let each test configure the"},{"line_number":620,"context_line":"            # factory so we can avoid having a global flag guarding against"}],"source_content_type":"text/x-python","patch_set":1,"id":"95eded19_264cbc24","line":617,"range":{"start_line":617,"start_character":44,"end_line":617,"end_character":50},"updated":"2021-10-27 15:49:27.000000000","message":"factory?","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9a9286254a6c621adddb17d6059956a39966944e","unresolved":false,"context_lines":[{"line_number":614,"context_line":"        self.version \u003d version"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"        if database \u003d\u003d \u0027main\u0027:"},{"line_number":617,"context_line":"            # NOTE(gibi): this inject a new factor for each test and"},{"line_number":618,"context_line":"            # self.factory_reset is used to clean up the factory at the end"},{"line_number":619,"context_line":"            # of the test case. This way we can let each test configure the"},{"line_number":620,"context_line":"            # factory so we can avoid having a global flag guarding against"}],"source_content_type":"text/x-python","patch_set":1,"id":"bfe9a170_8dbf406f","line":617,"range":{"start_line":617,"start_character":44,"end_line":617,"end_character":50},"in_reply_to":"95eded19_264cbc24","updated":"2021-11-04 16:27:30.000000000","message":"Done","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1d2c3bcfbe3a10f123ea3332e42b57cecef21a56","unresolved":true,"context_lines":[{"line_number":620,"context_line":"            # factory so we can avoid having a global flag guarding against"},{"line_number":621,"context_line":"            # factory re-configuration"},{"line_number":622,"context_line":"            self.factory_reset \u003d main_db_api.context_manager.patch_factory("},{"line_number":623,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":624,"context_line":"            main_db_api.configure(CONF)"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"            if connection is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"30ecc883_b03b6ae7","line":623,"updated":"2021-10-27 18:10:03.000000000","message":"Based on [1], it sounds like we\u0027re patching the wrong context manager here and we should call this on the ctxt_mgr created below (line 627)? Can that happen here or does it have to happen in the later change (which says it\u0027s only for cleanup)?\n\n[1] https://review.opendev.org/c/openstack/nova/+/815690/comment/f743e4d8_22fc8e3f/","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9a9286254a6c621adddb17d6059956a39966944e","unresolved":false,"context_lines":[{"line_number":620,"context_line":"            # factory so we can avoid having a global flag guarding against"},{"line_number":621,"context_line":"            # factory re-configuration"},{"line_number":622,"context_line":"            self.factory_reset \u003d main_db_api.context_manager.patch_factory("},{"line_number":623,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":624,"context_line":"            main_db_api.configure(CONF)"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"            if connection is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"48063308_ac2fad7c","line":623,"in_reply_to":"30ecc883_b03b6ae7","updated":"2021-11-04 16:27:30.000000000","message":"No, this is definitely needed. L627 was the case I thought I missed when I wrote [1]. \n\nBut I\u0027m lost again. Based on Melanie\u0027s comment[2] we don\u0027t need to patch if the connection is provided, as that result in a separate transaction factory.\n\n[2] https://review.opendev.org/c/openstack/nova/+/815690/comment/e5f08cad_342149e6/","commit_id":"fa2ac1c3f5bee419dc1db862fdc0aa37c4fb1a8e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f33c918c26a344573fafdbb542ec769cb6f82059","unresolved":true,"context_lines":[{"line_number":625,"context_line":"            self.factory_reset \u003d main_db_api.context_manager.patch_factory("},{"line_number":626,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":627,"context_line":"            main_db_api.configure(CONF)"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"            if connection is not None:"},{"line_number":630,"context_line":"                ctxt_mgr \u003d main_db_api.create_context_manager("},{"line_number":631,"context_line":"                    connection\u003dconnection)"}],"source_content_type":"text/x-python","patch_set":2,"id":"33636a72_663860ee","line":628,"updated":"2021-11-05 00:19:25.000000000","message":"Sorry I didn\u0027t mention this earlier ... but I had kept getting confused on whether there\u0027s a more proper way to do this temporary factory swap, without using private class directly from oslo.db.\n\nAfter thinking about it more, I think using the fixture from oslo.db [1] would work instead:\n\n new_engine \u003d enginefacade.transaction_context()\n self.useFixture(ReplaceEngineFacade(main_db_api.context_manager, new_engine))\n main_db_api.configure(CONF)\n\nand you wouldn\u0027t need self.factory_reset.\n\n[1] https://github.com/openstack/oslo.db/blob/d7742015f732625ff0c3e455ac5e1c01ba3d9a45/oslo_db/sqlalchemy/test_fixtures.py#L27","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8256a974856d89258cd8c2db0a05a40421d7fe67","unresolved":false,"context_lines":[{"line_number":625,"context_line":"            self.factory_reset \u003d main_db_api.context_manager.patch_factory("},{"line_number":626,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":627,"context_line":"            main_db_api.configure(CONF)"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"            if connection is not None:"},{"line_number":630,"context_line":"                ctxt_mgr \u003d main_db_api.create_context_manager("},{"line_number":631,"context_line":"                    connection\u003dconnection)"}],"source_content_type":"text/x-python","patch_set":2,"id":"c547ee33_b1d5fb6b","line":628,"in_reply_to":"33636a72_663860ee","updated":"2021-11-05 10:59:19.000000000","message":"Excellent point. Unfortunately I cannot do that directly in this patch as fixtures cannot be create during __init__ just during setUp (as they depend on things the base class setUp creates for proper cleanup handling). So we need this hack temporarily. Then the next patch in the series moves the dynamic things from __init__ to setUp and then I can add an extra patch to the series that replace the current hack with the usage of ReplaceEngineFacade.\n\nDone in https://review.opendev.org/c/openstack/nova/+/816820","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"22876143fd426e82e63816bb13e08368c9fd64e6","unresolved":false,"context_lines":[{"line_number":625,"context_line":"            self.factory_reset \u003d main_db_api.context_manager.patch_factory("},{"line_number":626,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":627,"context_line":"            main_db_api.configure(CONF)"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"            if connection is not None:"},{"line_number":630,"context_line":"                ctxt_mgr \u003d main_db_api.create_context_manager("},{"line_number":631,"context_line":"                    connection\u003dconnection)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bd2e9ae4_f41a3372","line":628,"in_reply_to":"c547ee33_b1d5fb6b","updated":"2021-11-09 08:39:52.000000000","message":"Understood, thanks for the details.","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f33c918c26a344573fafdbb542ec769cb6f82059","unresolved":true,"context_lines":[{"line_number":640,"context_line":"            self.factory_reset \u003d api_db_api.context_manager.patch_factory("},{"line_number":641,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":642,"context_line":"            api_db_api.configure(CONF)"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":"            self.get_engine \u003d api_db_api.get_engine"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"38146337_8ab97d22","line":643,"updated":"2021-11-05 00:19:25.000000000","message":"Same here except s/main_db_api/api_db_api/","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8256a974856d89258cd8c2db0a05a40421d7fe67","unresolved":false,"context_lines":[{"line_number":640,"context_line":"            self.factory_reset \u003d api_db_api.context_manager.patch_factory("},{"line_number":641,"context_line":"                enginefacade._TransactionFactory())"},{"line_number":642,"context_line":"            api_db_api.configure(CONF)"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":"            self.get_engine \u003d api_db_api.get_engine"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e168a53b_f95bd2ac","line":643,"in_reply_to":"38146337_8ab97d22","updated":"2021-11-05 10:59:19.000000000","message":"Done","commit_id":"f318f822fcf6dec4c3cd9b7e5111f3e1371aa51a"}]}