)]}'
{"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"36da974a0529fd07c893f356fd7a253f7bb2a93e","unresolved":false,"context_lines":[{"line_number":878,"context_line":"                    \u0027reboot\u0027)"},{"line_number":879,"context_line":"        except exception.InstanceIsLocked, e:"},{"line_number":880,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":881,"context_line":"            raise exc.HTTPForbidden()"},{"line_number":882,"context_line":"        except Exception, e:"},{"line_number":883,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":884,"context_line":"            raise exc.HTTPUnprocessableEntity()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALH%2F%2F%2Bc8%3D","line":881,"updated":"2012-07-27 12:43:51.000000000","message":"Hmm, this is weird. Why are we handling InstanceIsLocked here but nowhere else? Without looking too deeply, I\u0027d expect us to be handling it everywhere it might be raised or that it\u0027s being caught by some general exception handler elsewhere.","commit_id":"86f1c2398998a2933ffe03e2415ae0d077e0db28"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":878,"context_line":"                    \u0027reboot\u0027)"},{"line_number":879,"context_line":"        except exception.InstanceIsLocked, e:"},{"line_number":880,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":881,"context_line":"            raise exc.HTTPForbidden()"},{"line_number":882,"context_line":"        except Exception, e:"},{"line_number":883,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":884,"context_line":"            raise exc.HTTPUnprocessableEntity()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8BI%3D","line":881,"updated":"2012-07-31 16:17:04.000000000","message":"Repeating previous comment ...\n\nHmm, this is weird. Why are we handling InstanceIsLocked here but nowhere else? Without looking too deeply, I\u0027d expect us to be handling it everywhere it might be raised or that it\u0027s being caught by some general exception handler elsewhere.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c31e58cfd56deea3c4ff6f7f1d9e799589d208d","unresolved":false,"context_lines":[{"line_number":878,"context_line":"                    \u0027reboot\u0027)"},{"line_number":879,"context_line":"        except exception.InstanceIsLocked, e:"},{"line_number":880,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":881,"context_line":"            raise exc.HTTPForbidden()"},{"line_number":882,"context_line":"        except Exception, e:"},{"line_number":883,"context_line":"            LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)"},{"line_number":884,"context_line":"            raise exc.HTTPUnprocessableEntity()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F7zI%3D","line":881,"in_reply_to":"AAAALH%2F%2F8BI%3D","updated":"2012-07-31 20:06:25.000000000","message":"Sorry, I missed this one.\n\nI see that I put this here to squelch the test, but didn\u0027t finish anywhere else. However, I don\u0027t see where it needs to go for things other than delete, reboot, etc, as those are not explicitly called out in wsgi.py. I\u0027ll pull this out for the next round, since it\u0027s really just cleaning up the message that the user sees. Maybe we can discuss the proper course and/or address the actual message in a follow-on patch.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"}],"nova/compute/api.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"36da974a0529fd07c893f356fd7a253f7bb2a93e","unresolved":false,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"    @check_instance_lock"},{"line_number":844,"context_line":"    @wrap_check_policy"},{"line_number":845,"context_line":"    @check_instance_state(vm_state\u003dNone, task_state\u003dNone)"},{"line_number":846,"context_line":"    def soft_delete(self, context, instance):"},{"line_number":847,"context_line":"        \"\"\"Terminate an instance.\"\"\""},{"line_number":848,"context_line":"        LOG.debug(_(\u0027Going to try to soft delete instance\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALH%2F%2F%2Bc4%3D","line":845,"updated":"2012-07-27 12:43:51.000000000","message":"Bit of a nitpick, but I think I\u0027d prefer @check_instance_lock to come wrap_check_policy\n\nDon\u0027t have a hugely compelling reason, except that it seems weird to look at the instance, then enforce a general policy and then look at the instance again. Also, it looks prettier if the two check_instance decorators are together :)\n\nMight be a good case for merging the two later into a more general @check_instance, but I\u0027m happy with two separate decorators for now","commit_id":"86f1c2398998a2933ffe03e2415ae0d077e0db28"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":97,"context_line":"def check_instance_lock(function):"},{"line_number":98,"context_line":"    @functools.wraps(function)"},{"line_number":99,"context_line":"    def inner(self, context, instance, *args, **kwargs):"},{"line_number":100,"context_line":"        if instance[\u0027locked\u0027]:"},{"line_number":101,"context_line":"            raise exception.InstanceIsLocked(instance_uuid\u003dinstance[\u0027uuid\u0027])"},{"line_number":102,"context_line":"        return function(self, context, instance, *args, **kwargs)"},{"line_number":103,"context_line":"    return inner"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8BE%3D","line":100,"updated":"2012-07-31 16:17:04.000000000","message":"The server side check actually looks up the instance from the DB again.\n\nI wonder is there good cause for us to do that here? Could we be looking at stale info, or is it goodness to avoid the extra DB call.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"1701bba8831568241016c81b6162e23491cbcb0e","unresolved":false,"context_lines":[{"line_number":97,"context_line":"def check_instance_lock(function):"},{"line_number":98,"context_line":"    @functools.wraps(function)"},{"line_number":99,"context_line":"    def inner(self, context, instance, *args, **kwargs):"},{"line_number":100,"context_line":"        if instance[\u0027locked\u0027]:"},{"line_number":101,"context_line":"            raise exception.InstanceIsLocked(instance_uuid\u003dinstance[\u0027uuid\u0027])"},{"line_number":102,"context_line":"        return function(self, context, instance, *args, **kwargs)"},{"line_number":103,"context_line":"    return inner"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F79Y%3D","line":100,"in_reply_to":"AAAALH%2F%2F8BE%3D","updated":"2012-07-31 16:38:30.000000000","message":"I think avoiding the extra DB call is a good thing here.  The fact that it\u0027s still using the DB on the server side is a legacy artifact and will be going away soon.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":864,"context_line":""},{"line_number":865,"context_line":"            self.compute_rpcapi.power_off_instance(context, instance,"},{"line_number":866,"context_line":"                                                   lock_checked\u003dTrue)"},{"line_number":867,"context_line":"        else:"},{"line_number":868,"context_line":"            LOG.warning(_(\u0027No host for instance, deleting immediately\u0027),"},{"line_number":869,"context_line":"                        instance\u003dinstance)"},{"line_number":870,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8Aw%3D","line":867,"updated":"2012-07-31 16:17:04.000000000","message":"This pattern looks pretty ok - we have a decorator on the method to check the lock and within the method we set a flag asserting that the lock is checked","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":908,"context_line":"                    # Call since this can race with the terminate_instance"},{"line_number":909,"context_line":"                    self.compute_rpcapi.confirm_resize(context,"},{"line_number":910,"context_line":"                            instance, migration_ref[\u0027id\u0027],"},{"line_number":911,"context_line":"                            host\u003dsrc_host, cast\u003dFalse, lock_checked\u003dTrue)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"            self.compute_rpcapi.terminate_instance(context, instance)"},{"line_number":914,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8As%3D","line":911,"updated":"2012-07-31 16:17:04.000000000","message":"Here we\u0027re asserting that the lock is checked, but it must have been set by all the callers. Pretty annoying to verify and could easily get broken in future.\n\nObvious solution is for lock_checked to be a parameter to _delete()","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":1446,"context_line":"            \"request_spec\": jsonutils.to_primitive(request_spec),"},{"line_number":1447,"context_line":"            \"filter_properties\": filter_properties,"},{"line_number":1448,"context_line":"        }"},{"line_number":1449,"context_line":"        self.scheduler_rpcapi.prep_resize(context, **args)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"    @wrap_check_policy"},{"line_number":1452,"context_line":"    @check_instance_lock"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8AU%3D","line":1449,"updated":"2012-07-31 16:17:04.000000000","message":"Interesting!\n\nThis ends up getting casted to the the compute service and the lock is checked there.\n\nI guess we want to the lock_checked parameter to prep_resize in the scheduler API too","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"1701bba8831568241016c81b6162e23491cbcb0e","unresolved":false,"context_lines":[{"line_number":1446,"context_line":"            \"request_spec\": jsonutils.to_primitive(request_spec),"},{"line_number":1447,"context_line":"            \"filter_properties\": filter_properties,"},{"line_number":1448,"context_line":"        }"},{"line_number":1449,"context_line":"        self.scheduler_rpcapi.prep_resize(context, **args)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"    @wrap_check_policy"},{"line_number":1452,"context_line":"    @check_instance_lock"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F79U%3D","line":1449,"in_reply_to":"AAAALH%2F%2F8AU%3D","updated":"2012-07-31 16:38:30.000000000","message":"As we were talking about on IRC, if the lock_checked (renamed to instance_lock_checked perhaps?) info gets placed in the request context, it should automatically make it through the scheduler and to the compute service.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"    def remove_fixed_ip(self, context, instance, address):"},{"line_number":1461,"context_line":"        \"\"\"Remove fixed_ip from specified network to given instance.\"\"\""},{"line_number":1462,"context_line":"        self.compute_rpcapi.remove_fixed_ip_from_instance(context,"},{"line_number":1463,"context_line":"                instance\u003dinstance, address\u003daddress)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"    @wrap_check_policy"},{"line_number":1466,"context_line":"    @check_instance_lock"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8AQ%3D","line":1463,"updated":"2012-07-31 16:17:04.000000000","message":"Missing lock_checked here","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"                    instance,"},{"line_number":1472,"context_line":"                    vm_state\u003dvm_states.ACTIVE,"},{"line_number":1473,"context_line":"                    task_state\u003dtask_states.PAUSING)"},{"line_number":1474,"context_line":"        self.compute_rpcapi.pause_instance(context, instance\u003dinstance)"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"    @wrap_check_policy"},{"line_number":1477,"context_line":"    @check_instance_lock"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8AI%3D","line":1474,"updated":"2012-07-31 16:17:04.000000000","message":"Ditto","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"                    instance,"},{"line_number":1483,"context_line":"                    vm_state\u003dvm_states.PAUSED,"},{"line_number":1484,"context_line":"                    task_state\u003dtask_states.UNPAUSING)"},{"line_number":1485,"context_line":"        self.compute_rpcapi.unpause_instance(context, instance\u003dinstance)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"    @wrap_check_policy"},{"line_number":1488,"context_line":"    def get_diagnostics(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F8AA%3D","line":1485,"updated":"2012-07-31 16:17:04.000000000","message":"Ditto","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":1552,"context_line":"    def inject_file(self, context, instance, path, file_contents):"},{"line_number":1553,"context_line":"        \"\"\"Write a file to the given instance.\"\"\""},{"line_number":1554,"context_line":"        self.compute_rpcapi.inject_file(context, instance\u003dinstance, path\u003dpath,"},{"line_number":1555,"context_line":"                file_contents\u003dfile_contents)"},{"line_number":1556,"context_line":""},{"line_number":1557,"context_line":"    @wrap_check_policy"},{"line_number":1558,"context_line":"    def get_vnc_console(self, context, instance, console_type):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F7%2F0%3D","line":1555,"updated":"2012-07-31 16:17:04.000000000","message":"All the above calls too?","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"859aaa850257dde8e30190043b6a55c6ac481731","unresolved":false,"context_lines":[{"line_number":97,"context_line":"def check_instance_lock(function):"},{"line_number":98,"context_line":"    @functools.wraps(function)"},{"line_number":99,"context_line":"    def inner(self, context, instance, *args, **kwargs):"},{"line_number":100,"context_line":"        if instance[\u0027locked\u0027]:"},{"line_number":101,"context_line":"            raise exception.InstanceIsLocked(instance_uuid\u003dinstance[\u0027uuid\u0027])"},{"line_number":102,"context_line":"        context.instance_lock_checked \u003d True"},{"line_number":103,"context_line":"        return function(self, context, instance, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F6MQ%3D","line":100,"updated":"2012-08-02 16:41:33.000000000","message":"Dan made a point that raises an issue\n\nOn the server side we check whether this is an admin request and allow it if it is, so we should do the same here\n\nAnd we should unit test that admin requests are allowed on locked instances","commit_id":"64c94a1e4a98bc938f1f919d02857644588bddc1"}],"nova/compute/manager.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"1701bba8831568241016c81b6162e23491cbcb0e","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    # and the function may get either an instance_uuid or an instance."},{"line_number":167,"context_line":"    def _checks_instance_lock_core(self, cb, context, *args, **kwargs):"},{"line_number":168,"context_line":"        instance_uuid \u003d kwargs[\u0027instance_uuid\u0027]"},{"line_number":169,"context_line":"        if \"lock_checked\" in kwargs.keys():"},{"line_number":170,"context_line":"            locked \u003d False"},{"line_number":171,"context_line":"            del kwargs[\"lock_checked\"]"},{"line_number":172,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F79Q%3D","line":169,"updated":"2012-07-31 16:38:30.000000000","message":"This can be:\n\n    if \u0027lock_checked\u0027 in kwargs:\n\n.keys() not necessary.","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"1feeafebed1f86abdd6fced5daee73ea37035983","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        instance_uuid \u003d kwargs[\u0027instance_uuid\u0027]"},{"line_number":169,"context_line":"        if \"lock_checked\" in kwargs.keys():"},{"line_number":170,"context_line":"            locked \u003d False"},{"line_number":171,"context_line":"            del kwargs[\"lock_checked\"]"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            locked \u003d self._get_lock(context, instance_uuid)"},{"line_number":174,"context_line":"        admin \u003d context.is_admin"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAALH%2F%2F7%2Fk%3D","line":171,"updated":"2012-07-31 16:17:04.000000000","message":"I wonder ...\n\nHow evil would it be for @check_instance_lock on the client side to set \"lock_checked\" on the instance itself and have it deleted on the server side here?","commit_id":"302b32190a6f308a8a74ba1ec06f0b8daa891300"}],"nova/exception.py":[{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"705a166e3aabb1a86f721eade74ada5a56705636","unresolved":false,"context_lines":[{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":""},{"line_number":1122,"context_line":"class InstanceIsLocked(Invalid):"},{"line_number":1123,"context_line":"    message \u003d _(\"Instance is locked\")"},{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":""},{"line_number":1126,"context_line":"def get_context_from_function_and_args(function, args, kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2F%2FEs%3D","line":1123,"updated":"2012-07-26 16:57:48.000000000","message":"Would it make sense for completeness to sub the instance ID into the exception msg?","commit_id":"d041a1c73b7a535bf40c02d29d0f90c208346f1b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e2826075ffa1d8deef320e53b14dbec1754ea2b7","unresolved":false,"context_lines":[{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":""},{"line_number":1122,"context_line":"class InstanceIsLocked(Invalid):"},{"line_number":1123,"context_line":"    message \u003d _(\"Instance is locked\")"},{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":""},{"line_number":1126,"context_line":"def get_context_from_function_and_args(function, args, kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2F%2FEM%3D","line":1123,"in_reply_to":"AAAALH%2F%2F%2FEs%3D","updated":"2012-07-26 17:02:17.000000000","message":"Sure.","commit_id":"d041a1c73b7a535bf40c02d29d0f90c208346f1b"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"c8d3c04dd604355bd14d08b596137769b18b53be","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"class InstanceIsLocked(Invalid):"},{"line_number":1125,"context_line":"    message \u003d _(\"Instance %(instance_uuid)s is locked\")"},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"def get_context_from_function_and_args(function, args, kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F7Yk%3D","line":1125,"updated":"2012-08-01 10:22:25.000000000","message":"Note the related comment on the new test case in test_server_actions.py.\n\nSo in order to ensure this exception is mapped by the FaultWrapper to a 409 Conflict instead of a generic 500 Server Error, add the following two lines:\n\n  +    code \u003d 409\n  +    safe \u003d True\n\nAnother approach would be for the new InstanceIsLocked exception type to be a sub-class of InstanceInvalidState instead of Invalid, and then deal with it via the common.raise_http_conflict_for_instance_invalid_state code-path.","commit_id":"57929943e2ed3de6ccfb48ca203484f9fe1ff81f"}],"nova/tests/api/openstack/compute/test_server_actions.py":[{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"c8d3c04dd604355bd14d08b596137769b18b53be","unresolved":false,"context_lines":[{"line_number":719,"context_line":"        req \u003d fakes.HTTPRequest.blank(self.url)"},{"line_number":720,"context_line":"        self.assertRaises(webob.exc.HTTPUnprocessableEntity,"},{"line_number":721,"context_line":"                          self.controller._action_reboot,"},{"line_number":722,"context_line":"                          req, FAKE_UUID, body)"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"class TestServerActionXMLDeserializer(test.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F7Yo%3D","line":722,"updated":"2012-08-01 10:22:25.000000000","message":"So this test case got me thinking, the HTTPUnprocessableEntity is not a very illuminating exception for the client to receive in the case that the instance is locked.\n\nSo it looks like this is due to special casing for this individual action:\n\n  https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L881\n\nHowever there doesn\u0027t seem to be much consistency here, as in some of the other server action methods (e.g. revert_resize) have a catch-all exception handler that maps to 400 Bad Request instead (again without any explanation) and other action seem to allow the exception bubble up to FaultWrapper middleware.\n\nInstead, we could allow the InstanceIsLocked exception to bubble up to the FaultWrapper middleware in all cases, and from there cause it to be mapped to a 409 Conflict.\n\nI guess this would require an additional catch-and-rethrow exception clause in each case where there is already catch-all exception handling, e.g.:\n\n +++ b/nova/api/openstack/compute/servers.py\n @@ -846,6 +846,8 @@ class Controller(wsgi.Controller):\n          except exception.InstanceInvalidState as state_error:\n              common.raise_http_conflict_for_instance_invalid_state(state_error,\n                      \u0027revertResize\u0027)\n +        except exception.InstanceIsLocked, e:\n +            raise e\n          except Exception, e:\n              LOG.exception(_(\"Error in revert-resize %s\"), e)\n              raise exc.HTTPBadRequest()\n @@ -876,6 +878,8 @@ class Controller(wsgi.Controller):\n          except exception.InstanceInvalidState as state_error:\n              common.raise_http_conflict_for_instance_invalid_state(state_error,\n                      \u0027reboot\u0027)\n +        except exception.InstanceIsLocked, e:\n +            raise e\n          except Exception, e:\n              LOG.exception(_(\"Error in reboot %s\"), e, instance\u003dinstance)\n              raise exc.HTTPUnprocessableEntity()\n\n... etc.\n\nAnother potential approach would be for the InstanceIsLocked exception to be a sub-class of InstanceInvalidState instead of Invalid, and then deal with it instead via the common.raise_http_conflict_for_instance_invalid_state code-path.","commit_id":"57929943e2ed3de6ccfb48ca203484f9fe1ff81f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ce9e417e90f0a67e0c86db5e2305d762802b39f3","unresolved":false,"context_lines":[{"line_number":719,"context_line":"        req \u003d fakes.HTTPRequest.blank(self.url)"},{"line_number":720,"context_line":"        self.assertRaises(webob.exc.HTTPUnprocessableEntity,"},{"line_number":721,"context_line":"                          self.controller._action_reboot,"},{"line_number":722,"context_line":"                          req, FAKE_UUID, body)"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"class TestServerActionXMLDeserializer(test.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F7H4%3D","line":722,"in_reply_to":"AAAALH%2F%2F7Yo%3D","updated":"2012-08-01 16:44:26.000000000","message":"Just making it a subclass of InstanceInvalidState seems to do what I expect as a user. I get a 400 error, and the message about the instance being locked. It does this without a bunch of special cases in compute/server.py, so I think it looks pretty good.\n\nWill post another set in a minute. See if that looks good to you.","commit_id":"57929943e2ed3de6ccfb48ca203484f9fe1ff81f"}]}