)]}'
{"nova/compute/api.py":[{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":51,"context_line":"flags.DEFINE_integer(\u0027find_host_timeout\u0027, 30,"},{"line_number":52,"context_line":"                     \u0027Timeout after NN seconds when looking for a host.\u0027)"},{"line_number":53,"context_line":"flags.DEFINE_boolean(\u0027api_check_vm_states\u0027, True,"},{"line_number":54,"context_line":"                     \u0027Filter calls by vm state\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F91k%3D","line":54,"updated":"2011-11-16 22:31:20.000000000","message":"When would you want to turn this off? I would much rather have the ability to plug in my own vm-state transition logic if that\u0027s the point of this flag.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":51,"context_line":"flags.DEFINE_integer(\u0027find_host_timeout\u0027, 30,"},{"line_number":52,"context_line":"                     \u0027Timeout after NN seconds when looking for a host.\u0027)"},{"line_number":53,"context_line":"flags.DEFINE_boolean(\u0027api_check_vm_states\u0027, True,"},{"line_number":54,"context_line":"                     \u0027Filter calls by vm state\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9uE%3D","line":54,"in_reply_to":"AAAACn%2F%2F91k%3D","updated":"2011-11-17 11:57:57.000000000","message":"good point!\nInstead of enabling or disabling the mechanism we could be able to define which states are allowed or blocked from the flag. I will investigate if it is possible to set a flag to do it: I have a couple of ideas.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":51,"context_line":"flags.DEFINE_integer(\u0027find_host_timeout\u0027, 30,"},{"line_number":52,"context_line":"                     \u0027Timeout after NN seconds when looking for a host.\u0027)"},{"line_number":53,"context_line":"flags.DEFINE_boolean(\u0027api_check_vm_states\u0027, True,"},{"line_number":54,"context_line":"                     \u0027Filter calls by vm state\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9IM%3D","line":54,"in_reply_to":"AAAACn%2F%2F9uE%3D","updated":"2011-11-21 18:24:15.000000000","message":"we check if the methods are allowed or blocked according to the dictionaries defined in state_checker. I think the necessary flags would be very complicated and confusing. Do you think we can keep the boolean flag for debugging proposes?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    return True"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"class check_vm_state(object):"},{"line_number":72,"context_line":"    \"\"\"Class to wrap API functions that are sensitive to the VM state."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    If the instance is in the wrong state, the wrapper will raise an exception."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F91g%3D","line":71,"updated":"2011-11-16 22:31:20.000000000","message":"Does this have to be a class decorator? Why not a method?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    return True"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"class check_vm_state(object):"},{"line_number":72,"context_line":"    \"\"\"Class to wrap API functions that are sensitive to the VM state."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    If the instance is in the wrong state, the wrapper will raise an exception."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9uA%3D","line":71,"in_reply_to":"AAAACn%2F%2F91g%3D","updated":"2011-11-17 11:57:57.000000000","message":"I tried with a method, but we need the function name to lookup it in state_checker.is_blocked(). I know we can get the name of the function using f.__name__, but if the decorated function has other decorators, the f.__name__ could be the name of the other decorator. This would mean that the order of the decorators decides if the mechanism works or not, and I didn\u0027t like that: it would cause problems and could be potentially hard to debug. To solve this, I decided to create this class (I was inspired by scheduler_api.reroute_compute(...).","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":831,"context_line":"        self._do_restore(context, instance)"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @check_vm_state(state_checker.RESTORE)"},{"line_number":834,"context_line":"    @scheduler_api.reroute_compute(\"restore\")"},{"line_number":835,"context_line":"    def _do_restore(self, context, instance):"},{"line_number":836,"context_line":"        \"\"\"Restore implementation.\"\"\""},{"line_number":837,"context_line":"        if not _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F908%3D","line":834,"updated":"2011-11-16 22:31:20.000000000","message":"Should this be on the \u0027public\u0027 restore() method and not _do_restore()?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":831,"context_line":"        self._do_restore(context, instance)"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @check_vm_state(state_checker.RESTORE)"},{"line_number":834,"context_line":"    @scheduler_api.reroute_compute(\"restore\")"},{"line_number":835,"context_line":"    def _do_restore(self, context, instance):"},{"line_number":836,"context_line":"        \"\"\"Restore implementation.\"\"\""},{"line_number":837,"context_line":"        if not _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9t8%3D","line":834,"in_reply_to":"AAAACn%2F%2F908%3D","updated":"2011-11-17 11:57:57.000000000","message":"Originally, do_restore didn\u0027t exist. I created it passing as parameter the instance reference, because that is what the check_vm_state expects (and all the other public methods use the reference, and not the id). The other option was to modify the check_vm_state, but this would complicate the logic (it should analyze the type of the argument) and potentially do more db queries. If it is more clear, I can modify the check_vm_state, but I assumed that at some point restore will have the instance reference as a parameter (all the other methods were originally expecting he instance_id, and now expect the instance reference: I guess this is related to the fact that the API is moving from getting instance_id to instance_uuid).\nAnyway, maybe I should have left the scheduler_api decorator in the public restore.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":831,"context_line":"        self._do_restore(context, instance)"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @check_vm_state(state_checker.RESTORE)"},{"line_number":834,"context_line":"    @scheduler_api.reroute_compute(\"restore\")"},{"line_number":835,"context_line":"    def _do_restore(self, context, instance):"},{"line_number":836,"context_line":"        \"\"\"Restore implementation.\"\"\""},{"line_number":837,"context_line":"        if not _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9Jc%3D","line":834,"in_reply_to":"AAAACn%2F%2F9t8%3D","updated":"2011-11-21 18:24:15.000000000","message":"Done","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"                {\u0027host\u0027: migration_ref[\u0027dest_compute\u0027], })"},{"line_number":1263,"context_line":""},{"line_number":1264,"context_line":"    @scheduler_api.reroute_compute(state_checker.RESIZE)"},{"line_number":1265,"context_line":"    @check_vm_state(state_checker.RESIZE)"},{"line_number":1266,"context_line":"    def resize(self, context, instance, flavor_id\u003dNone):"},{"line_number":1267,"context_line":"        \"\"\"Resize (ie, migrate) a running instance."},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F91E%3D","line":1265,"updated":"2011-11-16 22:31:20.000000000","message":"Some of these decorators are inconsistent in their decoration order, does it matter? This one and revert_resize() seem to be swapped.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"                {\u0027host\u0027: migration_ref[\u0027dest_compute\u0027], })"},{"line_number":1263,"context_line":""},{"line_number":1264,"context_line":"    @scheduler_api.reroute_compute(state_checker.RESIZE)"},{"line_number":1265,"context_line":"    @check_vm_state(state_checker.RESIZE)"},{"line_number":1266,"context_line":"    def resize(self, context, instance, flavor_id\u003dNone):"},{"line_number":1267,"context_line":"        \"\"\"Resize (ie, migrate) a running instance."},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9Jg%3D","line":1265,"in_reply_to":"AAAACn%2F%2F91E%3D","updated":"2011-11-21 18:24:15.000000000","message":"Done","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"                {\u0027host\u0027: migration_ref[\u0027dest_compute\u0027], })"},{"line_number":1263,"context_line":""},{"line_number":1264,"context_line":"    @scheduler_api.reroute_compute(state_checker.RESIZE)"},{"line_number":1265,"context_line":"    @check_vm_state(state_checker.RESIZE)"},{"line_number":1266,"context_line":"    def resize(self, context, instance, flavor_id\u003dNone):"},{"line_number":1267,"context_line":"        \"\"\"Resize (ie, migrate) a running instance."},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9t4%3D","line":1265,"in_reply_to":"AAAACn%2F%2F91E%3D","updated":"2011-11-17 11:57:57.000000000","message":"OK, I will make it consistent","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"17467f6d04757f4e068b794eb1c4f1d1aa57547e","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            if FLAGS.api_check_vm_states and \\"},{"line_number":84,"context_line":"            state_checker.is_blocked(self.method_name, context, instance_ref):"},{"line_number":85,"context_line":"                raise exception.InstanceInvalidState(\\"},{"line_number":86,"context_line":"                    instance_id\u003dinstance_ref[\u0027id\u0027], method\u003dself.method_name)"},{"line_number":87,"context_line":"            else:"},{"line_number":88,"context_line":"                return f(api, context, instance_ref, *args, **kw)"},{"line_number":89,"context_line":"        return _state_checker_wrap"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2F1E%3D","line":86,"updated":"2011-12-01 05:53:57.000000000","message":"Should we not use uuid here instead?  We\u0027re moving to get all logs using uuid, etc.","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ab060f14a8ac0219b8e8c9f41bc4f6b6e8fe38ee","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            if FLAGS.api_check_vm_states and \\"},{"line_number":84,"context_line":"            state_checker.is_blocked(self.method_name, context, instance_ref):"},{"line_number":85,"context_line":"                raise exception.InstanceInvalidState(\\"},{"line_number":86,"context_line":"                    instance_id\u003dinstance_ref[\u0027id\u0027], method\u003dself.method_name)"},{"line_number":87,"context_line":"            else:"},{"line_number":88,"context_line":"                return f(api, context, instance_ref, *args, **kw)"},{"line_number":89,"context_line":"        return _state_checker_wrap"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F9wU%3D","line":86,"in_reply_to":"AAAAC3%2F%2F%2F1E%3D","updated":"2011-12-12 14:56:30.000000000","message":"Done","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"924ebeca41f0ecde32cf9dbeb0f36786795c11e1","unresolved":false,"context_lines":[{"line_number":51,"context_line":"flags.DEFINE_integer(\u0027find_host_timeout\u0027, 30,"},{"line_number":52,"context_line":"                     \u0027Timeout after NN seconds when looking for a host.\u0027)"},{"line_number":53,"context_line":"flags.DEFINE_boolean(\u0027api_check_vm_states\u0027, True,"},{"line_number":54,"context_line":"                     \u0027Filter calls by vm state\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def _is_queued_delete(instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAC3%2F%2F9h8%3D","line":54,"updated":"2011-12-12 22:03:09.000000000","message":"When would you ever want to set this to False? It seems like it would allow unsafe behavior to do so.","commit_id":"676d1d50d7b196f40c7d4d214dd12f9772fed163"}],"nova/compute/manager.py":[{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                            \u0027compute.instance.create\u0027,"},{"line_number":503,"context_line":"                            notifier.INFO, usage_info)"},{"line_number":504,"context_line":"            if self._is_instance_terminated(instance_id):"},{"line_number":505,"context_line":"                raise exception.InstanceNotFound"},{"line_number":506,"context_line":"        except exception.InstanceNotFound:"},{"line_number":507,"context_line":"            # NOTE (David Subiros): this exception will be raised if"},{"line_number":508,"context_line":"            # the instance was immediately deleted before it actually got"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F90o%3D","line":505,"updated":"2011-11-16 22:31:20.000000000","message":"Isn\u0027t in the responsibility of the delete method to ensure shutdown when deleting the instance?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                            \u0027compute.instance.create\u0027,"},{"line_number":503,"context_line":"                            notifier.INFO, usage_info)"},{"line_number":504,"context_line":"            if self._is_instance_terminated(instance_id):"},{"line_number":505,"context_line":"                raise exception.InstanceNotFound"},{"line_number":506,"context_line":"        except exception.InstanceNotFound:"},{"line_number":507,"context_line":"            # NOTE (David Subiros): this exception will be raised if"},{"line_number":508,"context_line":"            # the instance was immediately deleted before it actually got"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9tw%3D","line":505,"in_reply_to":"AAAACn%2F%2F90o%3D","updated":"2011-11-17 11:57:57.000000000","message":"The idea was to check if the instance was deleted before it got created, to make sure it is really terminated in that case. The other option was to disable the instance termination if it is still building, but this could be a wrong behaviour from the user point of view, and could potentially keep instances undeletable if they were blocked in building state forever.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":507,"context_line":"            # NOTE (David Subiros): this exception will be raised if"},{"line_number":508,"context_line":"            # the instance was immediately deleted before it actually got"},{"line_number":509,"context_line":"            # created. Running shutdown again to make sure it is really deleted"},{"line_number":510,"context_line":"            self._shutdown_instance_even_if_deleted(context, instance_id)"},{"line_number":511,"context_line":"        except Exception:"},{"line_number":512,"context_line":"            _cleanup()"},{"line_number":513,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F90k%3D","line":510,"updated":"2011-11-16 22:31:20.000000000","message":"As I said before, I\u0027m curious this is the responsibility of the _run_instance method, shouldn\u0027t we just assume everything was cleaned up if the domain was deleted somewhere else? Sure, we should do cleanup if the error was the fault of something in _run_instance, but that is what the _cleanup call below is for.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":507,"context_line":"            # NOTE (David Subiros): this exception will be raised if"},{"line_number":508,"context_line":"            # the instance was immediately deleted before it actually got"},{"line_number":509,"context_line":"            # created. Running shutdown again to make sure it is really deleted"},{"line_number":510,"context_line":"            self._shutdown_instance_even_if_deleted(context, instance_id)"},{"line_number":511,"context_line":"        except Exception:"},{"line_number":512,"context_line":"            _cleanup()"},{"line_number":513,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9ts%3D","line":510,"in_reply_to":"AAAACn%2F%2F90k%3D","updated":"2011-11-17 11:57:57.000000000","message":"The idea was that if the instance was terminated and deleted, and then run_instance continued to do things until realize that the instance is no longer alive (i.e. InstanceNotFound raised), it could potentially leave uncleaned things (everything that the function did from the moment it was terminated until run_instance realized that it was terminated). But you are right that maybe there is a more elegant solution for this: I thought about the terminating thread interrupting the creating thread, but I think this solution is safer.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":1884,"context_line":"            error_list.append(ex)"},{"line_number":1885,"context_line":""},{"line_number":1886,"context_line":"        try:"},{"line_number":1887,"context_line":"            self._check_timeout(context, db_instances)"},{"line_number":1888,"context_line":"        except Exception as ex:"},{"line_number":1889,"context_line":"            LOG.warning(_(\"Error during timeout check: %s\"), unicode(ex))"},{"line_number":1890,"context_line":"            error_list.append(ex)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9z4%3D","line":1887,"updated":"2011-11-16 22:31:20.000000000","message":"We used to have a process in place that did something similar to this and it was removed in favor of simply sync\u0027ing the power states. Here is my argument:\n\nThe hypervisor is authoritative for the the power_state of a particular VM.\n\nThe nova-compute manager is authoritative for the vm_states and task_states of a particular VM.\n\nThe manager code should be responsible for updating the states as it performs certain actions such as BUILD/REBUILD/etc and then it should also be responsible for following through and updating the state to ACTIVE or ERROR when things succeed or fail.\n\nIn the past loops like this very much hinder the ability of troubleshooting because it acts like a \"catch all\" and doesn\u0027t really provide us with any more information other than XXX instance has been in XXX state for XXX time.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":1884,"context_line":"            error_list.append(ex)"},{"line_number":1885,"context_line":""},{"line_number":1886,"context_line":"        try:"},{"line_number":1887,"context_line":"            self._check_timeout(context, db_instances)"},{"line_number":1888,"context_line":"        except Exception as ex:"},{"line_number":1889,"context_line":"            LOG.warning(_(\"Error during timeout check: %s\"), unicode(ex))"},{"line_number":1890,"context_line":"            error_list.append(ex)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9II%3D","line":1887,"in_reply_to":"AAAACn%2F%2F9to%3D","updated":"2011-11-21 18:24:15.000000000","message":"for now I\u0027m removing the timeout mechanism from this branch, it could be interesting to create a new branch to discuss it: but it\u0027s not the main focus of this one.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":1884,"context_line":"            error_list.append(ex)"},{"line_number":1885,"context_line":""},{"line_number":1886,"context_line":"        try:"},{"line_number":1887,"context_line":"            self._check_timeout(context, db_instances)"},{"line_number":1888,"context_line":"        except Exception as ex:"},{"line_number":1889,"context_line":"            LOG.warning(_(\"Error during timeout check: %s\"), unicode(ex))"},{"line_number":1890,"context_line":"            error_list.append(ex)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9to%3D","line":1887,"in_reply_to":"AAAACn%2F%2F9z4%3D","updated":"2011-11-17 11:57:57.000000000","message":"good argument :)\nThe reason why we implemented this was that sometimes the image caching process took a lot of time or even blocked, causing instances to be in BUILDING state forever.\nMaybe we could keep the timeout check just for non-hypervisor related states: for example, we could create a new state called caching, and have a timeout just for that particular task_state. Do you thing it could be a good solution?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":67,"name":"Vish Ishaya","email":"vishvananda@gmail.com","username":"vishvananda"},"change_message_id":"638bd05f97c2d7a41d24155fddd845e50e0f6bae","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        except:"},{"line_number":309,"context_line":"            return True"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def _shutdown_instance_even_if_deleted(self, context, instance_id):"},{"line_number":312,"context_line":"        \"\"\"Call terminate_instance even for already deleted instances\"\"\""},{"line_number":313,"context_line":"        LOG.info(\"Going to force the deletion of the vm %(instance_id)s,\""},{"line_number":314,"context_line":"                 \" even if it is deleted\" % locals())"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAC3%2F%2F9rk%3D","line":311,"updated":"2011-12-12 19:28:45.000000000","message":"Looks like you missed a uuid here?","commit_id":"676d1d50d7b196f40c7d4d214dd12f9772fed163"}],"nova/compute/state_checker.py":[{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"924ebeca41f0ecde32cf9dbeb0f36786795c11e1","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from nova.compute import vm_states as vm"},{"line_number":23,"context_line":"from nova import context as ctxt"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# Function names that run the state check before their execution:"},{"line_number":26,"context_line":"REBOOT \u003d \u0027reboot\u0027"},{"line_number":27,"context_line":"START \u003d \u0027start\u0027"},{"line_number":28,"context_line":"REBUILD \u003d \u0027rebuild\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAC3%2F%2F9iE%3D","line":25,"updated":"2011-12-12 22:03:09.000000000","message":"Is it necessary to create variables for each state here? It adds a layer of indirection that doesn\u0027t seem useful and just adds more work when adding new states. Or am I missing something?","commit_id":"676d1d50d7b196f40c7d4d214dd12f9772fed163"}],"nova/compute/task_states.py":[{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            RESIZE_CONFIRMING, RESIZE_VERIFY, REBUILDING, REBOOTING,"},{"line_number":71,"context_line":"            REBOOTING_HARD, PAUSING, UNPAUSING, SUSPENDING, RESUMING,"},{"line_number":72,"context_line":"            POWERING_OFF, POWERING_ON, RESCUING, UNRESCUING, DELETING,"},{"line_number":73,"context_line":"            STOPPING, STARTING]"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9zw%3D","line":73,"updated":"2011-11-16 22:31:20.000000000","message":"Would you buy something like:\n\n    def get_list():\n        for item in locals():\n            if not item.startswith(\"_\"):\n                yield item\n\n    or\n\n    return filter(lambda name: not name.startswith(\"_\"), locals())\n\n\n?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            RESIZE_CONFIRMING, RESIZE_VERIFY, REBUILDING, REBOOTING,"},{"line_number":71,"context_line":"            REBOOTING_HARD, PAUSING, UNPAUSING, SUSPENDING, RESUMING,"},{"line_number":72,"context_line":"            POWERING_OFF, POWERING_ON, RESCUING, UNRESCUING, DELETING,"},{"line_number":73,"context_line":"            STOPPING, STARTING]"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9tk%3D","line":73,"in_reply_to":"AAAACn%2F%2F9zw%3D","updated":"2011-11-17 11:57:57.000000000","message":"OK,\nIt would be a good way of adding or removing states easily (just need to modify the definition, and not the list).","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            RESIZE_CONFIRMING, RESIZE_VERIFY, REBUILDING, REBOOTING,"},{"line_number":71,"context_line":"            REBOOTING_HARD, PAUSING, UNPAUSING, SUSPENDING, RESUMING,"},{"line_number":72,"context_line":"            POWERING_OFF, POWERING_ON, RESCUING, UNRESCUING, DELETING,"},{"line_number":73,"context_line":"            STOPPING, STARTING]"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9JY%3D","line":73,"in_reply_to":"AAAACn%2F%2F9zw%3D","updated":"2011-11-21 18:24:15.000000000","message":"locals() doesn\u0027t return the right list, and globals() return also the \"get_list\" element. I think it\u0027s safer to keep it like this for now.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":67,"name":"Vish Ishaya","email":"vishvananda@gmail.com","username":"vishvananda"},"change_message_id":"638bd05f97c2d7a41d24155fddd845e50e0f6bae","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def get_list():"},{"line_number":66,"context_line":"    \"\"\"Returns a list of all the possible task_states\"\"\""},{"line_number":67,"context_line":"    return [SCHEDULING, BLOCK_DEVICE_MAPPING, NETWORKING, SPAWNING,"},{"line_number":68,"context_line":"            IMAGE_SNAPSHOT, IMAGE_BACKUP, UPDATING_PASSWORD, RESIZE_PREP,"},{"line_number":69,"context_line":"            RESIZE_MIGRATING, RESIZE_MIGRATED, RESIZE_FINISH, RESIZE_REVERTING,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAC3%2F%2F9qo%3D","line":66,"updated":"2011-12-12 19:28:45.000000000","message":"We could also do something fancy like:\n    import sys\n    current_module \u003d sys.modules[__name__]\n    global_values \u003d [getattr(current_module, x) for x in globals()\n                     if not x.startswith(\u0027_\u0027)]\n    return [x for x in global_values if isinstance(x, basestring)]\n\nbut perhaps that is too much magic.","commit_id":"676d1d50d7b196f40c7d4d214dd12f9772fed163"}],"nova/scheduler/manager.py":[{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        try:"},{"line_number":103,"context_line":"            return real_meth(*args, **kwargs)"},{"line_number":104,"context_line":"        except Exception as ex:"},{"line_number":105,"context_line":"            instance_id \u003d kwargs.get(\u0027instance_id\u0027, None)"},{"line_number":106,"context_line":"            if instance_id:"},{"line_number":107,"context_line":"                LOG.info(\"we were trying to schedule an instance, setting its\""},{"line_number":108,"context_line":"                         \" state to ERROR.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9zo%3D","line":105,"updated":"2011-11-16 22:31:20.000000000","message":"While I very much like the prospect of setting a VM to ERROR state if an error occurs scheduling it, this seems pretty fragile depending on \u0027instance_id\u0027 to be in kwargs. What if it\u0027s a volume that\u0027s being scheduled and happens to have kwargs with instance_id in it?","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        try:"},{"line_number":103,"context_line":"            return real_meth(*args, **kwargs)"},{"line_number":104,"context_line":"        except Exception as ex:"},{"line_number":105,"context_line":"            instance_id \u003d kwargs.get(\u0027instance_id\u0027, None)"},{"line_number":106,"context_line":"            if instance_id:"},{"line_number":107,"context_line":"                LOG.info(\"we were trying to schedule an instance, setting its\""},{"line_number":108,"context_line":"                         \" state to ERROR.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9JQ%3D","line":105,"in_reply_to":"AAAACn%2F%2F9zo%3D","updated":"2011-11-21 18:24:15.000000000","message":"Done","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        try:"},{"line_number":103,"context_line":"            return real_meth(*args, **kwargs)"},{"line_number":104,"context_line":"        except Exception as ex:"},{"line_number":105,"context_line":"            instance_id \u003d kwargs.get(\u0027instance_id\u0027, None)"},{"line_number":106,"context_line":"            if instance_id:"},{"line_number":107,"context_line":"                LOG.info(\"we were trying to schedule an instance, setting its\""},{"line_number":108,"context_line":"                         \" state to ERROR.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9tg%3D","line":105,"in_reply_to":"AAAACn%2F%2F9zo%3D","updated":"2011-11-17 11:57:57.000000000","message":"ok, thanks.\nmaybe it would be a better solution to check the method for which the scheduler is called. For example, check if the real_meth is run_instance or start_instance, and only try to get the instance_id and set the error state in that case.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"a22e77319b379b1cc39d5e02375b2424b7062ac7","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                         \" state to ERROR.\")"},{"line_number":109,"context_line":"                self.db.instance_update(context, instance_id,"},{"line_number":110,"context_line":"                            {\u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":111,"context_line":"            if type(ex) !\u003d exception.NoValidHost:"},{"line_number":112,"context_line":"                raise ex"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # NOTE (masumotok) : This method should be moved to nova.api.ec2.admin."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9zY%3D","line":111,"updated":"2011-11-16 22:31:20.000000000","message":"You\u0027re going to loose the original Would you buy making this try/except block look like:\n\n    try:\n        return real_meth(*args, **kwargs)\n    except exception.NoValidHost:\n        raise\n    except Exception as error:\n        # your logic here\n\n\nAlso, please make sure that you\u0027re logging the exception in *all* cases, we don\u0027t want to lose any exception information.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"cab23da5f4d87698986ba270de22850a035e87e2","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                         \" state to ERROR.\")"},{"line_number":109,"context_line":"                self.db.instance_update(context, instance_id,"},{"line_number":110,"context_line":"                            {\u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":111,"context_line":"            if type(ex) !\u003d exception.NoValidHost:"},{"line_number":112,"context_line":"                raise ex"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # NOTE (masumotok) : This method should be moved to nova.api.ec2.admin."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9JU%3D","line":111,"in_reply_to":"AAAACn%2F%2F9tc%3D","updated":"2011-11-21 18:24:15.000000000","message":"Done, I created an aux function, which can also be used in other places if necessary in the future.\nAlso, there was a bug in the code: now the DB entry for run_instance is created, so we cannot set ERROR state for that.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ff92442dfbfef0e93926d56e6a1a07e784975d14","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                         \" state to ERROR.\")"},{"line_number":109,"context_line":"                self.db.instance_update(context, instance_id,"},{"line_number":110,"context_line":"                            {\u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":111,"context_line":"            if type(ex) !\u003d exception.NoValidHost:"},{"line_number":112,"context_line":"                raise ex"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # NOTE (masumotok) : This method should be moved to nova.api.ec2.admin."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAACn%2F%2F9tc%3D","line":111,"in_reply_to":"AAAACn%2F%2F9zY%3D","updated":"2011-11-17 11:57:57.000000000","message":"ok, the idea was to set ERROR state whatever the exception, and reraise the unknown exceptions (and not NoValidHost, because we know it means that the scheduler couldn\u0027t find a valid host, which is a valid scenario). I will use your logic of capturing different exceptions in separate excepts.","commit_id":"06069471c863513b36c3f81a1814d2f00d4d6a2a"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"17467f6d04757f4e068b794eb1c4f1d1aa57547e","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from nova import manager"},{"line_number":31,"context_line":"from nova import rpc"},{"line_number":32,"context_line":"from nova import utils"},{"line_number":33,"context_line":"from nova.scheduler import zone_manager"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"LOG \u003d logging.getLogger(\u0027nova.scheduler.manager\u0027)"},{"line_number":36,"context_line":"FLAGS \u003d flags.FLAGS"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2F1M%3D","side":"PARENT","line":33,"updated":"2011-12-01 05:53:57.000000000","message":"This violates HACKING even if you didn\u0027t add it.  Can you move it up above nova.utils when you remove the duplicate import?","commit_id":"2a45e769b0185a43daa933e9eddd4de30b07ec0a"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ab060f14a8ac0219b8e8c9f41bc4f6b6e8fe38ee","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from nova import manager"},{"line_number":31,"context_line":"from nova import rpc"},{"line_number":32,"context_line":"from nova import utils"},{"line_number":33,"context_line":"from nova.scheduler import zone_manager"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"LOG \u003d logging.getLogger(\u0027nova.scheduler.manager\u0027)"},{"line_number":36,"context_line":"FLAGS \u003d flags.FLAGS"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F9wM%3D","side":"PARENT","line":33,"in_reply_to":"AAAAC3%2F%2F%2F1M%3D","updated":"2011-12-12 14:56:30.000000000","message":"Done","commit_id":"2a45e769b0185a43daa933e9eddd4de30b07ec0a"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"17467f6d04757f4e068b794eb1c4f1d1aa57547e","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from nova import manager"},{"line_number":32,"context_line":"from nova import rpc"},{"line_number":33,"context_line":"from nova import utils"},{"line_number":34,"context_line":"from nova.compute import vm_states"},{"line_number":35,"context_line":"from nova.scheduler import zone_manager"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"LOG \u003d logging.getLogger(\u0027nova.scheduler.manager\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2F1Q%3D","line":34,"updated":"2011-12-01 05:53:57.000000000","message":"This is already imported above in the correct spot according to HACKING.","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ab060f14a8ac0219b8e8c9f41bc4f6b6e8fe38ee","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from nova import manager"},{"line_number":32,"context_line":"from nova import rpc"},{"line_number":33,"context_line":"from nova import utils"},{"line_number":34,"context_line":"from nova.compute import vm_states"},{"line_number":35,"context_line":"from nova.scheduler import zone_manager"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"LOG \u003d logging.getLogger(\u0027nova.scheduler.manager\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F9wQ%3D","line":34,"in_reply_to":"AAAAC3%2F%2F%2F1Q%3D","updated":"2011-12-12 14:56:30.000000000","message":"Done","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"17467f6d04757f4e068b794eb1c4f1d1aa57547e","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    #                        method, the DB record probably does not exist yet."},{"line_number":113,"context_line":"    def _set_instance_error(self, method, context, ex, *args, **kwargs):"},{"line_number":114,"context_line":"        \"\"\"Sets VM to Error state\"\"\""},{"line_number":115,"context_line":"        if method \u003d\u003d \"start_instance\":"},{"line_number":116,"context_line":"            instance_id \u003d kwargs[\u0027instance_id\u0027]"},{"line_number":117,"context_line":"            LOG.warning(_(\"Failed to schedule_%(method)s: %(ex)s.  \""},{"line_number":118,"context_line":"                              \"Putting instance %(instance_id)s into \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2F1I%3D","line":115,"updated":"2011-12-01 05:53:57.000000000","message":"Saw your last comments.  There\u0027s issues with this branch too.  We need to set the instance to ERROR state for \u0027run_instance\u0027, too, but only if \u0027instance_id\u0027 is in kwargs.  If method is \u0027run_instance\u0027 but there\u0027s no \u0027instance_id\u0027 in the kwargs, then that means the instance DB record hasn\u0027t been created yet.","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"},{"author":{"_account_id":1861,"name":"david-perez5","username":"david-perez5"},"change_message_id":"ab060f14a8ac0219b8e8c9f41bc4f6b6e8fe38ee","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    #                        method, the DB record probably does not exist yet."},{"line_number":113,"context_line":"    def _set_instance_error(self, method, context, ex, *args, **kwargs):"},{"line_number":114,"context_line":"        \"\"\"Sets VM to Error state\"\"\""},{"line_number":115,"context_line":"        if method \u003d\u003d \"start_instance\":"},{"line_number":116,"context_line":"            instance_id \u003d kwargs[\u0027instance_id\u0027]"},{"line_number":117,"context_line":"            LOG.warning(_(\"Failed to schedule_%(method)s: %(ex)s.  \""},{"line_number":118,"context_line":"                              \"Putting instance %(instance_id)s into \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F9wA%3D","line":115,"in_reply_to":"AAAAC3%2F%2F%2F1I%3D","updated":"2011-12-12 14:56:30.000000000","message":"Done","commit_id":"6617e92ff26ffb58d4db2cc31867cfbeebad2182"}]}