)]}'
{"ironic/conductor/manager.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"50961c663f6c3c63f029996fbea4c125d994d190","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        with task_manager.acquire(context, node_id, shared\u003dFalse) as task:"},{"line_number":239,"context_line":"            node \u003d task.node"},{"line_number":240,"context_line":"            if not(node[\u0027provision_state\u0027] \u003d\u003d states.NOSTATE"},{"line_number":241,"context_line":"                         or node[\u0027provision_state\u0027] \u003d\u003d states.DEPLOYFAIL):"},{"line_number":242,"context_line":"                raise exception.InstanceDeployFailure(_("},{"line_number":243,"context_line":"                    \"RPC do_node_deploy called for %(node)s, but provision \""},{"line_number":244,"context_line":"                    \"state is already %(state)s.\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F8hg%3D","line":241,"updated":"2014-01-06 12:09:46.000000000","message":"May I suggest another syntax for it:\n\nif node[\u0027provision_state\u0027] not in [states.NOSTATE, states.DEPLOYFAIL]:\n   ...\n\nI think it\u0027s more readable","commit_id":"9c884b1675d3430a2cd3dfb407562e325eb67773"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"815896cb9db012777e8d0068b6aca00d61d69ac7","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        with task_manager.acquire(context, node_id, shared\u003dFalse) as task:"},{"line_number":239,"context_line":"            node \u003d task.node"},{"line_number":240,"context_line":"            if not(node[\u0027provision_state\u0027] \u003d\u003d states.NOSTATE"},{"line_number":241,"context_line":"                         or node[\u0027provision_state\u0027] \u003d\u003d states.DEPLOYFAIL):"},{"line_number":242,"context_line":"                raise exception.InstanceDeployFailure(_("},{"line_number":243,"context_line":"                    \"RPC do_node_deploy called for %(node)s, but provision \""},{"line_number":244,"context_line":"                    \"state is already %(state)s.\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F8eU%3D","line":241,"in_reply_to":"AAAAUH%2F%2F8hg%3D","updated":"2014-01-06 12:30:03.000000000","message":"Thank you Lucas, will do:)","commit_id":"9c884b1675d3430a2cd3dfb407562e325eb67773"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3d354463c1342459718c2ebef40119483def12f4","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        with task_manager.acquire(context, node_id, shared\u003dFalse) as task:"},{"line_number":239,"context_line":"            node \u003d task.node"},{"line_number":240,"context_line":"            if (node[\u0027provision_state\u0027] not in"},{"line_number":241,"context_line":"                            [states.NOSTATE, states.DEPLOYFAIL]):"},{"line_number":242,"context_line":"                raise exception.InstanceDeployFailure(_("},{"line_number":243,"context_line":"                    \"RPC do_node_deploy called for %(node)s, but provision \""},{"line_number":244,"context_line":"                    \"state is already %(state)s.\") %"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUH%2F%2F8Y0%3D","line":241,"updated":"2014-01-06 13:04:17.000000000","message":"Nit:\n\nI\u0027m sorry that in my suggestion on the previous review I was accessing the \u0027provision_state\u0027 attribute of the node using a dict-like fashion. There\u0027s some patches upstream trying to get rid of it already[1] so it might worth to do for the new patches as well.\n\nSo would be great to change it to something like:\n\nif node.provision_state not in [states.NOSTATE, states.DEPLOYFAIL]:\n    ....\n\n\n[1] https://review.openstack.org/#/c/64108/\n\n\n*****\n\nAlso, on the previous review I think I wasn\u0027t very clear (I mentioned it on the review message but not in an inline comment)  that would be nice to add one test for this change as well. \n\nThanks much.","commit_id":"240ae0300b48ca40580c69c21da0a890f16e783d"}],"ironic/tests/conductor/test_manager.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"785c9bf4b88424e31415c84f5a842f38c5e21ba5","unresolved":false,"context_lines":[{"line_number":306,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def test_do_node_deploy_again_with_fail_state(self):"},{"line_number":309,"context_line":"        # test node[\u0027provision_state\u0027] is not DEPLOYFAIL"},{"line_number":310,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027,"},{"line_number":311,"context_line":"                                    provision_state\u003dstates.DEPLOYFAIL)"},{"line_number":312,"context_line":"        node \u003d self.dbapi.create_node(ndict)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F2Cc%3D","line":309,"updated":"2014-01-07 19:25:39.000000000","message":"tiny nit:\n\nDo you mean: # test node[\u0027provision_state\u0027] is DEPLOYFAIL (without \"not\") ?","commit_id":"47e16f49ced926926f18d3803ead6f3d69e7209b"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"edcb0a957c4d80901aa3e8516c205b4fa1753cf0","unresolved":false,"context_lines":[{"line_number":306,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def test_do_node_deploy_again_with_fail_state(self):"},{"line_number":309,"context_line":"        # test node[\u0027provision_state\u0027] is not DEPLOYFAIL"},{"line_number":310,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027,"},{"line_number":311,"context_line":"                                    provision_state\u003dstates.DEPLOYFAIL)"},{"line_number":312,"context_line":"        node \u003d self.dbapi.create_node(ndict)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0%2B4%3D","line":309,"in_reply_to":"AAAAUH%2F%2F2Cc%3D","updated":"2014-01-08 00:15:37.000000000","message":"yes, will remove \u0027not\u0027, thank you Lucas:)","commit_id":"47e16f49ced926926f18d3803ead6f3d69e7209b"}]}