)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"971ae4f1d9d29f16a772d6ee61c9a6ebf4f592a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"670a558b_6a788c3d","updated":"2023-01-17 23:35:02.000000000","message":"First pass review, Nothing stands out as problematic, but I need to re-review it with a fresh brain in the morning.","commit_id":"511abcef6984ab4315113ab09c9f8e2e2223e30a"}],"ironic/drivers/modules/inspect_utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"71852bfa509bd729dc01cd3c34f4b40bc39ac3e6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f295cb9f_2b975ae1","updated":"2023-01-17 13:34:10.000000000","message":"Ideally, we need unit tests for the additions in isolation.","commit_id":"2c8e0bc6579f3396abf8c2e9ceb3b2135fc94968"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"71852bfa509bd729dc01cd3c34f4b40bc39ac3e6","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        LOG.debug(\u0027Introspection data storage is disabled, the data will \u0027"},{"line_number":66,"context_line":"                  \u0027not be saved for node %(node)s\u0027, {\u0027node\u0027: node.uuid})"},{"line_number":67,"context_line":"        return"},{"line_number":68,"context_line":"    introspection_data \u003d client.get_introspection_data("},{"line_number":69,"context_line":"        node.uuid, processed\u003dTrue)"},{"line_number":70,"context_line":"    inventory_data \u003d introspection_data.pop(\"inventory\")"},{"line_number":71,"context_line":"    plugin_data \u003d introspection_data"},{"line_number":72,"context_line":"    if store_data \u003d\u003d \u0027database\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b81f0a8a_126bbcf0","line":69,"range":{"start_line":68,"start_character":0,"end_line":69,"end_character":34},"updated":"2023-01-17 13:34:10.000000000","message":"This part is specific to inspector and should stay in inspector.py (otherwise other drivers won\u0027t be able to use this call). This function should accept the data.","commit_id":"2c8e0bc6579f3396abf8c2e9ceb3b2135fc94968"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"71852bfa509bd729dc01cd3c34f4b40bc39ac3e6","unresolved":true,"context_lines":[{"line_number":102,"context_line":"            api.request.context, node.id)"},{"line_number":103,"context_line":"        return node_inventory_convert(node_inventory)"},{"line_number":104,"context_line":"    if store_data \u003d\u003d \u0027swift\u0027:"},{"line_number":105,"context_line":"        return get_introspection_data(node.uuid)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"def store_introspection_data_in_swift(node_uuid, inventory_data, plugin_data):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d4862a79_620db8e1","line":105,"updated":"2023-01-17 13:34:10.000000000","message":"Looks like a recursive call, you meant get_introspection_data_from_swift? I wonder if this part is not sufficiently unit tested.","commit_id":"2c8e0bc6579f3396abf8c2e9ceb3b2135fc94968"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0e163ab6a87e69cc7707801dd983962278c71b58","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    inventory_data \u003d introspection_data.pop(\"inventory\")"},{"line_number":69,"context_line":"    plugin_data \u003d introspection_data"},{"line_number":70,"context_line":"    if store_data \u003d\u003d \u0027database\u0027:"},{"line_number":71,"context_line":"        node_inventory.NodeInventory("},{"line_number":72,"context_line":"            node_id\u003dnode.id,"},{"line_number":73,"context_line":"            inventory_data\u003dinventory_data,"},{"line_number":74,"context_line":"            plugin_data\u003dplugin_data).create()"}],"source_content_type":"text/x-python","patch_set":6,"id":"6d755123_82dda425","line":71,"updated":"2023-01-20 13:39:19.000000000","message":"We normally pass context when creating objects, at least I wasn\u0027t able to find any exceptions. I\u0027m not sure why it works the way it does for you, but let\u0027s maybe do it?","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"8034944f509efeaba81453f58ebc4c32c1225451","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    inventory_data \u003d introspection_data.pop(\"inventory\")"},{"line_number":69,"context_line":"    plugin_data \u003d introspection_data"},{"line_number":70,"context_line":"    if store_data \u003d\u003d \u0027database\u0027:"},{"line_number":71,"context_line":"        node_inventory.NodeInventory("},{"line_number":72,"context_line":"            node_id\u003dnode.id,"},{"line_number":73,"context_line":"            inventory_data\u003dinventory_data,"},{"line_number":74,"context_line":"            plugin_data\u003dplugin_data).create()"}],"source_content_type":"text/x-python","patch_set":6,"id":"1cf8f4a5_3128fc9e","line":71,"in_reply_to":"6d755123_82dda425","updated":"2023-01-21 11:23:58.000000000","message":"Done","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0e163ab6a87e69cc7707801dd983962278c71b58","unresolved":true,"context_lines":[{"line_number":103,"context_line":"        return get_introspection_data_from_swift(node.uuid)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def store_introspection_data_in_swift(node_uuid, inventory_data, plugin_data):"},{"line_number":107,"context_line":"    \"\"\"Uploads introspection data to Swift."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    :param data: data to store in Swift"}],"source_content_type":"text/x-python","patch_set":6,"id":"7445cfb4_703c5031","line":106,"updated":"2023-01-20 13:39:19.000000000","message":"Looking at unit tests, I started wondering if we need to mark this function as private (prefix it with an underscore). Then we can avoid unit-testing it separately and potential confusion for people writing drivers. Same for get..._from_swift and node_inventory_convert.","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"8034944f509efeaba81453f58ebc4c32c1225451","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        return get_introspection_data_from_swift(node.uuid)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def store_introspection_data_in_swift(node_uuid, inventory_data, plugin_data):"},{"line_number":107,"context_line":"    \"\"\"Uploads introspection data to Swift."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    :param data: data to store in Swift"}],"source_content_type":"text/x-python","patch_set":6,"id":"8411d589_1d57e643","line":106,"in_reply_to":"7445cfb4_703c5031","updated":"2023-01-21 11:23:58.000000000","message":"Done","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ec9bed26389c5694600b30fd84795fe21d9d01cf","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    store_data \u003d CONF.inventory.data_backend"},{"line_number":96,"context_line":"    if store_data \u003d\u003d \u0027none\u0027:"},{"line_number":97,"context_line":"        raise exception.NotFound("},{"line_number":98,"context_line":"            (_(\"Cannot obtain node inventory because it was not stored\")))"},{"line_number":99,"context_line":"    if store_data \u003d\u003d \u0027database\u0027:"},{"line_number":100,"context_line":"        node_inventory \u003d objects.NodeInventory.get_by_node_id("},{"line_number":101,"context_line":"            context, node.id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7002ad8b_f82d9e2f","line":98,"updated":"2023-01-23 16:56:40.000000000","message":"nit: some redundant brackets here","commit_id":"fe69e06c7c1ba400b4714e08860d2e40ba0af0d9"}],"ironic/drivers/modules/inspector.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6398ef96e5f6caa9b74f17518ad42bbcf39afbd7","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        _inspection_error_handler(task, error)"},{"line_number":365,"context_line":"    elif status.is_finished:"},{"line_number":366,"context_line":"        _clean_up(task)"},{"line_number":367,"context_line":"        introspection_data \u003d inspector_client.get_introspection_data("},{"line_number":368,"context_line":"            node.uuid, processed\u003dTrue)"},{"line_number":369,"context_line":"        inspect_utils.store_introspection_data(node, introspection_data)"},{"line_number":370,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c0a09469_3d24f9f6","line":367,"updated":"2023-01-19 10:52:13.000000000","message":"I suggest we short-circuit this is CONF.inventory.store_backend \u003d\u003d \u0027none\u0027. I know we also check it inside of store_introspection_data, but I think not having this API call is worth the duplication.","commit_id":"511abcef6984ab4315113ab09c9f8e2e2223e30a"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"8034944f509efeaba81453f58ebc4c32c1225451","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        _inspection_error_handler(task, error)"},{"line_number":365,"context_line":"    elif status.is_finished:"},{"line_number":366,"context_line":"        _clean_up(task)"},{"line_number":367,"context_line":"        introspection_data \u003d inspector_client.get_introspection_data("},{"line_number":368,"context_line":"            node.uuid, processed\u003dTrue)"},{"line_number":369,"context_line":"        inspect_utils.store_introspection_data(node, introspection_data)"},{"line_number":370,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"f7c36e42_dd87577f","line":367,"in_reply_to":"750f91ab_2c109628","updated":"2023-01-21 11:23:58.000000000","message":"Ack","commit_id":"511abcef6984ab4315113ab09c9f8e2e2223e30a"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0e163ab6a87e69cc7707801dd983962278c71b58","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        _inspection_error_handler(task, error)"},{"line_number":365,"context_line":"    elif status.is_finished:"},{"line_number":366,"context_line":"        _clean_up(task)"},{"line_number":367,"context_line":"        introspection_data \u003d inspector_client.get_introspection_data("},{"line_number":368,"context_line":"            node.uuid, processed\u003dTrue)"},{"line_number":369,"context_line":"        inspect_utils.store_introspection_data(node, introspection_data)"},{"line_number":370,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"750f91ab_2c109628","line":367,"in_reply_to":"ae731abf_09f030b7","updated":"2023-01-20 13:39:19.000000000","message":"It\u0027s less about inspector being absent, more about not making an unnecessary API call.","commit_id":"511abcef6984ab4315113ab09c9f8e2e2223e30a"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"3a73b986eafc4553c264742b80d3e7d95697f414","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        _inspection_error_handler(task, error)"},{"line_number":365,"context_line":"    elif status.is_finished:"},{"line_number":366,"context_line":"        _clean_up(task)"},{"line_number":367,"context_line":"        introspection_data \u003d inspector_client.get_introspection_data("},{"line_number":368,"context_line":"            node.uuid, processed\u003dTrue)"},{"line_number":369,"context_line":"        inspect_utils.store_introspection_data(node, introspection_data)"},{"line_number":370,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ae731abf_09f030b7","line":367,"in_reply_to":"c0a09469_3d24f9f6","updated":"2023-01-19 17:13:29.000000000","message":"+1\n\nI think we\u0027ll get to a point where operators might not have ironic-inspector running anymore, and we\u0027ll want to be fairly defensive of that, so if we can *skip* the call, then even better now.","commit_id":"511abcef6984ab4315113ab09c9f8e2e2223e30a"}],"ironic/tests/unit/drivers/modules/test_inspect_utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0e163ab6a87e69cc7707801dd983962278c71b58","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        self.assertEqual(self.fake_inventory_data, stored[\"inventory_data\"])"},{"line_number":114,"context_line":"        self.assertEqual(self.fake_plugin_data, stored[\"plugin_data\"])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    @mock.patch.object(swift, \u0027SwiftAPI\u0027, autospec\u003dTrue)"},{"line_number":117,"context_line":"    @mock.patch.object(utils, \u0027store_introspection_data_in_swift\u0027,"},{"line_number":118,"context_line":"                       autospec\u003dTrue)"},{"line_number":119,"context_line":"    def test_store_introspection_data_swift(self, mock_store_data,"}],"source_content_type":"text/x-python","patch_set":6,"id":"598e576c_37139759","line":116,"updated":"2023-01-20 13:39:19.000000000","message":"Mocking both SwiftAPI and store... does not make sense: the latter covers the former","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"8034944f509efeaba81453f58ebc4c32c1225451","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        self.assertEqual(self.fake_inventory_data, stored[\"inventory_data\"])"},{"line_number":114,"context_line":"        self.assertEqual(self.fake_plugin_data, stored[\"plugin_data\"])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    @mock.patch.object(swift, \u0027SwiftAPI\u0027, autospec\u003dTrue)"},{"line_number":117,"context_line":"    @mock.patch.object(utils, \u0027store_introspection_data_in_swift\u0027,"},{"line_number":118,"context_line":"                       autospec\u003dTrue)"},{"line_number":119,"context_line":"    def test_store_introspection_data_swift(self, mock_store_data,"}],"source_content_type":"text/x-python","patch_set":6,"id":"73da5051_61bf71b5","line":116,"in_reply_to":"598e576c_37139759","updated":"2023-01-21 11:23:58.000000000","message":"Done","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"}],"ironic/tests/unit/drivers/modules/test_inspector.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0e163ab6a87e69cc7707801dd983962278c71b58","unresolved":true,"context_lines":[{"line_number":552,"context_line":"            self.task)"},{"line_number":553,"context_line":"        self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"    def test_status_ok_store_inventory_in_db(self, mock_client):"},{"line_number":556,"context_line":"        CONF.set_override(\u0027data_backend\u0027, \u0027database\u0027,"},{"line_number":557,"context_line":"                          group\u003d\u0027inventory\u0027)"},{"line_number":558,"context_line":"        mock_get \u003d mock_client.return_value.get_introspection"}],"source_content_type":"text/x-python","patch_set":6,"id":"c897cc53_d3bde154","line":555,"updated":"2023-01-20 13:39:19.000000000","message":"You no longer need separate tests for db and swift in this file, just more store_introspection_data.","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"8034944f509efeaba81453f58ebc4c32c1225451","unresolved":false,"context_lines":[{"line_number":552,"context_line":"            self.task)"},{"line_number":553,"context_line":"        self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"    def test_status_ok_store_inventory_in_db(self, mock_client):"},{"line_number":556,"context_line":"        CONF.set_override(\u0027data_backend\u0027, \u0027database\u0027,"},{"line_number":557,"context_line":"                          group\u003d\u0027inventory\u0027)"},{"line_number":558,"context_line":"        mock_get \u003d mock_client.return_value.get_introspection"}],"source_content_type":"text/x-python","patch_set":6,"id":"e01a0511_7b286080","line":555,"in_reply_to":"c897cc53_d3bde154","updated":"2023-01-21 11:23:58.000000000","message":"Done","commit_id":"ba41515a06e98f91599f31217a8fd75fe67fd6fa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d8eba9f5bfdac10163f914e84da192bbf8396b26","unresolved":true,"context_lines":[{"line_number":559,"context_line":"        mock_get_data \u003d mock_client.return_value.get_introspection_data"},{"line_number":560,"context_line":"        inspector._check_status(self.task)"},{"line_number":561,"context_line":"        mock_get.assert_called_once_with(self.node.uuid)"},{"line_number":562,"context_line":"        mock_get_data.assert_called_once_with(self.node.uuid, processed\u003dTrue)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"    def test_status_ok_store_inventory_nostore(self, mock_client):"},{"line_number":565,"context_line":"        CONF.set_override(\u0027data_backend\u0027, \u0027none\u0027, group\u003d\u0027inventory\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0f2c3843_7c0cfac7","line":562,"updated":"2023-01-23 13:59:40.000000000","message":"This test no longer verifies that the data is stored. You need to mock store_introspection_data and make sure it\u0027s called with the right parameters.","commit_id":"2efd702669bc033b2d68f48a6782351dbe55bbad"}]}
