)]}'
{"ironic_inspector/test/unit/test_plugins_discovery.py":[{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"905133012a3090515e358a4fdea8028e74b23a5d","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        expected_data \u003d introspection_data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        discovery.enroll_node_not_found_hook(introspection_data)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        mock_create_node.assert_called_once_with("},{"line_number":70,"context_line":"            \u0027fake-hardware\u0027, ironic\u003dself.ironic,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_57f001ca","line":67,"range":{"start_line":67,"start_character":45,"end_line":67,"end_character":63},"updated":"2019-06-05 01:35:57.000000000","message":"maybe replace this one with self.data, the test is in an inconsistent state cuz it passes only because self.bmc_address is exactly identical to 1.2.3.4","commit_id":"d05e244becfc34cdd595e34e559375618f857f0c"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"318d0def60b0faa4022aee42501c9d7a97258223","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        expected_data \u003d introspection_data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        discovery.enroll_node_not_found_hook(introspection_data)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        mock_create_node.assert_called_once_with("},{"line_number":70,"context_line":"            \u0027fake-hardware\u0027, ironic\u003dself.ironic,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_cd7d426f","line":67,"range":{"start_line":67,"start_character":45,"end_line":67,"end_character":63},"in_reply_to":"9fb8cfa7_57f001ca","updated":"2019-06-05 03:42:00.000000000","message":"Yes, I noticed that. I will update in next patch.","commit_id":"d05e244becfc34cdd595e34e559375618f857f0c"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"ad67682a72954b54c5767d3efda348f62d8267d5","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d self.data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_08d2685d","line":63,"range":{"start_line":63,"start_character":8,"end_line":63,"end_character":38},"updated":"2019-06-05 06:06:47.000000000","message":"this line is not required","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"9ca42285fe9e67f2334a77fdad9d2f7402b340ef","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d self.data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_68fa0443","line":63,"range":{"start_line":63,"start_character":8,"end_line":63,"end_character":38},"in_reply_to":"9fb8cfa7_08d2685d","updated":"2019-06-05 06:40:34.000000000","message":"um, we can use self.data here. Just keeps same name for discovery.enroll_node_not_found_hook parameter.","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"ad67682a72954b54c5767d3efda348f62d8267d5","unresolved":false,"context_lines":[{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d self.data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        discovery.enroll_node_not_found_hook(introspection_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_c8cbf0a2","line":64,"range":{"start_line":64,"start_character":24,"end_line":64,"end_character":40},"updated":"2019-06-05 06:06:47.000000000","message":"It would be better to use deepcopy here.","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"9ca42285fe9e67f2334a77fdad9d2f7402b340ef","unresolved":false,"context_lines":[{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d self.data.copy()"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        discovery.enroll_node_not_found_hook(introspection_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_6873e416","line":64,"range":{"start_line":64,"start_character":24,"end_line":64,"end_character":40},"in_reply_to":"9fb8cfa7_c8cbf0a2","updated":"2019-06-05 06:40:34.000000000","message":"copy and deepcopy seems no difference in this case, but I can update.","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"ad67682a72954b54c5767d3efda348f62d8267d5","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            driver_info\u003d{\u0027ipmi_address\u0027: self.bmc_address})"},{"line_number":72,"context_line":"        mock_check_existing.assert_called_once_with("},{"line_number":73,"context_line":"            expected_data, {\u0027ipmi_address\u0027: self.bmc_address}, self.ironic)"},{"line_number":74,"context_line":"        self.assertTrue(introspection_data[\u0027auto_discovered\u0027])"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    @mock.patch.object(node_cache, \u0027create_node\u0027, autospec\u003dTrue)"},{"line_number":77,"context_line":"    @mock.patch.object(ir_utils, \u0027get_client\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_e8d03453","line":74,"range":{"start_line":74,"start_character":24,"end_line":74,"end_character":42},"updated":"2019-06-05 06:06:47.000000000","message":"and change this to expected_data.","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"9ca42285fe9e67f2334a77fdad9d2f7402b340ef","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            driver_info\u003d{\u0027ipmi_address\u0027: self.bmc_address})"},{"line_number":72,"context_line":"        mock_check_existing.assert_called_once_with("},{"line_number":73,"context_line":"            expected_data, {\u0027ipmi_address\u0027: self.bmc_address}, self.ironic)"},{"line_number":74,"context_line":"        self.assertTrue(introspection_data[\u0027auto_discovered\u0027])"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    @mock.patch.object(node_cache, \u0027create_node\u0027, autospec\u003dTrue)"},{"line_number":77,"context_line":"    @mock.patch.object(ir_utils, \u0027get_client\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_08c7c814","line":74,"range":{"start_line":74,"start_character":24,"end_line":74,"end_character":42},"in_reply_to":"9fb8cfa7_e8d03453","updated":"2019-06-05 06:40:34.000000000","message":"expected_data not update \u0027auto_discovered\u0027 key though enroll_node_not_found_hook called.","commit_id":"0d54491484fe339bdd9b4ad6bd0112b1ea98bf25"},{"author":{"_account_id":8767,"name":"Nikolay Fedotov","email":"nfedotov@cisco.com","username":"nfedotov"},"change_message_id":"b52d04e346fb246df9cd3ccec94b2395201892da","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_12aff2e3","line":63,"updated":"2019-06-05 09:23:08.000000000","message":"Code on left side {\u0027ipmi_address\u0027: \u00271.2.3.4\u0027} but self.data does not have it. Instead it contains {\u0027inventory\u0027: {\u0027bmc_address\u0027: self.bmc_address}} . I mean it is not same.\n\nI guess \u0027ipmi_address\u0027 is a legacy field and I have no idea what this is for. See comment here https://opendev.org/openstack/ironic-inspector/src/branch/master/ironic_inspector/plugins/standard.py#L246-L247\n\nIf ipmi_address is needed there should be a test for it. I suppose there a question \"Do we want to get rid of ipmi_address in introspection_data dictionary?\" and if so update before_processing method too.\n\nAnyway I do not know anything about \u0027ipmi_address\u0027 value in introspection_data.","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"bbd578e02498b1a00aa419a00bdc2eca57991516","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_c3174ea3","line":63,"in_reply_to":"9fb8cfa7_12aff2e3","updated":"2019-06-05 12:13:26.000000000","message":"Hi, Nikolay. From the old test, the dictionary {\"ipmi_address\": \u00271.2.3.4\u0027} is not a whole introspection_data, please see comments from[1].\n\nSo I use self.data as introspection_data here.\n\n[1] https://review.opendev.org/#/c/663062/1/ironic_inspector/test/unit/test_plugins_discovery.py","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"f734b896c3426166b42fe183056e8e920a5c9f4e","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_76096c93","line":63,"in_reply_to":"9fb8cfa7_1684b00b","updated":"2019-06-06 07:51:19.000000000","message":"um, I\u0027m not sure about it. In the https://opendev.org/openstack/ironic-inspector/src/branch/master/ironic_inspector/utils.py#L37 provides fallback to \u0027ipmi_address\u0027.\nSo let\u0027s see other reviewers\u0027 idea.","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"},{"author":{"_account_id":12860,"name":"Dongcan Ye","email":"yedongcan@yeah.net","username":"yedongcan"},"change_message_id":"7f7635defb7622e682f89fdda28eeaac68c17dde","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_ce8d2e32","line":63,"in_reply_to":"9fb8cfa7_6e28c233","updated":"2019-06-05 16:17:47.000000000","message":"Yes, I think we use bmc_address instead of ipmi_address in inventory. \u0027ipmi_address\u0027 used in node_driver_info. That\u0027s what we see in L71 and L73.","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"},{"author":{"_account_id":8767,"name":"Nikolay Fedotov","email":"nfedotov@cisco.com","username":"nfedotov"},"change_message_id":"30f4f53433c40d0f9685cb45e944c494d7c25ef8","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_6e28c233","line":63,"in_reply_to":"9fb8cfa7_c3174ea3","updated":"2019-06-05 16:11:23.000000000","message":"Agree. But there is no {\u0027ipmi_address\u0027: \u00271.2.3.4\u0027} in self.data, isn\u0027t it?","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"},{"author":{"_account_id":8767,"name":"Nikolay Fedotov","email":"nfedotov@cisco.com","username":"nfedotov"},"change_message_id":"e35bd209348f1146a8e38a753f9a285c9b560916","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client,"},{"line_number":61,"context_line":"                                      mock_create_node):"},{"line_number":62,"context_line":"        mock_client.return_value \u003d self.ironic"},{"line_number":63,"context_line":"        introspection_data \u003d self.data"},{"line_number":64,"context_line":"        expected_data \u003d copy.deepcopy(self.data)"},{"line_number":65,"context_line":"        mock_check_existing \u003d copy_call_args(mock_check_existing)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_1684b00b","line":63,"in_reply_to":"9fb8cfa7_ce8d2e32","updated":"2019-06-06 07:44:21.000000000","message":"Ok. If introspection_data[\u0027ipmi_address\u0027] is not used anymore we can remove it from code here https://opendev.org/openstack/ironic-inspector/src/branch/master/ironic_inspector/plugins/standard.py#L246-L247 and maybe somewhere else. \nI suppose change at line #63 should not happen in that patch. First need to update code then later update unit tests. The idea could be \"do not use introspection_data[\u0027ipmi_address\u0027] in whole project\"","commit_id":"1f1b211943fc1296f11de79171e92417dafcff8b"}]}