)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"f87bd36d3bd6f065363a8e89284b432cc18d3cae","unresolved":false,"context_lines":[{"line_number":13,"context_line":"message."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibe73c95b3c945feef4e1d2878826e6acc8d27b86"},{"line_number":16,"context_line":"Implements: blueprint manual-cleaning"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a7e1126_6cb4e2f1","line":16,"updated":"2015-12-17 12:35:09.000000000","message":"Hi Ruby, please use bug 1526290 in commit messages in this chain.","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"}],"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"0d555814d3d4dc34ffb661ad00bbdf5f6417ee25","unresolved":false,"context_lines":[{"line_number":447,"context_line":"            # response codes, we have this check here to deal with cases where"},{"line_number":448,"context_line":"            # a node is already being operated on (DEPLOYING or such) and we"},{"line_number":449,"context_line":"            # want to continue returning 409. Without it, we\u0027d return 400."},{"line_number":450,"context_line":"            if rpc_node.reservation:"},{"line_number":451,"context_line":"                raise exception.NodeLocked(node\u003drpc_node.uuid,"},{"line_number":452,"context_line":"                                           host\u003drpc_node.reservation)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_0f15e23e","line":450,"range":{"start_line":450,"start_character":35,"end_line":450,"end_character":36},"updated":"2015-12-18 01:36:53.000000000","message":"I have a concern here. Because node is reserved in cleaning state, it seems that the logic will always raise NodeLocked exception(CONFLICT) rather than InvalidStateRequeste(BadRequest) in test.\nI assume this is not we want to return to user.","commit_id":"43869719901e7a19a9d313d1e2d73587743df58e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"cf8a86839b26e8ab2e7c8900813fb70a3e02330e","unresolved":false,"context_lines":[{"line_number":447,"context_line":"            # response codes, we have this check here to deal with cases where"},{"line_number":448,"context_line":"            # a node is already being operated on (DEPLOYING or such) and we"},{"line_number":449,"context_line":"            # want to continue returning 409. Without it, we\u0027d return 400."},{"line_number":450,"context_line":"            if rpc_node.reservation:"},{"line_number":451,"context_line":"                raise exception.NodeLocked(node\u003drpc_node.uuid,"},{"line_number":452,"context_line":"                                           host\u003drpc_node.reservation)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_526e2d02","line":450,"range":{"start_line":450,"start_character":35,"end_line":450,"end_character":36},"in_reply_to":"3a7e1126_0f15e23e","updated":"2015-12-18 02:21:16.000000000","message":"Oh... sigh. I should have read the comment before. Now I\u0027m not sure what we should be returning to the user. Based on the comment, it looks like I should return 409 in this case. I need to think about what this backwards compatibility thing means; I think that was pre-state-machine.","commit_id":"43869719901e7a19a9d313d1e2d73587743df58e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9fa1ea0bed5dec40478bff66384fdb4da6831dda","unresolved":false,"context_lines":[{"line_number":447,"context_line":"            # response codes, we have this check here to deal with cases where"},{"line_number":448,"context_line":"            # a node is already being operated on (DEPLOYING or such) and we"},{"line_number":449,"context_line":"            # want to continue returning 409. Without it, we\u0027d return 400."},{"line_number":450,"context_line":"            if rpc_node.reservation:"},{"line_number":451,"context_line":"                raise exception.NodeLocked(node\u003drpc_node.uuid,"},{"line_number":452,"context_line":"                                           host\u003drpc_node.reservation)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a7b0d38_fd5bc52c","line":450,"range":{"start_line":450,"start_character":35,"end_line":450,"end_character":36},"in_reply_to":"3a7e1126_526e2d02","updated":"2015-12-22 20:33:01.000000000","message":"To preserve the same behaviour of returning 409, I don\u0027t have to make any changes at all. The only difference is that instead of being detected at the API level, in this case the request will go to the conductor before the 409 is returned. The user will have the same experience.\n\nThanks for pointing it out Tan.","commit_id":"43869719901e7a19a9d313d1e2d73587743df58e"}],"ironic/tests/unit/api/v1/test_nodes.py":[{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"a8e41a45997f8f445aa934c9d76d1697f6f820d7","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_65872810","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"updated":"2015-12-17 09:28:39.000000000","message":"target\u003ddone is not accepted at all I think, not only from cleaning to available","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"bc7d2a00c955696d8bc92815931c1b1d2ce1032d","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_c236fd02","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"in_reply_to":"3a7e1126_65872810","updated":"2015-12-17 13:14:29.000000000","message":"Yeah, \u0027done\u0027 to cleaning is part of the state machine, but it\u0027s not allowed in the API [0] at all.\n\n[0] https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/node.py#L462-L482","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"92ac1c21b2ef04116b0a879aabb50e5543fad1eb","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_8240f557","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"in_reply_to":"3a7e1126_65872810","updated":"2015-12-17 13:13:21.000000000","message":"Yes, that\u0027s true.\n\n(Sorry, is that all you wanted to mention or was there something you wanted me to do/change?)","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5278cc48dc5f5156f51d2d9961cdeeae82a6360a","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_c1d90d85","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"in_reply_to":"3a7e1126_b31c5a14","updated":"2015-12-17 15:20:40.000000000","message":"Lucas suggested I either remove this test or add a generic test to ensure \u0027done\u0027 can\u0027t be set, so I\u0027m going to remove it. Because the other would be outside the scope of this patch and it could lead to more comments ;)","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"7e37fe61561feb05d1cfa93470df10059fb36a35","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_1df99b4d","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"in_reply_to":"3a7e1126_c1d90d85","updated":"2015-12-18 01:33:26.000000000","message":"Agree with Lucas, add a generic test is what I mean but I am fine to do this in another patch.","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b2e1b3645987e8c12ec68681afba03e113491eca","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"        self.node.save()"},{"line_number":1977,"context_line":""},{"line_number":1978,"context_line":"        ret \u003d self.put_json(\u0027/nodes/%s/states/provision\u0027 % self.node.uuid,"},{"line_number":1979,"context_line":"                            {\u0027target\u0027: \u0027done\u0027},"},{"line_number":1980,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1981,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1982,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_b31c5a14","line":1979,"range":{"start_line":1979,"start_character":44,"end_line":1979,"end_character":47},"in_reply_to":"3a7e1126_c236fd02","updated":"2015-12-17 14:32:05.000000000","message":"Right. \u0027done\u0027 isn\u0027t allowed in the API, and that\u0027s what this is testing.\n\nWhat do you want me to do about this, if anything?","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"a8e41a45997f8f445aa934c9d76d1697f6f820d7","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_77a03bf5","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"updated":"2015-12-17 09:28:39.000000000","message":"nodes in cleaning state are always reserved right? so this should return http_client.CONFLICT","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"7e37fe61561feb05d1cfa93470df10059fb36a35","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_fd592f5b","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"in_reply_to":"3a7e1126_33a5cab4","updated":"2015-12-18 01:33:26.000000000","message":"@Yuriy, agree with Lucas, if it is async clean steps the provision_state is cleanwait which is not in the scope of this test.","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e0ef943bb976bbfb41c60356f0989f4ae29dd087","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_9c4b8659","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"in_reply_to":"3a7e1126_77a03bf5","updated":"2015-12-17 10:40:13.000000000","message":"This is true only for sync cleaning (IPA does async), reference: https://bugs.launchpad.net/ironic/+bug/1522008","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"92ac1c21b2ef04116b0a879aabb50e5543fad1eb","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_1d278613","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"in_reply_to":"3a7e1126_9c4b8659","updated":"2015-12-17 13:13:21.000000000","message":"@Tan. Yes, nodes in CLEANING are reserved. But NO, it shouldn\u0027t return CONFLICT. The whole point of this patch is for it to return BAD_REQUEST, not CONFLICT because someone that looks at the state machine diagram [1] may think that they should be able to perform this transition via the API.\n\n@Yuriy. It is only true for manual cleaning (I mean, this condition/test, because the node is in CLEANING and the target is MANAGEABLE which only happens for manual cleaning.)\n\n[1] http://docs.openstack.org/developer/ironic/_images/states.svg","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"bc7d2a00c955696d8bc92815931c1b1d2ce1032d","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_fd4b9aae","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"in_reply_to":"3a7e1126_9c4b8659","updated":"2015-12-17 13:14:29.000000000","message":"Async clean steps should be in CLEANWAIT state no? Because that means they are running on the node and not being executed by the conductor (such as erase_devices) [0]\n\n[0] https://github.com/openstack/ironic/blob/3732114d5c3dd658bd74d939cb59e793f499284e/ironic/drivers/base.py#L1056-L1064","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b2e1b3645987e8c12ec68681afba03e113491eca","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"                            {\u0027target\u0027: states.VERBS[\u0027manage\u0027]},"},{"line_number":1991,"context_line":"                            headers\u003d{api_base.Version.string: \"1.7\"},"},{"line_number":1992,"context_line":"                            expect_errors\u003dTrue)"},{"line_number":1993,"context_line":"        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"    @mock.patch.object(rpcapi.ConductorAPI, \u0027do_provisioning_action\u0027)"},{"line_number":1996,"context_line":"    def test_bad_requests_in_managed_state(self, mock_dpa):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_33a5cab4","line":1993,"range":{"start_line":1993,"start_character":37,"end_line":1993,"end_character":48},"in_reply_to":"3a7e1126_fd4b9aae","updated":"2015-12-17 14:32:05.000000000","message":"@Lucas. This test isn\u0027t about async clean steps.","commit_id":"51b0018283fca165b195d5f32730be12e8f57905"}]}
