)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"5b991cc7fd2ffe314ce0616195e3e48d0258c6ea","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-01-07 15:09:39 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix empty node-set-power-state output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Closes-Bug: #1266775"},{"line_number":10,"context_line":"Change-Id: Ibc2005d4cb9a43ac20ec0624f95dce002b0e02b2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAAUH%2F%2FzGU%3D","line":8,"updated":"2014-01-08 10:30:53.000000000","message":"I understand this is easy to understand the fix, however It is better that we follow the commit message guideline to have description here to show why/how is this patch.","commit_id":"103ad7dc325e7018128fa105198c9f8420fd6608"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"7e7b849e5bb351e9fdc5b83153d35f3c35e35ca2","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-01-07 15:09:39 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix empty node-set-power-state output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Closes-Bug: #1266775"},{"line_number":10,"context_line":"Change-Id: Ibc2005d4cb9a43ac20ec0624f95dce002b0e02b2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAAUH%2F%2Ftqw%3D","line":8,"in_reply_to":"AAAAUH%2F%2FzGU%3D","updated":"2014-01-09 10:05:08.000000000","message":"Done","commit_id":"103ad7dc325e7018128fa105198c9f8420fd6608"}],"ironicclient/tests/v1/test_node_shell.py":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"51f5a1eb38e26c5b925a1b2d3ff52f8565d510c2","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        fake_print_dict \u003d lambda data, *args, **kwargs: output.update(data)"},{"line_number":52,"context_line":"        with mock.patch.object(common_utils, \u0027print_dict\u0027, fake_print_dict):"},{"line_number":53,"context_line":"            state \u003d mock.MagicMock()"},{"line_number":54,"context_line":"            state.power_state \u003d \u0027off\u0027"},{"line_number":55,"context_line":"            mgr \u003d mock.MagicMock()"},{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtMM%3D","line":54,"updated":"2014-01-09 11:34:03.000000000","message":"Looks like we did not referred this state.power_state value, so how do you think if this line is required or more assertions to be appended to valid the \u0027current\u0027/\u0027requested\u0027 output values?","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"ae7feeb3124883b2d5660f59a628ab4dcd46cb67","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        fake_print_dict \u003d lambda data, *args, **kwargs: output.update(data)"},{"line_number":52,"context_line":"        with mock.patch.object(common_utils, \u0027print_dict\u0027, fake_print_dict):"},{"line_number":53,"context_line":"            state \u003d mock.MagicMock()"},{"line_number":54,"context_line":"            state.power_state \u003d \u0027off\u0027"},{"line_number":55,"context_line":"            mgr \u003d mock.MagicMock()"},{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FgSg%3D","line":54,"in_reply_to":"AAAAUH%2F%2FtIw%3D","updated":"2014-01-13 08:35:19.000000000","message":"OK, got it.","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"e78593390f99a524a352082b60e21d437da2c1b4","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        fake_print_dict \u003d lambda data, *args, **kwargs: output.update(data)"},{"line_number":52,"context_line":"        with mock.patch.object(common_utils, \u0027print_dict\u0027, fake_print_dict):"},{"line_number":53,"context_line":"            state \u003d mock.MagicMock()"},{"line_number":54,"context_line":"            state.power_state \u003d \u0027off\u0027"},{"line_number":55,"context_line":"            mgr \u003d mock.MagicMock()"},{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtIw%3D","line":54,"in_reply_to":"AAAAUH%2F%2FtMM%3D","updated":"2014-01-09 11:55:36.000000000","message":"It\u0027s passed through the mgr variable to n_shell.do_node_set_power_state. The assertion checks the output generated by this method, so not sure what other assertions you meant. Do you have any particular idea?","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"51f5a1eb38e26c5b925a1b2d3ff52f8565d510c2","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtOA%3D","line":59,"updated":"2014-01-09 11:34:03.000000000","message":"No need to hard-code such uuid, we can call utils.generate_uuid()  to get an auto-generated uuid\n\n    from ironic.common import utils\n    \n    utils.generate_uuid()","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"a2055c059aaaed0b1c50d764d999e95b928b2b4c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2B%2BJA%3D","line":59,"in_reply_to":"AAAAUH%2F%2B%2BL0%3D","updated":"2014-01-20 08:57:02.000000000","message":"Yes, agree with you, I am fine, thank you.","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"cafbb0133f7fef6b4ccfb2d5fc6ccd4555e85269","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2B%2BL0%3D","line":59,"in_reply_to":"AAAAUH%2F%2FPE4%3D","updated":"2014-01-20 08:46:07.000000000","message":"I agree, copying that code to the client would make sense. I also agree with your suggestion to do it in an other patch because we need to fix that in every tests.","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"80d136fe8148d083f56f60f50dfc94ea8f108b50","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FPE4%3D","line":59,"in_reply_to":"AAAAUH%2F%2FSRU%3D","updated":"2014-01-16 03:12:09.000000000","message":"I think we can do, that is useful util method for testing code, and we can hold here for more other comments also.","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"c74d7035c9d768537c7a219131d2a6b5ca2468fd","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FSRU%3D","line":59,"in_reply_to":"AAAAUH%2F%2FTkM%3D","updated":"2014-01-15 13:49:48.000000000","message":"Yeah, sure, I was just saying the it\u0027s probably not a good idea to create a code dependency in ironicclient to ironic. Or are you suggesting to copypaste that code from ironic to ironicclient?","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"9c39e5412d424d1845346aefe9dc11b760e2d0df","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FTkM%3D","line":59,"in_reply_to":"AAAAUH%2F%2FY5A%3D","updated":"2014-01-15 08:40:09.000000000","message":"Hi Imre, \n\nironic.common.utils is the existing file [1] in ironic client  package, but no such generate_uuid in our client utils.py file now, however we can find it in our ironic server side code [2].\n\n[1] https://github.com/openstack/python-ironicclient/blob/master/ironicclient/common/utils.py\n\n[2] https://github.com/openstack/ironic/blob/master/ironic/common/utils.py#L107\n\nThanks","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"66fdc8271e9f4ca2150ef2d151a7083208b7249b","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FY5A%3D","line":59,"in_reply_to":"AAAAUH%2F%2FgS0%3D","updated":"2014-01-14 10:06:49.000000000","message":"Not sure if that\u0027s a good idea, as it would introduce a dependency to ironic...","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"ae7feeb3124883b2d5660f59a628ab4dcd46cb67","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FgS0%3D","line":59,"in_reply_to":"AAAAUH%2F%2FtE4%3D","updated":"2014-01-13 08:35:19.000000000","message":"OK, that is fine for this patch, we can import ironic.common.utils to client with new patch.","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"e78593390f99a524a352082b60e21d437da2c1b4","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtLI%3D","line":59,"in_reply_to":"AAAAUH%2F%2FtOA%3D","updated":"2014-01-09 11:55:36.000000000","message":"Done","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"38c346121960bd35ee3c7bce9c2a1c8cfcfda29b","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            mgr.node.set_power_state.return_value \u003d state"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"            args \u003d mock.MagicMock()"},{"line_number":59,"context_line":"            args.node \u003d \u00271b544db4-f400-41cd-ad51-544cedd515e9\u0027"},{"line_number":60,"context_line":"            args.power_state \u003d \u0027on\u0027"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"            n_shell.do_node_set_power_state(mgr, args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtE4%3D","line":59,"in_reply_to":"AAAAUH%2F%2FtOA%3D","updated":"2014-01-09 12:03:14.000000000","message":"Sorry, I missed the check it earlier but there\u0027s no utils.generate_uuid() in python-ironicclient. It\u0027s done in the same way in other test files:\nhttps://github.com/openstack/python-ironicclient/blob/master/ironicclient/tests/v1/test_node.py\nhttps://github.com/openstack/python-ironicclient/blob/master/ironicclient/tests/v1/test_chassis.py","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"}],"ironicclient/v1/node_shell.py":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"5b991cc7fd2ffe314ce0616195e3e48d0258c6ea","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    data \u003d {\u0027current\u0027: state.power_state,"},{"line_number":160,"context_line":"            \u0027requested\u0027: requested_state}"},{"line_number":161,"context_line":"    utils.print_dict(data, wrap\u003d72)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@utils.arg(\u0027node\u0027,"},{"line_number":165,"context_line":"           metavar\u003d\u0027\u003cnode uuid\u003e\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FzDE%3D","line":162,"updated":"2014-01-08 10:30:53.000000000","message":"We change the shell code, so can you add case to cover this change in our test_node_shell.py [1] file?\n\nhttps://github.com/openstack/python-ironicclient/blob/master/ironicclient/tests/v1/test_node_shell.py","commit_id":"103ad7dc325e7018128fa105198c9f8420fd6608"},{"author":{"_account_id":7419,"name":"Imre Farkas","email":"farkas.imre@gmail.com","username":"ifarkas"},"change_message_id":"7e7b849e5bb351e9fdc5b83153d35f3c35e35ca2","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    data \u003d {\u0027current\u0027: state.power_state,"},{"line_number":160,"context_line":"            \u0027requested\u0027: requested_state}"},{"line_number":161,"context_line":"    utils.print_dict(data, wrap\u003d72)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@utils.arg(\u0027node\u0027,"},{"line_number":165,"context_line":"           metavar\u003d\u0027\u003cnode uuid\u003e\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FtpM%3D","line":162,"in_reply_to":"AAAAUH%2F%2FzDE%3D","updated":"2014-01-09 10:05:08.000000000","message":"Thanks Haomeng, that is an excellent point!","commit_id":"103ad7dc325e7018128fa105198c9f8420fd6608"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"d5732627d4b1ac420b478ef863644f2eb7696e28","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    state \u003d cc.node.set_power_state(args.node, requested_state)"},{"line_number":159,"context_line":"    data \u003d {\u0027current\u0027: state.power_state,"},{"line_number":160,"context_line":"            \u0027requested\u0027: requested_state}"},{"line_number":161,"context_line":"    utils.print_dict(data, wrap\u003d72)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2BQXE%3D","line":160,"updated":"2014-01-28 17:22:15.000000000","message":"I don\u0027t think it\u0027s correct. The user will definitely know what he requested, this output was intended to show what is the target power state of the node on server - e.g. to let user know whether the server has started transitioning to desired state or not.\n\nThe usual output of powering node ON will be:\n\n    current : power off\n    target : power on\n\nWhich means the node is going to be turned on and the user should continue polling ironic node-show or GET to /nodes/\u003cuuid\u003e/states if he want\u0027s to know when it will become turned on.\n\nAlternate output - when the node is already powered ON, and the user does power on again. The output for this will be:\n\n    current : power on\n    target : none (or empty string)\n\nwhich is good, because this lets user know that he don\u0027t need to continue polling, it\u0027s already in desired state. That\u0027s why I think we need to leave target_power_state in the output.\n\nHowever we have a bigger problem here. There is a patch https://review.openstack.org/#/c/67506/1 , once it is merged, we won\u0027t have the .set_power_state() output at all. This happens because we switching to the following strategy:\n\n    1. The user sends request to change power state.\n    2. Once API gets request to change power state it will:\n       a) validate if everything is OK with given parameters and with the specified node\n       b) start a background task to change power state if (a) has passed\n       c) return 202 to the client if (a) has passed, or return appropriate code if not.\n\nThe problem is that changing target_power_state will be a part background task, we can\u0027t rely on how soon it will be started (we probably may have a task queue in future, so it may start after a while). Even without background tasks mechanism we can\u0027t rely that target_power_state immediately becomes relevant, because it can take some time for conductor to start processing the request. Therefore there are no need to return body including target_power_state, it will be not relevant. All we can do is to return 202 to show that the request has been accepted, and suggest to the user to do 3 :)\n\n    3. The user should issue a subsequent node-show or GET to /states requests to track what is going with his node\n\nSo what I\u0027d like to suggest here is to not to print any info retrieved from response body, and to print something like: \"The request to change node \u003cuuid\u003e power state to ON was accepted. Please use node-show \u003cuuid\u003e to track the processing of the request.\"\n\nI don\u0027t think we will return 200 for case described in \"Alternate output\" section above, I think we\u0027ll always return 202, the user will see i\u0027ts already processed once he issue a node-show.\n\nAny other thoughts? :)","commit_id":"190f291edc1bee1f731819f0bea6bcee61511374"}]}