)]}'
{"nova/compute/api.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4df8a4ebc03a2aaa07bd64351d44f7f218c58d58","unresolved":false,"context_lines":[{"line_number":112,"context_line":"# trigger."},{"line_number":113,"context_line":"CELLS \u003d []"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"_NO_STATE \u003d object()"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def check_instance_state(vm_state\u003dNone, task_state\u003dNone, launched\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_87ec3d2b","line":115,"updated":"2017-03-03 21:49:53.000000000","message":"I\u0027m not familiar with this idiom...why this rather than setting _NO_STATE\u003dset([None]) and skipping the lines at 135/136 ?","commit_id":"17a67d51510cc9beb4587f4197b465158127ddb0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d2e7377202c3a1b89c8f51fe43462abf6557734a","unresolved":false,"context_lines":[{"line_number":112,"context_line":"# trigger."},{"line_number":113,"context_line":"CELLS \u003d []"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"_NO_STATE \u003d object()"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def check_instance_state(vm_state\u003dNone, task_state\u003dNone, launched\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffe62b97_2bb385ec","line":115,"in_reply_to":"ba2be162_87ec3d2b","updated":"2017-03-27 14:52:26.000000000","message":"It\u0027s actually a reasonably common idiom: it\u0027s just a shorthand which creates a unique, immutablish sentinel. It\u0027s normally used as an argument default which I\u0027m not doing there, but I\u0027m still using it as an argument sentinel (see line 2208).\n\nI\u0027m going to change it anyway, though: it\u0027s never returned or passed to anything so there\u0027s no risk of it being modified. I\u0027m consequently also going to change its name to be more specific (I think I was originally going to use it for vm_state too, but that doesn\u0027t make sense).","commit_id":"17a67d51510cc9beb4587f4197b465158127ddb0"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"0759ba7a53e154e1360560737745e5e333f2b082","unresolved":false,"context_lines":[{"line_number":115,"context_line":"_NO_STATE \u003d object()"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def check_instance_state(vm_state\u003dNone, task_state\u003dNone, launched\u003dTrue):"},{"line_number":119,"context_line":"    \"\"\"Decorator to check VM and/or task state before entry to API functions."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    If the instance is in the wrong state, or has not been successfully"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffe62b97_c8166af2","line":118,"updated":"2017-03-27 08:14:19.000000000","message":"I believe to be readable it\u0027s worth to describe params,\nlike:\n\nvm_state: (None, string, list)\n  None - state is not checked\n  ...\ntask_state: (None, _NO_STATE, string, list)\n  None - state is not checked\n  _NO_STATE - state has to be None\n  ...\n\nIt\u0027s really not explicit.","commit_id":"17a67d51510cc9beb4587f4197b465158127ddb0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d2e7377202c3a1b89c8f51fe43462abf6557734a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"_NO_STATE \u003d object()"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def check_instance_state(vm_state\u003dNone, task_state\u003dNone, launched\u003dTrue):"},{"line_number":119,"context_line":"    \"\"\"Decorator to check VM and/or task state before entry to API functions."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    If the instance is in the wrong state, or has not been successfully"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffe62b97_6b2ced01","line":118,"in_reply_to":"ffe62b97_c8166af2","updated":"2017-03-27 14:52:26.000000000","message":"Actually I should probably have updated the whole text to describe the new behaviour. I\u0027ll do that.","commit_id":"17a67d51510cc9beb4587f4197b465158127ddb0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5c1ae3b2401300ad8dbc996086a3e6c8bff22ad4","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        # also assert that there is no task state, so for brevity we make that"},{"line_number":128,"context_line":"        # the default."},{"line_number":129,"context_line":"        if task_state is None:"},{"line_number":130,"context_line":"            task_state \u003d _NO_STATE"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if not isinstance(vm_state, set):"},{"line_number":133,"context_line":"            vm_state \u003d set(vm_state)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a1ced50_ce6cc4d2","line":130,"range":{"start_line":130,"start_character":25,"end_line":130,"end_character":34},"updated":"2017-11-22 11:01:27.000000000","message":"Yeah, setting this to \u0027set([None])\u0027","commit_id":"17a67d51510cc9beb4587f4197b465158127ddb0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5c1ae3b2401300ad8dbc996086a3e6c8bff22ad4","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        if task_state is None:"},{"line_number":145,"context_line":"            task_state \u003d _NO_TASK_STATE"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if not isinstance(vm_state, set):"},{"line_number":148,"context_line":"            vm_state \u003d set(vm_state)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    if task_state is not None and not isinstance(task_state, set):"},{"line_number":151,"context_line":"        task_state \u003d set(task_state)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_d06081a8","line":148,"range":{"start_line":147,"start_character":8,"end_line":148,"end_character":36},"updated":"2017-11-22 11:01:27.000000000","message":"I\u0027m guessing this is here to allow us to use both a list and a single variable for this attribute","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5c1ae3b2401300ad8dbc996086a3e6c8bff22ad4","unresolved":false,"context_lines":[{"line_number":2275,"context_line":"    # long as the instance has been launched."},{"line_number":2276,"context_line":"    @check_instance_lock"},{"line_number":2277,"context_line":"    @check_instance_cell"},{"line_number":2278,"context_line":"    @check_instance_state()"},{"line_number":2279,"context_line":"    def soft_delete(self, context, instance):"},{"line_number":2280,"context_line":"        \"\"\"Terminate an instance.\"\"\""},{"line_number":2281,"context_line":"        LOG.debug(\u0027Going to try to soft delete instance\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_b55a8f27","line":2278,"range":{"start_line":2278,"start_character":25,"end_line":2278,"end_character":27},"updated":"2017-11-22 11:01:27.000000000","message":"nit: do you need these still?","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"65d2f59c899406bfdc3498b42d313d2923b6fa62","unresolved":false,"context_lines":[{"line_number":2275,"context_line":"    # long as the instance has been launched."},{"line_number":2276,"context_line":"    @check_instance_lock"},{"line_number":2277,"context_line":"    @check_instance_cell"},{"line_number":2278,"context_line":"    @check_instance_state()"},{"line_number":2279,"context_line":"    def soft_delete(self, context, instance):"},{"line_number":2280,"context_line":"        \"\"\"Terminate an instance.\"\"\""},{"line_number":2281,"context_line":"        LOG.debug(\u0027Going to try to soft delete instance\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_8bd16ad0","line":2278,"range":{"start_line":2278,"start_character":25,"end_line":2278,"end_character":27},"in_reply_to":"ff82abbf_b55a8f27","updated":"2017-11-22 12:14:24.000000000","message":"Yes it is needed. A decorator shall be a function that takes the decorated function as a parameter. By calling check_instance_state() we get back outer from L153 that takes the decorated function as parameter. Instead @check_instance_state would mean that the check_instance_state function will take the decorated function as parameter. Which is not true our case.","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5c1ae3b2401300ad8dbc996086a3e6c8bff22ad4","unresolved":false,"context_lines":[{"line_number":4185,"context_line":"        # We allow creating the snapshot in any vm_state as long as there is"},{"line_number":4186,"context_line":"        # no task being performed on the instance and it has a host."},{"line_number":4187,"context_line":"        @check_instance_host"},{"line_number":4188,"context_line":"        @check_instance_state(task_state\u003d_NO_TASK_STATE)"},{"line_number":4189,"context_line":"        def do_volume_snapshot_create(self, context, instance):"},{"line_number":4190,"context_line":"            self.compute_rpcapi.volume_snapshot_create(context, instance,"},{"line_number":4191,"context_line":"                    volume_id, create_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_70c17580","line":4188,"range":{"start_line":4188,"start_character":41,"end_line":4188,"end_character":55},"updated":"2017-11-22 11:01:27.000000000","message":"So here\u0027s the first bug","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5c1ae3b2401300ad8dbc996086a3e6c8bff22ad4","unresolved":false,"context_lines":[{"line_number":4207,"context_line":"        # We allow deleting the snapshot in any vm_state as long as there is"},{"line_number":4208,"context_line":"        # no task being performed on the instance and it has a host."},{"line_number":4209,"context_line":"        @check_instance_host"},{"line_number":4210,"context_line":"        @check_instance_state(task_state\u003d_NO_TASK_STATE)"},{"line_number":4211,"context_line":"        def do_volume_snapshot_delete(self, context, instance):"},{"line_number":4212,"context_line":"            self.compute_rpcapi.volume_snapshot_delete(context, instance,"},{"line_number":4213,"context_line":"                    volume_id, snapshot_id, delete_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_10c0297b","line":4210,"range":{"start_line":4210,"start_character":41,"end_line":4210,"end_character":55},"updated":"2017-11-22 11:01:27.000000000","message":"and the second.\n\nDo we need tests for these? Is it something that could even be tested?","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"65d2f59c899406bfdc3498b42d313d2923b6fa62","unresolved":false,"context_lines":[{"line_number":4207,"context_line":"        # We allow deleting the snapshot in any vm_state as long as there is"},{"line_number":4208,"context_line":"        # no task being performed on the instance and it has a host."},{"line_number":4209,"context_line":"        @check_instance_host"},{"line_number":4210,"context_line":"        @check_instance_state(task_state\u003d_NO_TASK_STATE)"},{"line_number":4211,"context_line":"        def do_volume_snapshot_delete(self, context, instance):"},{"line_number":4212,"context_line":"            self.compute_rpcapi.volume_snapshot_delete(context, instance,"},{"line_number":4213,"context_line":"                    volume_id, snapshot_id, delete_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_cb1a127c","line":4210,"range":{"start_line":4210,"start_character":41,"end_line":4210,"end_character":55},"in_reply_to":"ff82abbf_10c0297b","updated":"2017-11-22 12:14:24.000000000","message":"I agree to add some test to these cases. Also it seems that we have no unit test coverage for the check_instance_state decorator either.","commit_id":"fbf5972645cf7730e25a7c1410a22961e2b6b595"}]}