)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d21a4130fbabbfabd5a99bc25db2f2ec178c2555","unresolved":false,"context_lines":[{"line_number":3486,"context_line":""},{"line_number":3487,"context_line":"                    # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":3488,"context_line":"                    # mark the instance as belonging to this host."},{"line_number":3489,"context_line":"                    self._set_migration_status(migration, \u0027done\u0027)"},{"line_number":3490,"context_line":""},{"line_number":3491,"context_line":"    def _do_rebuild_instance_with_claim("},{"line_number":3492,"context_line":"            self, context, instance, orig_image_ref, image_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_45a094a6","line":3489,"updated":"2020-10-13 15:53:29.000000000","message":"This is a perfectly sensible solution. However, we already have things like \u0027abort_instance_claim\u0027 and \u0027drop_move_claim\u0027 in the resource tracker. Would, a new \u0027confirm_evacuate_claim\u0027 helper be reasonable? (wrapped in the \u0027@utils.synchronized\u0027 decorator, naturally)","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a25ccbcf6e9b0b5138c703a2f17c085e5c07da8f","unresolved":false,"context_lines":[{"line_number":3486,"context_line":""},{"line_number":3487,"context_line":"                    # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":3488,"context_line":"                    # mark the instance as belonging to this host."},{"line_number":3489,"context_line":"                    self._set_migration_status(migration, \u0027done\u0027)"},{"line_number":3490,"context_line":""},{"line_number":3491,"context_line":"    def _do_rebuild_instance_with_claim("},{"line_number":3492,"context_line":"            self, context, instance, orig_image_ref, image_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_3eb8fb28","line":3489,"in_reply_to":"7f6b1bfe_272bd294","updated":"2020-10-14 14:08:56.000000000","message":"Restored the solution from PS1","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d9dcb6ee2174c1d431dd9d439baaad2670325ec8","unresolved":false,"context_lines":[{"line_number":3486,"context_line":""},{"line_number":3487,"context_line":"                    # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":3488,"context_line":"                    # mark the instance as belonging to this host."},{"line_number":3489,"context_line":"                    self._set_migration_status(migration, \u0027done\u0027)"},{"line_number":3490,"context_line":""},{"line_number":3491,"context_line":"    def _do_rebuild_instance_with_claim("},{"line_number":3492,"context_line":"            self, context, instance, orig_image_ref, image_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_89c62967","line":3489,"in_reply_to":"7f6b1bfe_3eb8fb28","updated":"2020-10-15 08:57:16.000000000","message":"Thanks. If you do end up reworking these, feel free to stick me on the review. I\u0027m all for it :)","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd7feefa4db07f1644a2434d89e297d098daa010","unresolved":false,"context_lines":[{"line_number":3486,"context_line":""},{"line_number":3487,"context_line":"                    # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":3488,"context_line":"                    # mark the instance as belonging to this host."},{"line_number":3489,"context_line":"                    self._set_migration_status(migration, \u0027done\u0027)"},{"line_number":3490,"context_line":""},{"line_number":3491,"context_line":"    def _do_rebuild_instance_with_claim("},{"line_number":3492,"context_line":"            self, context, instance, orig_image_ref, image_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_64c2e8eb","line":3489,"in_reply_to":"7f6b1bfe_45a094a6","updated":"2020-10-14 09:18:18.000000000","message":"That was basically my first solution in PS1[1]. But I did not like to move the logic out from here just to warp it with a lock. I think that solution trades the readability to solve a technicality. \n\nI know that this is now a different style that we had before. But honestly I also feel bad about our existing style:\n\n    def function_without_lock():\n        \n        @utils.synchronized(lock-name)\n        def function_with_lock():\n            # logic or even another _do_function function call\n        \n        function_with_lock()\n\n\nIt would be a lot lest cruft to say:\n\n    def function():\n        with lock:\n            # logic \n\nSure the later needs an actual lock objects instead of lock-name hence the property returning the lock from the RT.\n\nI\u0027m OK to go back to PS1 style if the goal is to preserve style I just feel bad about it.\n\n[1]https://review.opendev.org/#/c/754815/1/nova/compute/resource_tracker.py@1991","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7eec30a2bf7852a06e90cf81351d15787d8f353f","unresolved":false,"context_lines":[{"line_number":3486,"context_line":""},{"line_number":3487,"context_line":"                    # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":3488,"context_line":"                    # mark the instance as belonging to this host."},{"line_number":3489,"context_line":"                    self._set_migration_status(migration, \u0027done\u0027)"},{"line_number":3490,"context_line":""},{"line_number":3491,"context_line":"    def _do_rebuild_instance_with_claim("},{"line_number":3492,"context_line":"            self, context, instance, orig_image_ref, image_meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_272bd294","line":3489,"in_reply_to":"7f6b1bfe_64c2e8eb","updated":"2020-10-14 10:06:20.000000000","message":"Personally, I prefer the PS1 style purely because that means everything\u0027s being done the same way and once I understand one path then I understand them all. With that said, I agree though that going with context managers rather than decorators could lead to much less of this inner function silliness. I guess it depends on whether we ever plan to copy this approach to the existing examples and remove e.g. the \u0027drop_move_claim_at_dest\u0027 and \u0027abort_instance_claim\u0027 helpers. If so, let\u0027s do this. If not, let\u0027s keeping the same pattern across similar code paths?","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"acc7ee4a83b6c6de17edc9582d6ff4a1eb974b00","unresolved":false,"context_lines":[{"line_number":1972,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair\u003dTrue)"},{"line_number":1975,"context_line":"    def finish_evacuation(self, instance, node, migration):"},{"line_number":1976,"context_line":"        instance.apply_migration_context()"},{"line_number":1977,"context_line":"        # NOTE (ndipanov): This save will now update the host and node"},{"line_number":1978,"context_line":"        # attributes making sure that next RT pass is consistent since"},{"line_number":1979,"context_line":"        # it will be based on the instance and not the migration DB"},{"line_number":1980,"context_line":"        # entry."},{"line_number":1981,"context_line":"        print(\u0027node\u0027, node)"},{"line_number":1982,"context_line":"        instance.host \u003d self.host"},{"line_number":1983,"context_line":"        instance.node \u003d node"},{"line_number":1984,"context_line":"        instance.save()"},{"line_number":1985,"context_line":"        instance.drop_migration_context()"},{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"        # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":1988,"context_line":"        # mark the instance as belonging to this host."},{"line_number":1989,"context_line":"        if migration:"},{"line_number":1990,"context_line":"            migration.status \u003d \u0027done\u0027"},{"line_number":1991,"context_line":"            migration.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_8f6da005","line":1991,"range":{"start_line":1975,"start_character":1,"end_line":1991,"end_character":28},"updated":"2020-09-29 07:26:45.000000000","message":"I hate to move this out from the compute manager rebuild flow but it seems that the COMPUTE_RESOURCE_SEMAPHORE is owned by the resource tracker and there is no easy way to provide a context manager locking the semaphore as nova built infra around the synchronized decorator.","commit_id":"80bd2bc125fec38375a381d987bc46d804a100fd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"906e083c20077094bab013ec5c984dbe3370209c","unresolved":false,"context_lines":[{"line_number":1972,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair\u003dTrue)"},{"line_number":1975,"context_line":"    def finish_evacuation(self, instance, node, migration):"},{"line_number":1976,"context_line":"        instance.apply_migration_context()"},{"line_number":1977,"context_line":"        # NOTE (ndipanov): This save will now update the host and node"},{"line_number":1978,"context_line":"        # attributes making sure that next RT pass is consistent since"},{"line_number":1979,"context_line":"        # it will be based on the instance and not the migration DB"},{"line_number":1980,"context_line":"        # entry."},{"line_number":1981,"context_line":"        print(\u0027node\u0027, node)"},{"line_number":1982,"context_line":"        instance.host \u003d self.host"},{"line_number":1983,"context_line":"        instance.node \u003d node"},{"line_number":1984,"context_line":"        instance.save()"},{"line_number":1985,"context_line":"        instance.drop_migration_context()"},{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"        # NOTE (ndipanov): Mark the migration as done only after we"},{"line_number":1988,"context_line":"        # mark the instance as belonging to this host."},{"line_number":1989,"context_line":"        if migration:"},{"line_number":1990,"context_line":"            migration.status \u003d \u0027done\u0027"},{"line_number":1991,"context_line":"            migration.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_ecdee53a","line":1991,"range":{"start_line":1975,"start_character":1,"end_line":1991,"end_character":28},"in_reply_to":"9f560f44_8f6da005","updated":"2020-09-29 11:34:13.000000000","message":"OK, I think I solved this.","commit_id":"80bd2bc125fec38375a381d987bc46d804a100fd"},{"author":{"_account_id":24713,"name":"Wonil Choi","email":"wonil0522@gmail.com","username":"wonil22"},"change_message_id":"2d030484e246c63a9b05f74bbb9ee8781480df1c","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        self.pci_tracker.free_instance_claims(context, instance)"},{"line_number":1972,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"    def compute_resource_semaphore(self):"},{"line_number":1975,"context_line":"        \"\"\"Returns a context manager that allows locking the"},{"line_number":1976,"context_line":"        COMPUTE_RESOURCE_SEMAPHORE"},{"line_number":1977,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_d8ed40f1","line":1974,"range":{"start_line":1974,"start_character":8,"end_line":1974,"end_character":34},"updated":"2020-10-06 03:01:45.000000000","message":"lock_compute_resource_semaphore() is better, isn\u0027t it ?","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dd7feefa4db07f1644a2434d89e297d098daa010","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        self.pci_tracker.free_instance_claims(context, instance)"},{"line_number":1972,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"    def compute_resource_semaphore(self):"},{"line_number":1975,"context_line":"        \"\"\"Returns a context manager that allows locking the"},{"line_number":1976,"context_line":"        COMPUTE_RESOURCE_SEMAPHORE"},{"line_number":1977,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_873626b4","line":1974,"range":{"start_line":1974,"start_character":8,"end_line":1974,"end_character":34},"in_reply_to":"9f560f44_d8ed40f1","updated":"2020-10-14 09:18:18.000000000","message":"It would be misleading. This function call itself does not lock the underlying semaphore. It just returns a context manager wrapping that semaphore.  The caller needs to activate the context manager by putting it in a `with` construct to have it locked.","commit_id":"12446c168d44709df6c1735ed97ed1df2ff2e321"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9b2884be117cdeebcdc7e49631c2188c382aa338","unresolved":false,"context_lines":[{"line_number":1978,"context_line":"        # attributes making sure that next RT pass is consistent since"},{"line_number":1979,"context_line":"        # it will be based on the instance and not the migration DB"},{"line_number":1980,"context_line":"        # entry."},{"line_number":1981,"context_line":"        print(\u0027node\u0027, node)"},{"line_number":1982,"context_line":"        instance.host \u003d self.host"},{"line_number":1983,"context_line":"        instance.node \u003d node"},{"line_number":1984,"context_line":"        instance.save()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f6b1bfe_290ef58c","line":1981,"range":{"start_line":1981,"start_character":0,"end_line":1981,"end_character":27},"updated":"2020-10-15 09:14:29.000000000","message":"whoops","commit_id":"c0aa3ab286ac3e9f122cb428b1626721f65e67fd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f0be8d05c67881c530f3416b7b92bfaa4a4eab8d","unresolved":false,"context_lines":[{"line_number":1978,"context_line":"        # attributes making sure that next RT pass is consistent since"},{"line_number":1979,"context_line":"        # it will be based on the instance and not the migration DB"},{"line_number":1980,"context_line":"        # entry."},{"line_number":1981,"context_line":"        print(\u0027node\u0027, node)"},{"line_number":1982,"context_line":"        instance.host \u003d self.host"},{"line_number":1983,"context_line":"        instance.node \u003d node"},{"line_number":1984,"context_line":"        instance.save()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f681702_498ef127","line":1981,"range":{"start_line":1981,"start_character":0,"end_line":1981,"end_character":27},"in_reply_to":"7f6b1bfe_290ef58c","updated":"2020-10-16 11:31:05.000000000","message":":facepalm:\nDone.","commit_id":"c0aa3ab286ac3e9f122cb428b1626721f65e67fd"}]}