)]}'
{"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"4769197dade72e425003e5d5f91ad42065b45d86","unresolved":false,"context_lines":[{"line_number":1168,"context_line":"            self.compute_api.set_admin_password(context, server, password)"},{"line_number":1169,"context_line":"        except exception.InstancePasswordSetFailed:"},{"line_number":1170,"context_line":"            msg \u003d _(\"Unable to set password on instance\")"},{"line_number":1171,"context_line":"            raise exc.HTTPNotImplemented(explanation\u003dmsg)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        return webob.Response(status_int\u003d202)"},{"line_number":1174,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAM3%2F%2FX04%3D","line":1171,"updated":"2013-01-25 00:40:18.000000000","message":"Hm ... so this is raising HTTPNotImplemented for any type of failure.  It seems like this only makes sense in the cases where we got NotImplemented from the driver layer.  Otherwise, the old 500 seems to make more sense.  Thoughts?","commit_id":"f7dde9812f6d460012ec30fea4a4a73920897570"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"69f97c0839c28ac8f87b145412e75c7d4f5b56c9","unresolved":false,"context_lines":[{"line_number":1168,"context_line":"            self.compute_api.set_admin_password(context, server, password)"},{"line_number":1169,"context_line":"        except exception.InstancePasswordSetFailed:"},{"line_number":1170,"context_line":"            msg \u003d _(\"Unable to set password on instance\")"},{"line_number":1171,"context_line":"            raise exc.HTTPNotImplemented(explanation\u003dmsg)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        return webob.Response(status_int\u003d202)"},{"line_number":1174,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAM3%2F%2FXxY%3D","line":1171,"in_reply_to":"AAAAM3%2F%2FX04%3D","updated":"2013-01-25 01:33:58.000000000","message":"I agree. We should return 500 if it\u0027s implemented in the driver but it failed for some reason (like timeout or whatever).\n\nHTTPNotImplemented (501) should be fine when the driver doesn\u0027t implement this.","commit_id":"f7dde9812f6d460012ec30fea4a4a73920897570"}],"nova/compute/manager.py":[{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"e7872968384fdf9993323b1019faa27791ebb174","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"                                      task_state\u003dNone,"},{"line_number":1626,"context_line":"                                      expected_task_state\u003dtask_states."},{"line_number":1627,"context_line":"                                      UPDATING_PASSWORD)"},{"line_number":1628,"context_line":"                raise NotImplementedError(_msg)"},{"line_number":1629,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":1630,"context_line":"                # interrupted by another (most likely delete) task"},{"line_number":1631,"context_line":"                # do not retry"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAM3%2F%2FW2Y%3D","line":1628,"updated":"2013-01-25 22:30:33.000000000","message":"raised NotImplementedError here instead of exception.InstancePasswordSetFailed as in previous patch such that we can throw the correct HTTP exception in the API layer.","commit_id":"5b93f27e5de59f0e48333fc27c4027046eb589fc"},{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"a59cdc41712fb7202ed323078d62ce7da71d0c4a","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"                                      task_state\u003dNone,"},{"line_number":1626,"context_line":"                                      expected_task_state\u003dtask_states."},{"line_number":1627,"context_line":"                                      UPDATING_PASSWORD)"},{"line_number":1628,"context_line":"                raise NotImplementedError(_msg)"},{"line_number":1629,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":1630,"context_line":"                # interrupted by another (most likely delete) task"},{"line_number":1631,"context_line":"                # do not retry"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FUbk%3D","line":1628,"updated":"2013-01-29 04:02:05.000000000","message":"Point of reference for other comment.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"}],"nova/tests/compute/test_compute.py":[{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"11b81a2b1e29a6a8edbe05705a06d60ed4fab09f","unresolved":false,"context_lines":[{"line_number":1200,"context_line":"                                                vm_states.ERROR,"},{"line_number":1201,"context_line":"                                                None,"},{"line_number":1202,"context_line":"                                                expected_exception)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    def test_set_admin_password_driver_not_implemented(self):"},{"line_number":1205,"context_line":"        \"\"\""},{"line_number":1206,"context_line":"        Ensure expected exception is raised if set_admin_password not"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FR78%3D","line":1203,"updated":"2013-01-30 13:06:33.000000000","message":"I know, it was here before but this test is wrong or at least doesn\u0027t not meet with the test name intentions (which is test the policy)\nraise notAuthorized from the driver doesn\u0027t make any sense (that is what _do_test_set_admin_password_driver_error does) unless you\u0027re trying to exemplify the system behavior when a unexpected exception is raise, if so this should be moved to another test with a better name, although we should keep the \u0027test not authorized\u0027 but using it right this time (assure that 403 is returned when you are not authorized)","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"7e5e2f52389e9b889c4055a254202046334120e5","unresolved":false,"context_lines":[{"line_number":1200,"context_line":"                                                vm_states.ERROR,"},{"line_number":1201,"context_line":"                                                None,"},{"line_number":1202,"context_line":"                                                expected_exception)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    def test_set_admin_password_driver_not_implemented(self):"},{"line_number":1205,"context_line":"        \"\"\""},{"line_number":1206,"context_line":"        Ensure expected exception is raised if set_admin_password not"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FRuc%3D","line":1203,"in_reply_to":"AAAAM3%2F%2FR78%3D","updated":"2013-01-30 15:58:51.000000000","message":"I don\u0027t understand. If your problem is with the test cases, you should file a new bug to address them since that seems outside the scope of this change.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"0da48ef8297f864e971c532eef82545d92746833","unresolved":false,"context_lines":[{"line_number":1200,"context_line":"                                                vm_states.ERROR,"},{"line_number":1201,"context_line":"                                                None,"},{"line_number":1202,"context_line":"                                                expected_exception)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    def test_set_admin_password_driver_not_implemented(self):"},{"line_number":1205,"context_line":"        \"\"\""},{"line_number":1206,"context_line":"        Ensure expected exception is raised if set_admin_password not"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FRps%3D","line":1203,"in_reply_to":"AAAAM3%2F%2FRuc%3D","updated":"2013-01-30 18:03:24.000000000","message":"ok, I\u0027ll","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"8355775f4ab75a2f6a8229f9337811de94e80e57","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        implemented by driver."},{"line_number":1208,"context_line":"        \"\"\""},{"line_number":1209,"context_line":"        exc \u003d NotImplementedError()"},{"line_number":1210,"context_line":"        expected_exception \u003d NotImplementedError"},{"line_number":1211,"context_line":"        self._do_test_set_admin_password_driver_error(exc,"},{"line_number":1212,"context_line":"                                                      vm_states.ACTIVE,"},{"line_number":1213,"context_line":"                                                      None,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FVbY%3D","line":1210,"updated":"2013-01-28 16:16:42.000000000","message":"This may not jive with the original author\u0027s expectations around the function. If so, it might be impossible to conduct a 501 error with this function without hacky code, i.e. parsing exception message of InstancePasswordSetFailed.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"23710df930a83c48dae53372867aebc63ea69729","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        implemented by driver."},{"line_number":1208,"context_line":"        \"\"\""},{"line_number":1209,"context_line":"        exc \u003d NotImplementedError()"},{"line_number":1210,"context_line":"        expected_exception \u003d NotImplementedError"},{"line_number":1211,"context_line":"        self._do_test_set_admin_password_driver_error(exc,"},{"line_number":1212,"context_line":"                                                      vm_states.ACTIVE,"},{"line_number":1213,"context_line":"                                                      None,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FUbQ%3D","line":1210,"in_reply_to":"AAAAM3%2F%2FUbY%3D","updated":"2013-01-29 04:07:54.000000000","message":"Ahh, I see. You\u0027re referring to the intentions of the test not the code being tested.\n\nI think this is fine, it\u0027s pretty common to test that an exception raised by an stub falls through to the test case.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"a59cdc41712fb7202ed323078d62ce7da71d0c4a","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        implemented by driver."},{"line_number":1208,"context_line":"        \"\"\""},{"line_number":1209,"context_line":"        exc \u003d NotImplementedError()"},{"line_number":1210,"context_line":"        expected_exception \u003d NotImplementedError"},{"line_number":1211,"context_line":"        self._do_test_set_admin_password_driver_error(exc,"},{"line_number":1212,"context_line":"                                                      vm_states.ACTIVE,"},{"line_number":1213,"context_line":"                                                      None,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FUbY%3D","line":1210,"in_reply_to":"AAAAM3%2F%2FUjQ%3D","updated":"2013-01-29 04:02:05.000000000","message":"I am raising and catching an exception of the same class. Previously, NotImplementedError was raised, but re-raised as an InstancePasswordSetFailed exception (to prevent information extrusion). See point of reference in manager.py diff.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"0e7524b208c97cc6733ab9006ede4ce6330b2d84","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        implemented by driver."},{"line_number":1208,"context_line":"        \"\"\""},{"line_number":1209,"context_line":"        exc \u003d NotImplementedError()"},{"line_number":1210,"context_line":"        expected_exception \u003d NotImplementedError"},{"line_number":1211,"context_line":"        self._do_test_set_admin_password_driver_error(exc,"},{"line_number":1212,"context_line":"                                                      vm_states.ACTIVE,"},{"line_number":1213,"context_line":"                                                      None,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FUjQ%3D","line":1210,"in_reply_to":"AAAAM3%2F%2FVbY%3D","updated":"2013-01-29 02:01:34.000000000","message":"I\u0027m not sure I understand you comment here. Can you elaborate on what you think the author\u0027s original intentions were?","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"}],"nova/virt/xenapi/agent.py":[{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"3b295e0c523d21b3fbdcf9cfbfe7adf7a253b34a","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        if resp[\u0027returncode\u0027] !\u003d \u0027D0\u0027:"},{"line_number":186,"context_line":"            msg \u003d _(\u0027Failed to exchange keys: %(resp)r\u0027) % locals()"},{"line_number":187,"context_line":"            LOG.error(msg, instance\u003dself.instance)"},{"line_number":188,"context_line":"            raise NotImplementedError(msg)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Some old versions of the Windows agent have a trailing \\\\r\\\\n"},{"line_number":191,"context_line":"        # (ie CRLF escaped) for some reason. Strip that off."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FR7c%3D","line":188,"updated":"2013-01-30 13:18:42.000000000","message":"and I don\u0027t think raise NotImplementedError makes sense here. The implementation is here, although I\u0027m not sure InstancePasswordSetFailled is the best choice..","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":6670,"name":"Amir Sadoughi","email":"amir.sadoughi@rackspace.com","username":"amir-sadoughi"},"change_message_id":"7e5e2f52389e9b889c4055a254202046334120e5","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        if resp[\u0027returncode\u0027] !\u003d \u0027D0\u0027:"},{"line_number":186,"context_line":"            msg \u003d _(\u0027Failed to exchange keys: %(resp)r\u0027) % locals()"},{"line_number":187,"context_line":"            LOG.error(msg, instance\u003dself.instance)"},{"line_number":188,"context_line":"            raise NotImplementedError(msg)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Some old versions of the Windows agent have a trailing \\\\r\\\\n"},{"line_number":191,"context_line":"        # (ie CRLF escaped) for some reason. Strip that off."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FRuU%3D","line":188,"in_reply_to":"AAAAM3%2F%2FR7c%3D","updated":"2013-01-30 15:58:51.000000000","message":"Yes, I agree that NotImplementedError is a strange choice. This was my attempt to implement the 501 error in the code invoking this function. Unless you want to parse messages from the actual Exception instances, I don\u0027t see another way to implement a 501 error given how the code is structured currently. Ultimately, If this is deemed unfit, we could go without the 501 error implementation.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":385,"name":"Jason Kölker","email":"jason@koelker.net","username":"jason-koelker"},"change_message_id":"0daa879a277ac3cc5ca231e2f7e0b990800f2872","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        if resp[\u0027returncode\u0027] !\u003d \u0027D0\u0027:"},{"line_number":186,"context_line":"            msg \u003d _(\u0027Failed to exchange keys: %(resp)r\u0027) % locals()"},{"line_number":187,"context_line":"            LOG.error(msg, instance\u003dself.instance)"},{"line_number":188,"context_line":"            raise NotImplementedError(msg)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Some old versions of the Windows agent have a trailing \\\\r\\\\n"},{"line_number":191,"context_line":"        # (ie CRLF escaped) for some reason. Strip that off."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FRiM%3D","line":188,"in_reply_to":"AAAAM3%2F%2FRjg%3D","updated":"2013-01-30 18:15:52.000000000","message":"If the key exchange fails, the most likely reason is the agent is not running. The agent is what implements the the action, so the action the user is requesting is not implemented in the vm. Part of the problem in nova in general is exceptions are used for message passing. I guess perhaps we could do a new agent specific exception for just this special case.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"},{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"0da48ef8297f864e971c532eef82545d92746833","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        if resp[\u0027returncode\u0027] !\u003d \u0027D0\u0027:"},{"line_number":186,"context_line":"            msg \u003d _(\u0027Failed to exchange keys: %(resp)r\u0027) % locals()"},{"line_number":187,"context_line":"            LOG.error(msg, instance\u003dself.instance)"},{"line_number":188,"context_line":"            raise NotImplementedError(msg)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Some old versions of the Windows agent have a trailing \\\\r\\\\n"},{"line_number":191,"context_line":"        # (ie CRLF escaped) for some reason. Strip that off."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FRjg%3D","line":188,"in_reply_to":"AAAAM3%2F%2FRuU%3D","updated":"2013-01-30 18:03:24.000000000","message":"Still don\u0027t get your point in return 501 error (NotImplemented) if this a server internal error, 500. You\u0027re changing the behavior of the code, this was previously catched on manager.py in:\nexcept Exception, e:\n         # Catch all here because this could be anything\n(line 1633 in your patch) and raised as InstancePasswordSetFailed which is a 500 error.\n\nI agree that 501 should be raise for the drivers which didn\u0027t have implementation for the operation, but this is not such case.","commit_id":"4dc160bf91d21b42363e5187adb96e59f95da717"}]}