)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"f3b8eebd6ff32d2509b3a5d2c778098483f8bead","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b27c1bf9_562c41c5","updated":"2021-12-22 08:54:14.000000000","message":"thanks for the patch. Some questions inline.","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"0694496866c276a311d5e15eefb02020a29c4656","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"edc52bd0_267b1d0d","updated":"2022-01-11 06:07:16.000000000","message":"Thank you Steve.","commit_id":"c2d98c2294979114c702b2a662a768af0c29fadf"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"4785bb94abfdca18e0f37f770c31deef8293f714","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9e2b896f_0516abc4","updated":"2022-01-12 22:52:57.000000000","message":"recheck","commit_id":"c2d98c2294979114c702b2a662a768af0c29fadf"}],"ironic/drivers/modules/drac/bios.py":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"f3b8eebd6ff32d2509b3a5d2c778098483f8bead","unresolved":true,"context_lines":[{"line_number":415,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time_before_reboot\u0027,"},{"line_number":416,"context_line":"                                          factory_reset_time_before_reboot)"},{"line_number":417,"context_line":"            # Store the current time to later check if factory reset times out"},{"line_number":418,"context_line":"            # TODO(sbaker) Storing a timestamp with the timezone is"},{"line_number":419,"context_line":"            # inconsistent with other timestamp usages, fix the timestamp"},{"line_number":420,"context_line":"            # comparison so that node.timestamp_driver_internal_info can be"},{"line_number":421,"context_line":"            # used here."},{"line_number":422,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time\u0027,"},{"line_number":423,"context_line":"                                          timeutils.utcnow(with_timezone\u003dTrue))"},{"line_number":424,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9757e48a_9cda1fd7","line":421,"range":{"start_line":418,"start_character":12,"end_line":421,"end_character":24},"updated":"2021-12-22 08:54:14.000000000","message":"JFYI, there is reason why it is this way [1]. To be clear, as it was confusing before, it is still UTC NOW with timezone UTC+0, not system\u0027s local time or local time zone. This is necessary because of the way `timeutils.parse_isotime` works (see the link for examples). Not sure if there is another way around it, but didn\u0027t find similar cases in Ironic that needs the same.\n\nIf there is no definite plan how to update this, I rather remove this TODO or replace with explanation why can\u0027t use `timestamp_driver_internal_info`.\n\n[1] https://review.opendev.org/c/openstack/ironic/+/748696/comment/c4502f27_11144ca8/","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"871002c039316aa8e16ef359e3fb0f7600c0d8db","unresolved":true,"context_lines":[{"line_number":415,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time_before_reboot\u0027,"},{"line_number":416,"context_line":"                                          factory_reset_time_before_reboot)"},{"line_number":417,"context_line":"            # Store the current time to later check if factory reset times out"},{"line_number":418,"context_line":"            # TODO(sbaker) Storing a timestamp with the timezone is"},{"line_number":419,"context_line":"            # inconsistent with other timestamp usages, fix the timestamp"},{"line_number":420,"context_line":"            # comparison so that node.timestamp_driver_internal_info can be"},{"line_number":421,"context_line":"            # used here."},{"line_number":422,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time\u0027,"},{"line_number":423,"context_line":"                                          timeutils.utcnow(with_timezone\u003dTrue))"},{"line_number":424,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c76ce6c5_11575b22","line":421,"range":{"start_line":418,"start_character":12,"end_line":421,"end_character":24},"in_reply_to":"9757e48a_9cda1fd7","updated":"2021-12-22 09:41:49.000000000","message":"thinking further, it actually could be fixed now, as the [1]^ was mainly about another lines that stay as they are.\n\nNow ln 422-423 can be replace with \n\n  node.timestamp_driver_internal_info(\u0027factory_reset_time\u0027)\n\nas \n\n  timeutils.utcnow(with_timezone\u003dTrue)\n\ncan be replaced with \n\n timeutils.utcnow().isoformat()\n\nhere (it\u0027s the same time, only different format/data type) and it will still work at line 226 where it\u0027s used.\n\n\nAlso found another instance when similar calculation is done in redfish firmware upgrade steps [2][3]\n\n[2] https://opendev.org/openstack/ironic/src/commit/1a03e613299c04c2bb6ea929ee94e776d3da2137/ironic/drivers/modules/redfish/management.py#L831-L832\n[3] https://opendev.org/openstack/ironic/src/commit/1a03e613299c04c2bb6ea929ee94e776d3da2137/ironic/drivers/modules/redfish/management.py#L931-L933","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"c4da4064c6f0ac98f2b3c55ec35a2f6b01598688","unresolved":true,"context_lines":[{"line_number":415,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time_before_reboot\u0027,"},{"line_number":416,"context_line":"                                          factory_reset_time_before_reboot)"},{"line_number":417,"context_line":"            # Store the current time to later check if factory reset times out"},{"line_number":418,"context_line":"            # TODO(sbaker) Storing a timestamp with the timezone is"},{"line_number":419,"context_line":"            # inconsistent with other timestamp usages, fix the timestamp"},{"line_number":420,"context_line":"            # comparison so that node.timestamp_driver_internal_info can be"},{"line_number":421,"context_line":"            # used here."},{"line_number":422,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time\u0027,"},{"line_number":423,"context_line":"                                          timeutils.utcnow(with_timezone\u003dTrue))"},{"line_number":424,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"f8fda306_a2648607","line":421,"range":{"start_line":418,"start_character":12,"end_line":421,"end_character":24},"in_reply_to":"c76ce6c5_11575b22","updated":"2022-01-05 02:50:50.000000000","message":"The value is parsed with timeutils.parse_isotime, which correctly parses timeutils.utcnow().isoformat() and timeutils.utcnow(with_timezone\u003dTrue)\n\nSo I agree with Aija that we can move to timestamp_driver_internal_info here now. I\u0027ll do a follow-up which does that.","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"1ef259b7265d7eaec485c1bdf373510381975f9a","unresolved":false,"context_lines":[{"line_number":415,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time_before_reboot\u0027,"},{"line_number":416,"context_line":"                                          factory_reset_time_before_reboot)"},{"line_number":417,"context_line":"            # Store the current time to later check if factory reset times out"},{"line_number":418,"context_line":"            # TODO(sbaker) Storing a timestamp with the timezone is"},{"line_number":419,"context_line":"            # inconsistent with other timestamp usages, fix the timestamp"},{"line_number":420,"context_line":"            # comparison so that node.timestamp_driver_internal_info can be"},{"line_number":421,"context_line":"            # used here."},{"line_number":422,"context_line":"            node.set_driver_internal_info(\u0027factory_reset_time\u0027,"},{"line_number":423,"context_line":"                                          timeutils.utcnow(with_timezone\u003dTrue))"},{"line_number":424,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"085858a1_dac40b1e","line":421,"range":{"start_line":418,"start_character":12,"end_line":421,"end_character":24},"in_reply_to":"f8fda306_a2648607","updated":"2022-01-05 08:57:38.000000000","message":"thank you","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"}],"ironic/drivers/modules/drac/raid.py":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"f3b8eebd6ff32d2509b3a5d2c778098483f8bead","unresolved":true,"context_lines":[{"line_number":1012,"context_line":"        node.save()"},{"line_number":1013,"context_line":"        return"},{"line_number":1014,"context_line":""},{"line_number":1015,"context_line":"    i_raid_config_parameters \u003d []"},{"line_number":1016,"context_line":"    i_raid_config_job_ids \u003d node.driver_internal_info.get("},{"line_number":1017,"context_line":"        \u0027raid_config_job_ids\u0027, [])"},{"line_number":1018,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1bfbae2d_138b817d","line":1015,"range":{"start_line":1015,"start_character":4,"end_line":1015,"end_character":6},"updated":"2021-12-22 08:54:14.000000000","message":"why `i_`?` internal`? Maybe could leave it at out as it\u0027s not immediately clear and `raid_config_parameters` seems OK on its own.","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"cffcbc0002ae6140af5b5c721a238478469eb70e","unresolved":true,"context_lines":[{"line_number":1012,"context_line":"        node.save()"},{"line_number":1013,"context_line":"        return"},{"line_number":1014,"context_line":""},{"line_number":1015,"context_line":"    i_raid_config_parameters \u003d []"},{"line_number":1016,"context_line":"    i_raid_config_job_ids \u003d node.driver_internal_info.get("},{"line_number":1017,"context_line":"        \u0027raid_config_job_ids\u0027, [])"},{"line_number":1018,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"6810f7f0_e47411a9","line":1015,"range":{"start_line":1015,"start_character":4,"end_line":1015,"end_character":6},"in_reply_to":"1bfbae2d_138b817d","updated":"2022-01-05 02:56:14.000000000","message":"raid_config_parameters is already a variable defined on #1032, so it had to be unique","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"1ef259b7265d7eaec485c1bdf373510381975f9a","unresolved":false,"context_lines":[{"line_number":1012,"context_line":"        node.save()"},{"line_number":1013,"context_line":"        return"},{"line_number":1014,"context_line":""},{"line_number":1015,"context_line":"    i_raid_config_parameters \u003d []"},{"line_number":1016,"context_line":"    i_raid_config_job_ids \u003d node.driver_internal_info.get("},{"line_number":1017,"context_line":"        \u0027raid_config_job_ids\u0027, [])"},{"line_number":1018,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"70cbc303_9fd9ab88","line":1015,"range":{"start_line":1015,"start_character":4,"end_line":1015,"end_character":6},"in_reply_to":"6810f7f0_e47411a9","updated":"2022-01-05 08:57:38.000000000","message":"Ack","commit_id":"29a2a45fb7eef641333223dc06066183653e7984"}]}