)]}'
{"ironic/conductor/manager.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"36cc7e0192334b42c568c4e2e82f4ed894c999da","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            else:"},{"line_number":1384,"context_line":"                LOG.warning("},{"line_number":1385,"context_line":"                    _LW(\u0027Error in deploy of node %(node)s: %(err)s\u0027), args)"},{"line_number":1386,"context_line":"                node.last_error \u003d _(\"Failed to deploy. Error: %s\") % e"},{"line_number":1387,"context_line":"    else:"},{"line_number":1388,"context_line":"        # Update conductor_affinity to reference this conductor\u0027s ID"},{"line_number":1389,"context_line":"        # since there may be local persistent state"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_8c762e3e","line":1386,"updated":"2015-01-12 19:52:33.000000000","message":"here\u0027s a suggestion. Instead of \u0027preparing\u0027, have something like \u0027step\u0027:\n  1. step \u003d \u0027prepare to deploy\u0027\n  2. step \u003d \u0027deploy\u0027\n\nAnd then you wouldn\u0027t need the \u0027if preparing\u0027:\n\n  args \u003d {\u0027step\u0027:step, ...}\n  LOG.warning(_LW(\u0027Failed to %(step)s to node...), args)\n  node.last_error \u003d _(\"Failed to %(step)s. Error...\u0027)...","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1ae09aff069d58d7e7ed98132058abbfe00ff3b0","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            else:"},{"line_number":1384,"context_line":"                LOG.warning("},{"line_number":1385,"context_line":"                    _LW(\u0027Error in deploy of node %(node)s: %(err)s\u0027), args)"},{"line_number":1386,"context_line":"                node.last_error \u003d _(\"Failed to deploy. Error: %s\") % e"},{"line_number":1387,"context_line":"    else:"},{"line_number":1388,"context_line":"        # Update conductor_affinity to reference this conductor\u0027s ID"},{"line_number":1389,"context_line":"        # since there may be local persistent state"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_164b1260","line":1386,"in_reply_to":"3a961159_8c762e3e","updated":"2015-01-13 11:01:51.000000000","message":"If I did that, the step would be untranslated. Please forgive my French for the sake of example: \"c\u0027est un erreur maintenant l\u0027etage prepare to deploy\". I could do:\n\n    step \u003d _(\u0027prepare to deploy\u0027)\n\nbut that presupposes English-like sentence composition patterns. IE it probably won\u0027t work in Arabic, Chinese etc.","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3a289dfa35cd5ad0d48b0310530b6e40ab8396c8","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            else:"},{"line_number":1384,"context_line":"                LOG.warning("},{"line_number":1385,"context_line":"                    _LW(\u0027Error in deploy of node %(node)s: %(err)s\u0027), args)"},{"line_number":1386,"context_line":"                node.last_error \u003d _(\"Failed to deploy. Error: %s\") % e"},{"line_number":1387,"context_line":"    else:"},{"line_number":1388,"context_line":"        # Update conductor_affinity to reference this conductor\u0027s ID"},{"line_number":1389,"context_line":"        # since there may be local persistent state"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_ea50592e","line":1386,"updated":"2015-01-13 13:15:26.000000000","message":"To make the code cleaner, avoid setting flags and checking conditions, I believe it\u0027s easier to separate prepare() and deploy() into it\u0027s own try...except block, for e.g:\n\n\n try:\n     task.driver.deploy.prepare(task)\n except Exception as e:\n     with excutils.save_and_reraise_exception():\n          LOG.warning(\u0027Error while preparing ...)\n          ...\n\n try:\n     task.driver.deploy.deploy(task)\n except Exception as e:\n     with excutils.save_and_reraise_exception():\n          LOG.warning(\u0027Error in deploy ...)\n          ...\n else:\n      node.conductor_affinity \u003d conductor_id \n      ...","commit_id":"faac910f483a6a397f6109f4554feb29cb16bcfe"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"65fe92f022676777ae49588d455acfa71c6a306a","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            else:"},{"line_number":1384,"context_line":"                LOG.warning("},{"line_number":1385,"context_line":"                    _LW(\u0027Error in deploy of node %(node)s: %(err)s\u0027), args)"},{"line_number":1386,"context_line":"                node.last_error \u003d _(\"Failed to deploy. Error: %s\") % e"},{"line_number":1387,"context_line":"    else:"},{"line_number":1388,"context_line":"        # Update conductor_affinity to reference this conductor\u0027s ID"},{"line_number":1389,"context_line":"        # since there may be local persistent state"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_a51e82ba","line":1386,"in_reply_to":"3a961159_ea50592e","updated":"2015-01-13 13:34:26.000000000","message":"I\u0027m unconvinced the result will be any clearer, but I\u0027ll put up a new version and you can see.","commit_id":"faac910f483a6a397f6109f4554feb29cb16bcfe"}],"ironic/tests/conductor/test_manager.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"6688a4cdae35b9ee6e262472dbdd0ffe0bad1f43","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        self._test_do_node_deploy_validate_fail(mock_validate)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.prepare\u0027)"},{"line_number":924,"context_line":"    def test__do_node_deploy_driver_raises_prepare_error(self, mock_deploy):"},{"line_number":925,"context_line":"        self._start_service()"},{"line_number":926,"context_line":"        # test when driver.deploy.prepare raises an exception"},{"line_number":927,"context_line":"        mock_deploy.side_effect \u003d exception.InstanceDeployFailure(\u0027test\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_acc12a31","line":924,"updated":"2015-01-12 19:48:49.000000000","message":"you are mocking driver.prepare, but the variable is named mock_deploy -- please rename it.\n\nActually, please mock both driver.deploy and driver.prepare here","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1ae09aff069d58d7e7ed98132058abbfe00ff3b0","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        self._test_do_node_deploy_validate_fail(mock_validate)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.prepare\u0027)"},{"line_number":924,"context_line":"    def test__do_node_deploy_driver_raises_prepare_error(self, mock_deploy):"},{"line_number":925,"context_line":"        self._start_service()"},{"line_number":926,"context_line":"        # test when driver.deploy.prepare raises an exception"},{"line_number":927,"context_line":"        mock_deploy.side_effect \u003d exception.InstanceDeployFailure(\u0027test\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_f6df6664","line":924,"in_reply_to":"3a961159_acc12a31","updated":"2015-01-13 11:01:51.000000000","message":"Done","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"6688a4cdae35b9ee6e262472dbdd0ffe0bad1f43","unresolved":false,"context_lines":[{"line_number":939,"context_line":"        #             any longer. Instead, it is cleared when the instance"},{"line_number":940,"context_line":"        #             is deleted."},{"line_number":941,"context_line":"        self.assertEqual(states.ACTIVE, node.target_provision_state)"},{"line_number":942,"context_line":"        self.assertTrue(node.last_error.startswith("},{"line_number":943,"context_line":"            \u0027Failed to prepare to deploy\u0027))"},{"line_number":944,"context_line":"        mock_deploy.assert_called_once_with(mock.ANY)"},{"line_number":945,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_4c44461b","line":942,"updated":"2015-01-12 19:48:49.000000000","message":"We can\u0027t be comparing to a translated error string within a unit test -- this may cause tests to fail if LOCALE is different and the referenced string is translated.","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1ae09aff069d58d7e7ed98132058abbfe00ff3b0","unresolved":false,"context_lines":[{"line_number":939,"context_line":"        #             any longer. Instead, it is cleared when the instance"},{"line_number":940,"context_line":"        #             is deleted."},{"line_number":941,"context_line":"        self.assertEqual(states.ACTIVE, node.target_provision_state)"},{"line_number":942,"context_line":"        self.assertTrue(node.last_error.startswith("},{"line_number":943,"context_line":"            \u0027Failed to prepare to deploy\u0027))"},{"line_number":944,"context_line":"        mock_deploy.assert_called_once_with(mock.ANY)"},{"line_number":945,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_f6fbe6bb","line":942,"in_reply_to":"3a961159_4c44461b","updated":"2015-01-13 11:01:51.000000000","message":"Done","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"6688a4cdae35b9ee6e262472dbdd0ffe0bad1f43","unresolved":false,"context_lines":[{"line_number":941,"context_line":"        self.assertEqual(states.ACTIVE, node.target_provision_state)"},{"line_number":942,"context_line":"        self.assertTrue(node.last_error.startswith("},{"line_number":943,"context_line":"            \u0027Failed to prepare to deploy\u0027))"},{"line_number":944,"context_line":"        mock_deploy.assert_called_once_with(mock.ANY)"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.deploy\u0027)"},{"line_number":947,"context_line":"    def test__do_node_deploy_driver_raises_error(self, mock_deploy):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_6c3ca231","line":944,"updated":"2015-01-12 19:48:49.000000000","message":"Please also check that driver.deploy was not called when driver.prepare raised an error. The variable here is misnamed -- I suggest the following two assertions instead (assuming you rename the variable and add the second mock I suggested above)\n\n  self.assertTrue(mock_prepare.called)\n  self.assertFalse(mock_deploy.called)","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1ae09aff069d58d7e7ed98132058abbfe00ff3b0","unresolved":false,"context_lines":[{"line_number":941,"context_line":"        self.assertEqual(states.ACTIVE, node.target_provision_state)"},{"line_number":942,"context_line":"        self.assertTrue(node.last_error.startswith("},{"line_number":943,"context_line":"            \u0027Failed to prepare to deploy\u0027))"},{"line_number":944,"context_line":"        mock_deploy.assert_called_once_with(mock.ANY)"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.deploy\u0027)"},{"line_number":947,"context_line":"    def test__do_node_deploy_driver_raises_error(self, mock_deploy):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_162a322f","line":944,"in_reply_to":"3a961159_6c3ca231","updated":"2015-01-13 11:01:51.000000000","message":"Done","commit_id":"046dae8e2c9c88871f45274c2867c67890dab4da"}]}