)]}'
{"ironic/conductor/manager.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b614e8529f467ee338371619a960060e131673b3","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            raise exception.NodeInMaintenance(op\u003d_(\u0027provisioning\u0027),"},{"line_number":346,"context_line":"                                              node\u003dnode.uuid)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        task.driver.deploy.validate(task, node)"},{"line_number":349,"context_line":"        try:"},{"line_number":350,"context_line":"            # Start requested action in the background."},{"line_number":351,"context_line":"            thread \u003d self._spawn_worker(self._do_node_deploy, context, task)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2Fdi4%3D","line":348,"updated":"2014-03-05 23:35:42.000000000","message":"validate may raise exception. This should be in a try: block which would set node.last_error","commit_id":"79b695d144134b814e10bcb3f6e5f3001e2f3938"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0614c2d2f7b3a3a480533a7610772814f607aac9","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            raise exception.NodeInMaintenance(op\u003d_(\u0027provisioning\u0027),"},{"line_number":346,"context_line":"                                              node\u003dnode.uuid)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        task.driver.deploy.validate(task, node)"},{"line_number":349,"context_line":"        try:"},{"line_number":350,"context_line":"            # Start requested action in the background."},{"line_number":351,"context_line":"            thread \u003d self._spawn_worker(self._do_node_deploy, context, task)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2FdAc%3D","line":348,"in_reply_to":"AAAAVn%2F%2Fdi4%3D","updated":"2014-03-06 00:39:22.000000000","message":"oh +1, we have to wrap this pretty much this whole code in a try...except block otherwise if it fails it\u0027s not going to release the lock (as u pointed it out to me).\n\nAbout saving the last_error, I don\u0027t think it\u0027s needed because it\u0027s a sync call and the errors are going to be bubbled up to the api and returned to the client.","commit_id":"79b695d144134b814e10bcb3f6e5f3001e2f3938"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b614e8529f467ee338371619a960060e131673b3","unresolved":false,"context_lines":[{"line_number":360,"context_line":"            node.provision_state \u003d states.DEPLOYING"},{"line_number":361,"context_line":"            node.target_provision_state \u003d states.DEPLOYDONE"},{"line_number":362,"context_line":"            node.last_error \u003d None"},{"line_number":363,"context_line":"            node.save(context)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def _do_node_deploy(self, context, task):"},{"line_number":366,"context_line":"        \"\"\"Prepare the environment and deploy a node.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2Fdi8%3D","line":363,"updated":"2014-03-05 23:35:42.000000000","message":"this else: block must be done before self._spawn_worker, or else you are creating a race condition.","commit_id":"79b695d144134b814e10bcb3f6e5f3001e2f3938"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0614c2d2f7b3a3a480533a7610772814f607aac9","unresolved":false,"context_lines":[{"line_number":360,"context_line":"            node.provision_state \u003d states.DEPLOYING"},{"line_number":361,"context_line":"            node.target_provision_state \u003d states.DEPLOYDONE"},{"line_number":362,"context_line":"            node.last_error \u003d None"},{"line_number":363,"context_line":"            node.save(context)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def _do_node_deploy(self, context, task):"},{"line_number":366,"context_line":"        \"\"\"Prepare the environment and deploy a node.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVn%2F%2FdAU%3D","line":363,"in_reply_to":"AAAAVn%2F%2Fdi8%3D","updated":"2014-03-06 00:39:22.000000000","message":"+1","commit_id":"79b695d144134b814e10bcb3f6e5f3001e2f3938"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            node.provision_state \u003d states.DEPLOYING"},{"line_number":352,"context_line":"            node.target_provision_state \u003d states.DEPLOYDONE"},{"line_number":353,"context_line":"            node.last_error \u003d None"},{"line_number":354,"context_line":"            node.save(context)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"            # Start requested action in the background."},{"line_number":357,"context_line":"            thread \u003d self._spawn_worker(self._do_node_deploy, context, task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FaGE%3D","line":354,"updated":"2014-03-06 09:32:35.000000000","message":"I saw the comment in previous patch, but what if response lost, f.e. due to connection problem. I\u0027d leave at least some short note in last error in this case. What do you think?\n\nPls refer to similar section at https://review.openstack.org/#/c/74063/6/ironic/conductor/manager.py\n\nIf no, then I need to update my patch, because the behaviour won\u0027t be consistent","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            node.provision_state \u003d states.DEPLOYING"},{"line_number":352,"context_line":"            node.target_provision_state \u003d states.DEPLOYDONE"},{"line_number":353,"context_line":"            node.last_error \u003d None"},{"line_number":354,"context_line":"            node.save(context)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"            # Start requested action in the background."},{"line_number":357,"context_line":"            thread \u003d self._spawn_worker(self._do_node_deploy, context, task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWsI%3D","line":354,"in_reply_to":"AAAAVn%2F%2FaGE%3D","updated":"2014-03-06 17:19:27.000000000","message":"Hi max,\n\nI see your point but I don\u0027t know if I agree with that. Because the last_error was created to hold the error that happens in the background, so if we start using it to sync operations we are going to be inconsistent with the rest of the ironic code and the reason why that field was created.\n\nIn case of a connection problem, he might still be able to see the problem in the LOGs.\n\nSo I prefer to keep it, and I think it should be changed in ur patch as well (I\u0027m going to add a comment there)","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"}],"ironic/tests/conductor/test_manager.py":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":514,"context_line":"        self.assertRaises(exception.InstanceDeployFailure,"},{"line_number":515,"context_line":"                          self.service.do_node_deploy,"},{"line_number":516,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    def test_do_node_deploy_maintenance(self):"},{"line_number":519,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, maintenance\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FZ%2BU%3D","line":516,"updated":"2014-03-06 09:32:35.000000000","message":"I\u0027d also test that the lock is released. Since we switched to manual lock management it\u0027s easy to miss release_resources now..","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":514,"context_line":"        self.assertRaises(exception.InstanceDeployFailure,"},{"line_number":515,"context_line":"                          self.service.do_node_deploy,"},{"line_number":516,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    def test_do_node_deploy_maintenance(self):"},{"line_number":519,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, maintenance\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWjY%3D","line":516,"in_reply_to":"AAAAVn%2F%2FZ%2BU%3D","updated":"2014-03-06 17:19:27.000000000","message":"+1","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":521,"context_line":"        self.assertRaises(exception.NodeInMaintenance,"},{"line_number":522,"context_line":"                          self.service.do_node_deploy,"},{"line_number":523,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.validate\u0027)"},{"line_number":526,"context_line":"    def test_do_node_deploy_validate_fail(self, mock_validate):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FZ%2Bg%3D","line":523,"updated":"2014-03-06 09:32:35.000000000","message":"ditto resources","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":521,"context_line":"        self.assertRaises(exception.NodeInMaintenance,"},{"line_number":522,"context_line":"                          self.service.do_node_deploy,"},{"line_number":523,"context_line":"                          self.context, node[\u0027uuid\u0027])"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.validate\u0027)"},{"line_number":526,"context_line":"    def test_do_node_deploy_validate_fail(self, mock_validate):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWjQ%3D","line":523,"in_reply_to":"AAAAVn%2F%2FZ%2Bg%3D","updated":"2014-03-06 17:19:27.000000000","message":"+1","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":529,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":530,"context_line":"        self.assertRaises(exception.InvalidParameterValue,"},{"line_number":531,"context_line":"                          self.service.do_node_deploy,"},{"line_number":532,"context_line":"                          self.context, node.uuid)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.deploy\u0027)"},{"line_number":535,"context_line":"    def test__do_node_deploy_driver_raises_error(self, mock_deploy):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FaBU%3D","line":532,"updated":"2014-03-06 09:32:35.000000000","message":"ditto release resources","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":529,"context_line":"        node \u003d self.dbapi.create_node(ndict)"},{"line_number":530,"context_line":"        self.assertRaises(exception.InvalidParameterValue,"},{"line_number":531,"context_line":"                          self.service.do_node_deploy,"},{"line_number":532,"context_line":"                          self.context, node.uuid)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.deploy\u0027)"},{"line_number":535,"context_line":"    def test__do_node_deploy_driver_raises_error(self, mock_deploy):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWjE%3D","line":532,"in_reply_to":"AAAAVn%2F%2FaBU%3D","updated":"2014-03-06 17:19:27.000000000","message":"+1","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":582,"context_line":"            self.assertEqual(node.provision_state, states.DEPLOYING)"},{"line_number":583,"context_line":"            self.assertEqual(node.target_provision_state, states.DEPLOYDONE)"},{"line_number":584,"context_line":"            self.assertIsNone(node.last_error)"},{"line_number":585,"context_line":"            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"    def test_do_node_tear_down_invalid_state(self):"},{"line_number":588,"context_line":"        # test node[\u0027provision_state\u0027] is incorrect for tear_down"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FaAE%3D","line":585,"updated":"2014-03-06 09:32:35.000000000","message":"ditto release","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":582,"context_line":"            self.assertEqual(node.provision_state, states.DEPLOYING)"},{"line_number":583,"context_line":"            self.assertEqual(node.target_provision_state, states.DEPLOYDONE)"},{"line_number":584,"context_line":"            self.assertIsNone(node.last_error)"},{"line_number":585,"context_line":"            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"    def test_do_node_tear_down_invalid_state(self):"},{"line_number":588,"context_line":"        # test node[\u0027provision_state\u0027] is incorrect for tear_down"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWi8%3D","line":585,"in_reply_to":"AAAAVn%2F%2FaAE%3D","updated":"2014-03-06 17:19:27.000000000","message":"+1","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"95c48bbeac879b27ee7aae6c8ac1715a49bbae96","unresolved":false,"context_lines":[{"line_number":583,"context_line":"            self.assertEqual(node.target_provision_state, states.DEPLOYDONE)"},{"line_number":584,"context_line":"            self.assertIsNone(node.last_error)"},{"line_number":585,"context_line":"            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"    def test_do_node_tear_down_invalid_state(self):"},{"line_number":588,"context_line":"        # test node[\u0027provision_state\u0027] is incorrect for tear_down"},{"line_number":589,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FZ%2FA%3D","line":586,"updated":"2014-03-06 09:32:35.000000000","message":"With this patch we also introduced the new pathway - NoFreeConductorWorker. I made similar test for change_power_state and vendor_passthru (test_change_node_power_state_worker_pool_full) so I\u0027d also add similar one here","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"65ff4cecbe9db12abd09200b766c197c820244bd","unresolved":false,"context_lines":[{"line_number":583,"context_line":"            self.assertEqual(node.target_provision_state, states.DEPLOYDONE)"},{"line_number":584,"context_line":"            self.assertIsNone(node.last_error)"},{"line_number":585,"context_line":"            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"    def test_do_node_tear_down_invalid_state(self):"},{"line_number":588,"context_line":"        # test node[\u0027provision_state\u0027] is incorrect for tear_down"},{"line_number":589,"context_line":"        ndict \u003d utils.get_test_node(driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVn%2F%2FWiw%3D","line":586,"in_reply_to":"AAAAVn%2F%2FZ%2FA%3D","updated":"2014-03-06 17:19:27.000000000","message":"+1, thanks will take a look at it","commit_id":"cc227cfc71659b9a705f5a558b5a1faf1d70cb83"}]}