)]}'
{"ironic/tests/unit/drivers/modules/test_ipmitool.py":[{"author":{"_account_id":16635,"name":"deray","email":"debayan.ray@gmail.com","username":"deray"},"change_message_id":"7a5f5cec61a2e96b871836a44a91974f87b7de29","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        with tempfile.NamedTemporaryFile() as pw_file_handle:"},{"line_number":1077,"context_line":"            pw_file \u003d pw_file_handle.name"},{"line_number":1078,"context_line":"            file_handle \u003d open(pw_file, \"w\")"},{"line_number":1079,"context_line":"            args \u003d ["},{"line_number":1080,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1081,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_3ecbef1f","line":1078,"range":{"start_line":1078,"start_character":26,"end_line":1078,"end_character":44},"updated":"2016-03-17 08:30:39.000000000","message":"is it really required to create a file for the UT case? Can\u0027t we mock a file object to cater to the needs of this test case?","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"104364d61a79c789c61f112885f5acf9bf5fb365","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        with tempfile.NamedTemporaryFile() as pw_file_handle:"},{"line_number":1077,"context_line":"            pw_file \u003d pw_file_handle.name"},{"line_number":1078,"context_line":"            file_handle \u003d open(pw_file, \"w\")"},{"line_number":1079,"context_line":"            args \u003d ["},{"line_number":1080,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1081,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_ae831e55","line":1078,"range":{"start_line":1078,"start_character":26,"end_line":1078,"end_character":44},"in_reply_to":"1af94dfe_3ecbef1f","updated":"2016-03-18 00:56:50.000000000","message":"I agree with you that it is not really required.\nAnd I confirmed that the code you proposed at the line 1101 certainly worked.\n\nBut I think it is worthy to process it in another patch because it is a little bit diverted from the problem this patch proposed to fix, and also there are several places we can apply your proposed code in this file, test_ipmitool.py.","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":16635,"name":"deray","email":"debayan.ray@gmail.com","username":"deray"},"change_message_id":"7a5f5cec61a2e96b871836a44a91974f87b7de29","unresolved":false,"context_lines":[{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1090,"context_line":"            mock_pwf.return_value \u003d file_handle"},{"line_number":1091,"context_line":"            mock_exec.side_effect \u003d iter("},{"line_number":1092,"context_line":"                [processutils.ProcessExecutionError(\"x\")])"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1095,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_fef8674b","line":1092,"range":{"start_line":1091,"start_character":36,"end_line":1092,"end_character":17},"updated":"2016-03-17 08:30:39.000000000","message":"casting it to ``iter`` is not required.\n\n  mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"104364d61a79c789c61f112885f5acf9bf5fb365","unresolved":false,"context_lines":[{"line_number":1088,"context_line":""},{"line_number":1089,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1090,"context_line":"            mock_pwf.return_value \u003d file_handle"},{"line_number":1091,"context_line":"            mock_exec.side_effect \u003d iter("},{"line_number":1092,"context_line":"                [processutils.ProcessExecutionError(\"x\")])"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1095,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_8e7aa286","line":1092,"range":{"start_line":1091,"start_character":36,"end_line":1092,"end_character":17},"in_reply_to":"1af94dfe_fef8674b","updated":"2016-03-18 00:56:50.000000000","message":"I\u0027ll remove that.","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"10c4ba49fc3d384857f73c7c980f73a97f48e440","unresolved":false,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1095,"context_line":"                              ipmi._exec_ipmitool,"},{"line_number":1096,"context_line":"                              self.info, \u0027A B C\u0027)"},{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_5d008501","line":1096,"updated":"2016-03-17 06:24:06.000000000","message":"You could also explicitly close the file by using pw_file_handle.close()","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"104364d61a79c789c61f112885f5acf9bf5fb365","unresolved":false,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1095,"context_line":"                              ipmi._exec_ipmitool,"},{"line_number":1096,"context_line":"                              self.info, \u0027A B C\u0027)"},{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_ae75de98","line":1096,"in_reply_to":"1af94dfe_5d008501","updated":"2016-03-18 00:56:50.000000000","message":"I\u0027ll add that.","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":16635,"name":"deray","email":"debayan.ray@gmail.com","username":"deray"},"change_message_id":"7a5f5cec61a2e96b871836a44a91974f87b7de29","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"},{"line_number":1100,"context_line":"        self.assertEqual(1, mock_exec.call_count)"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)"},{"line_number":1103,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1104,"context_line":"    def test__exec_ipmitool_exception_retry("}],"source_content_type":"text/x-python","patch_set":1,"id":"1af94dfe_926f97c4","line":1101,"updated":"2016-03-17 08:30:39.000000000","message":"why not make it this way then?::\n\n    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)\n    @mock.patch.object(ipmi, \u0027_make_password_file\u0027, autospec\u003dTrue)\n    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)\n    def test__exec_ipmitool_exception(\n            self, mock_exec, mock_pwf, mock_support, mock_sleep):\n        pw_file_mock \u003d mock.MagicMock(spec\u003dfile)\n        args \u003d [\n            \u0027ipmitool\u0027,\n            \u0027-I\u0027, \u0027lanplus\u0027,\n            \u0027-H\u0027, self.info[\u0027address\u0027],\n            \u0027-L\u0027, self.info[\u0027priv_level\u0027],\n            \u0027-U\u0027, self.info[\u0027username\u0027],\n            \u0027-f\u0027, pw_file_mock.__enter__(),\n            \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,\n        ]\n\n        mock_support.return_value \u003d False\n        mock_pwf.return_value \u003d pw_file_mock\n        mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")\n\n        self.assertRaises(processutils.ProcessExecutionError,\n                          ipmi._exec_ipmitool,\n                          self.info, \u0027A B C\u0027)\n        mock_support.assert_called_once_with(\u0027timing\u0027)\n        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])\n        mock_exec.assert_called_once_with(*args)\n        self.assertEqual(1, mock_exec.call_count)\n\nof course: you need to do this after the import statements to keep py34 happy.\n\n  if six.PY3:\n    import io\n    file \u003d io.BytesIO","commit_id":"cfd7ca6d944906b3a6027fe5dd34b6774d4f077e"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"11542114e8dcdf15fce0d3e83457dd7149cb4f5c","unresolved":false,"context_lines":[{"line_number":1093,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1094,"context_line":"                              ipmi._exec_ipmitool,"},{"line_number":1095,"context_line":"                              self.info, \u0027A B C\u0027)"},{"line_number":1096,"context_line":"            pw_file_handle.close()"},{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1af94dfe_acefa61c","line":1096,"updated":"2016-03-18 02:14:53.000000000","message":"Per my understanding, the exception you want to fix is caused  in L1093, which didn\u0027t close the file_handle after raise exception. So you can either\n1. using \"with\" statement which will close the file_handle in __exit__ like you did before\nOr\n2. You manually close the file_handler like I suggest.\nYou don\u0027t have to do them both.","commit_id":"cbe28c4af4a282da13592315b766ee2958a35882"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"38bd84c237ffa9643ba97fb257d1833a86dcc8aa","unresolved":false,"context_lines":[{"line_number":1093,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1094,"context_line":"                              ipmi._exec_ipmitool,"},{"line_number":1095,"context_line":"                              self.info, \u0027A B C\u0027)"},{"line_number":1096,"context_line":"            pw_file_handle.close()"},{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1af94dfe_ec3d2ede","line":1096,"in_reply_to":"1af94dfe_acefa61c","updated":"2016-03-18 02:56:00.000000000","message":"My understanding is that there are two file handles.\n\nOne is opened by __enter__(), and closed by __exit__().\n\nThe other is opened manually by open() at L.1076.\n\nSo two file handles point to the same file.\n\nTherefor we need to close both, what do you think?","commit_id":"cbe28c4af4a282da13592315b766ee2958a35882"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"ad6013ba93a69ca1c246be70b289ef63d00ec823","unresolved":false,"context_lines":[{"line_number":1077,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1078,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1079,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1080,"context_line":"        pw_file_mock \u003d mock.MagicMock(spec\u003dfile)"},{"line_number":1081,"context_line":"        args \u003d ["},{"line_number":1082,"context_line":"            \u0027ipmitool\u0027,"},{"line_number":1083,"context_line":"            \u0027-I\u0027, \u0027lanplus\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_6695533f","line":1080,"updated":"2016-03-21 08:43:12.000000000","message":"but I just realize you can use mock_open like L380, and you don\u0027t have to handle the detail by yourself and import io.BytesIO","commit_id":"6b4ca86313b0cfc72ec27531e34c9cbe5d282dc4"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"83d3c048c4a10ce12b2b3a16960e58d0af6eda71","unresolved":false,"context_lines":[{"line_number":1077,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1078,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1079,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1080,"context_line":"        pw_file_mock \u003d mock.MagicMock(spec\u003dfile)"},{"line_number":1081,"context_line":"        args \u003d ["},{"line_number":1082,"context_line":"            \u0027ipmitool\u0027,"},{"line_number":1083,"context_line":"            \u0027-I\u0027, \u0027lanplus\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_306363de","line":1080,"in_reply_to":"fa0719c6_6695533f","updated":"2016-03-22 00:53:31.000000000","message":"Oh, that\u0027s nice to enable this fix without importing io.BytesIO.\nI\u0027ll fix that.","commit_id":"6b4ca86313b0cfc72ec27531e34c9cbe5d282dc4"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"28fb06880632c2b8545c8a582bffb215b8dd7bdd","unresolved":false,"context_lines":[{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"},{"line_number":1100,"context_line":"        self.assertEqual(1, mock_exec.call_count)"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)"},{"line_number":1103,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_280e8934","line":1100,"updated":"2016-03-21 08:33:33.000000000","message":"this line is not really needed, as previous line already asserts that this mock was called only *once*","commit_id":"6b4ca86313b0cfc72ec27531e34c9cbe5d282dc4"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"83d3c048c4a10ce12b2b3a16960e58d0af6eda71","unresolved":false,"context_lines":[{"line_number":1097,"context_line":"        mock_support.assert_called_once_with(\u0027timing\u0027)"},{"line_number":1098,"context_line":"        mock_pwf.assert_called_once_with(self.info[\u0027password\u0027])"},{"line_number":1099,"context_line":"        mock_exec.assert_called_once_with(*args)"},{"line_number":1100,"context_line":"        self.assertEqual(1, mock_exec.call_count)"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)"},{"line_number":1103,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_b07c9373","line":1100,"in_reply_to":"fa0719c6_280e8934","updated":"2016-03-22 00:53:31.000000000","message":"I checked the commit log and changes of this line.\nPlease find it, it seems this line is necessary.\n\nhttps://github.com/openstack/ironic/commit/59ccf51c179a92a1e9541fa54715ace8b110de91","commit_id":"6b4ca86313b0cfc72ec27531e34c9cbe5d282dc4"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"8138b8da61443041914c75e7549518d6c81fba58","unresolved":false,"context_lines":[{"line_number":1073,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1074,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        pw_file_mock \u003d mock.mock_open(mock.MagicMock(name\u003d\u0027pw_file\u0027))"},{"line_number":1077,"context_line":"        with mock.patch(\u0027tempfile.NamedTemporaryFile\u0027, pw_file_mock):"},{"line_number":1078,"context_line":"            args \u003d ["},{"line_number":1079,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1080,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"},{"line_number":1081,"context_line":"                \u0027-H\u0027, self.info[\u0027address\u0027],"},{"line_number":1082,"context_line":"                \u0027-L\u0027, self.info[\u0027priv_level\u0027],"},{"line_number":1083,"context_line":"                \u0027-U\u0027, self.info[\u0027username\u0027],"},{"line_number":1084,"context_line":"                \u0027-f\u0027, pw_file_mock.__enter__(),"},{"line_number":1085,"context_line":"                \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,"},{"line_number":1086,"context_line":"            ]"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1089,"context_line":"            mock_pwf.return_value \u003d pw_file_mock"},{"line_number":1090,"context_line":"            mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")"},{"line_number":1091,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1092,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_7f1c8013","line":1089,"range":{"start_line":1076,"start_character":0,"end_line":1089,"end_character":48},"updated":"2016-03-22 09:09:29.000000000","message":"This works but I would suggest you do like this:\n  pw_file_mock \u003d mock.mock_open(mock.MagicMock(name\u003d\u0027pw_file\u0027))\n  with mock.patch(\u0027tempfile.NamedTemporaryFile\u0027, pw_file_mock):\n      mock_file_handle \u003d pw_file_mock.return_value\n      args \u003d [\n          \u0027ipmitool\u0027,\n          \u0027-I\u0027, \u0027lanplus\u0027,\n          \u0027-H\u0027, self.info[\u0027address\u0027],\n          \u0027-L\u0027, self.info[\u0027priv_level\u0027],\n          \u0027-U\u0027, self.info[\u0027username\u0027],\n          \u0027-f\u0027, mock_file_handle,\n          \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,\n      ]\n\n      mock_support.return_value \u003d False\n      mock_pwf.return_value \u003d mock_file_handle\n\nIt\u0027s bad to call a private method like __enter__() WDYT?","commit_id":"456381430eaf6c934c88d6273f01e833e0206049"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"f7cfc08d39121628193a344986e7e88a667c0237","unresolved":false,"context_lines":[{"line_number":1073,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1074,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        pw_file_mock \u003d mock.mock_open(mock.MagicMock(name\u003d\u0027pw_file\u0027))"},{"line_number":1077,"context_line":"        with mock.patch(\u0027tempfile.NamedTemporaryFile\u0027, pw_file_mock):"},{"line_number":1078,"context_line":"            args \u003d ["},{"line_number":1079,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1080,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"},{"line_number":1081,"context_line":"                \u0027-H\u0027, self.info[\u0027address\u0027],"},{"line_number":1082,"context_line":"                \u0027-L\u0027, self.info[\u0027priv_level\u0027],"},{"line_number":1083,"context_line":"                \u0027-U\u0027, self.info[\u0027username\u0027],"},{"line_number":1084,"context_line":"                \u0027-f\u0027, pw_file_mock.__enter__(),"},{"line_number":1085,"context_line":"                \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,"},{"line_number":1086,"context_line":"            ]"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1089,"context_line":"            mock_pwf.return_value \u003d pw_file_mock"},{"line_number":1090,"context_line":"            mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")"},{"line_number":1091,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1092,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_0d19a5d7","line":1089,"range":{"start_line":1076,"start_character":0,"end_line":1089,"end_character":48},"in_reply_to":"fa0719c6_09a1a0a3","updated":"2016-03-23 06:35:05.000000000","message":"Yes, you are right for two file handles. But I change my mind, the tempfile.NamedTemporaryFile is useless. The previously problem is we create a temp file but didn\u0027t close it. So in that case, \u0027with\u0027 can solve that.\nBut right now, we didn\u0027t have to create tempfile anymore, we just need to pass a mock_pw_file to ipmitool. So I would suggest do like this:\n\npw_file_mock \u003d mock.mock_open()\nmock_file_handle \u003d pw_file_mock.return_value\nargs \u003d [\n          \u0027ipmitool\u0027,\n          \u0027-I\u0027, \u0027lanplus\u0027,\n          \u0027-H\u0027, self.info[\u0027address\u0027],\n          \u0027-L\u0027, self.info[\u0027priv_level\u0027],\n          \u0027-U\u0027, self.info[\u0027username\u0027],\n          \u0027-f\u0027, mock_file_handle,\n          \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,\n      ]\n      mock_support.return_value \u003d False\n      mock_pwf.return_value \u003d mock_file_handle","commit_id":"456381430eaf6c934c88d6273f01e833e0206049"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"f350b100f21044e2c9543fd6389d6cc8176b6167","unresolved":false,"context_lines":[{"line_number":1073,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1074,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        pw_file_mock \u003d mock.mock_open(mock.MagicMock(name\u003d\u0027pw_file\u0027))"},{"line_number":1077,"context_line":"        with mock.patch(\u0027tempfile.NamedTemporaryFile\u0027, pw_file_mock):"},{"line_number":1078,"context_line":"            args \u003d ["},{"line_number":1079,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1080,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"},{"line_number":1081,"context_line":"                \u0027-H\u0027, self.info[\u0027address\u0027],"},{"line_number":1082,"context_line":"                \u0027-L\u0027, self.info[\u0027priv_level\u0027],"},{"line_number":1083,"context_line":"                \u0027-U\u0027, self.info[\u0027username\u0027],"},{"line_number":1084,"context_line":"                \u0027-f\u0027, pw_file_mock.__enter__(),"},{"line_number":1085,"context_line":"                \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,"},{"line_number":1086,"context_line":"            ]"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1089,"context_line":"            mock_pwf.return_value \u003d pw_file_mock"},{"line_number":1090,"context_line":"            mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")"},{"line_number":1091,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1092,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_5e5a252b","line":1089,"range":{"start_line":1076,"start_character":0,"end_line":1089,"end_character":48},"in_reply_to":"fa0719c6_0d19a5d7","updated":"2016-03-23 07:34:07.000000000","message":"Okay, Tan, we reached to the simplest solution :-)\nI\u0027ll post the update shortly.\n\nBest regards,\nNaohiro","commit_id":"456381430eaf6c934c88d6273f01e833e0206049"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"3f82ff1acd626bd608c64e26a25c1dba0dd41cea","unresolved":false,"context_lines":[{"line_number":1073,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1074,"context_line":"    def test__exec_ipmitool_exception("},{"line_number":1075,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":1076,"context_line":"        pw_file_mock \u003d mock.mock_open(mock.MagicMock(name\u003d\u0027pw_file\u0027))"},{"line_number":1077,"context_line":"        with mock.patch(\u0027tempfile.NamedTemporaryFile\u0027, pw_file_mock):"},{"line_number":1078,"context_line":"            args \u003d ["},{"line_number":1079,"context_line":"                \u0027ipmitool\u0027,"},{"line_number":1080,"context_line":"                \u0027-I\u0027, \u0027lanplus\u0027,"},{"line_number":1081,"context_line":"                \u0027-H\u0027, self.info[\u0027address\u0027],"},{"line_number":1082,"context_line":"                \u0027-L\u0027, self.info[\u0027priv_level\u0027],"},{"line_number":1083,"context_line":"                \u0027-U\u0027, self.info[\u0027username\u0027],"},{"line_number":1084,"context_line":"                \u0027-f\u0027, pw_file_mock.__enter__(),"},{"line_number":1085,"context_line":"                \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,"},{"line_number":1086,"context_line":"            ]"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"            mock_support.return_value \u003d False"},{"line_number":1089,"context_line":"            mock_pwf.return_value \u003d pw_file_mock"},{"line_number":1090,"context_line":"            mock_exec.side_effect \u003d processutils.ProcessExecutionError(\"x\")"},{"line_number":1091,"context_line":"            self.assertRaises(processutils.ProcessExecutionError,"},{"line_number":1092,"context_line":"                              ipmi._exec_ipmitool,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_09a1a0a3","line":1089,"range":{"start_line":1076,"start_character":0,"end_line":1089,"end_character":48},"in_reply_to":"fa0719c6_7f1c8013","updated":"2016-03-23 00:39:26.000000000","message":"Tan,\n\nGenerally speaking, yes, we shouldn\u0027t call any private method.\nHowever calling \"pw_file_mock.return_value\" is NOT semantically correct.\nBecause the pw_file_mock is a mock of open() function, if we call \"pw_file_mock.return_value\", then we open the second file handle.\nAlthough tempfile.NamedTemporaryFile has already opened the first file handle via __enter__().\nSo I have to ask the same question I asked in the patch set 2.\n\n\u003e My understanding is that there are two file handles.\n\u003e One is opened by __enter__(), and closed by __exit__().\n\u003e The other is opened manually by open() at L.1076.\n\u003e So two file handles point to the same file.\n\u003e Therefor we need to close both, what do you think?\n\nBest regards,\nNaohiro","commit_id":"456381430eaf6c934c88d6273f01e833e0206049"},{"author":{"_account_id":16635,"name":"deray","email":"debayan.ray@gmail.com","username":"deray"},"change_message_id":"4210d3815f7644897f93efc2b29d874958705833","unresolved":false,"context_lines":[{"line_number":658,"context_line":"            \u0027-H\u0027, self.info[\u0027address\u0027],"},{"line_number":659,"context_line":"            \u0027-L\u0027, self.info[\u0027priv_level\u0027],"},{"line_number":660,"context_line":"            \u0027-U\u0027, self.info[\u0027username\u0027],"},{"line_number":661,"context_line":"            \u0027-f\u0027, mock_file_handle,"},{"line_number":662,"context_line":"            \u0027A\u0027, \u0027B\u0027, \u0027C\u0027,"},{"line_number":663,"context_line":"        ]"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba0121b8_dfd2d3d1","line":661,"range":{"start_line":661,"start_character":18,"end_line":661,"end_character":34},"updated":"2016-03-30 09:38:43.000000000","message":"It\u0027s not a file_handle semantically speaking. It should be the file name rather. Btw, please refer my general review comment section for my take on this. This applies to all the changes in this file.","commit_id":"9b97dc91291ca7bb654badfa7cf28a25854bab4a"},{"author":{"_account_id":13362,"name":"Tan Lin","email":"lin.tan@intel.com","username":"zero"},"change_message_id":"63f386a3a425794a5337dd67741b74a128ee26e7","unresolved":false,"context_lines":[{"line_number":680,"context_line":"            self, mock_exec, mock_pwf, mock_support, mock_sleep):"},{"line_number":681,"context_line":"        ipmi.LAST_CMD_TIME \u003d {}"},{"line_number":682,"context_line":"        pw_file_mock \u003d mock.mock_open()"},{"line_number":683,"context_line":"        mock_file_handle \u003d pw_file_mock.return_value"},{"line_number":684,"context_line":"        args \u003d [["},{"line_number":685,"context_line":"            \u0027ipmitool\u0027,"},{"line_number":686,"context_line":"            \u0027-I\u0027, \u0027lanplus\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_38810990","line":683,"updated":"2016-03-24 02:48:50.000000000","message":"better to create two file_handles :)","commit_id":"9b97dc91291ca7bb654badfa7cf28a25854bab4a"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"cfa89345f2483ae15180dddf4a0a55cb598f53b9","unresolved":false,"context_lines":[{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)"},{"line_number":889,"context_line":"    @mock.patch.object("},{"line_number":890,"context_line":"        ipmi, \u0027_make_password_file\u0027, wraps\u003d_make_password_file_stub)"},{"line_number":891,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":892,"context_line":"    def test__exec_ipmitool_without_password(self, mock_exec,"},{"line_number":893,"context_line":"                                             _make_password_file_mock,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a061dce_3dd1a5ce","line":890,"updated":"2016-04-04 14:00:43.000000000","message":"Hm, interesting, mock.patch docstring does not mention that wraps is also available here, not only for mock.Mock constructor. But it seems to work :)","commit_id":"b35b0de7e0cfaa2eab3ad161d83e07040d5db526"},{"author":{"_account_id":16635,"name":"deray","email":"debayan.ray@gmail.com","username":"deray"},"change_message_id":"e06ac09f0eb2765161234a628ca2c333208ecae7","unresolved":false,"context_lines":[{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    @mock.patch.object(ipmi, \u0027_is_option_supported\u0027, autospec\u003dTrue)"},{"line_number":889,"context_line":"    @mock.patch.object("},{"line_number":890,"context_line":"        ipmi, \u0027_make_password_file\u0027, wraps\u003d_make_password_file_stub)"},{"line_number":891,"context_line":"    @mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":892,"context_line":"    def test__exec_ipmitool_without_password(self, mock_exec,"},{"line_number":893,"context_line":"                                             _make_password_file_mock,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a061dce_030564c1","line":890,"in_reply_to":"9a061dce_3dd1a5ce","updated":"2016-04-04 14:28:38.000000000","message":"@Vdrok, good observation. Just now, going thru the docs figured out a place which might seem to capture this:\n\nhttps://docs.python.org/3/library/unittest.mock.html#unittest.mock.patch\n\nJust before the patch example there it says::\n\n  \"patch() takes arbitrary keyword arguments. These will be passed to the Mock (or new_callable) on construction.\"","commit_id":"b35b0de7e0cfaa2eab3ad161d83e07040d5db526"}]}