)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7bd31f23_662cb8d7","updated":"2025-03-06 12:08:45.000000000","message":"Thank you, I really like where this is moving! The argument handling is nice and tidy now. I\u0027ve left a bunch of comments, most of which are rather minor.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"a8da48a5a379d06ea728e589355f2d04f7a3304c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7391d5e5_76477bff","updated":"2025-03-05 14:17:50.000000000","message":"recheck need updated ci results as numerous ci fixes have merged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"014a73f56a44299c672a1340c719edd43e3cf81a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"cdda55f1_1d172330","updated":"2025-03-10 19:16:18.000000000","message":"Hopefully better test coverage now. Tempest will be a follow-up after merge (likely by me or not).","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"771360eb_4c51990e","updated":"2025-03-11 14:39:05.000000000","message":"Thank you so much, I really appreciate you working on this huge feature!\n\nI\u0027ve left a couple more comments, they can be addressed in another follow-up since this one is pretty important to merge soon.","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d65d5536b2e537d368e6fa418cfc48de3504f69f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"33b6cdfe_67a92c31","updated":"2025-03-11 16:23:57.000000000","message":"recheck POST_FAILURE","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"}],"ironic/common/exception.py":[{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"aabf53e8e5952300d536cbd2c4d2ddb7927b1353","unresolved":true,"context_lines":[{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"class RuleConditionCheckFailure(IronicException):"},{"line_number":1107,"context_line":"    \"\"\"Raised when an inspection rule action fails during execution.\"\"\""},{"line_number":1108,"context_line":"    _msg_fmt \u003d _(\"Inspection rule condition check failed. Reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"2fa56e60_315837bb","line":1107,"updated":"2025-02-27 11:07:38.000000000","message":"In the docstring, it should say inspection rule condition, not action","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"468c5a35a1cf7b6d089544a788fda6bb3a428a2a","unresolved":false,"context_lines":[{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"class RuleConditionCheckFailure(IronicException):"},{"line_number":1107,"context_line":"    \"\"\"Raised when an inspection rule action fails during execution.\"\"\""},{"line_number":1108,"context_line":"    _msg_fmt \u003d _(\"Inspection rule condition check failed. Reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"3984eb57_661c7c11","line":1107,"in_reply_to":"2fa56e60_315837bb","updated":"2025-03-06 19:22:41.000000000","message":"Acknowledged","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":1097,"context_line":"    _msg_fmt \u003d _(\"Rule %(rule)s could not be found.\")"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"class RuleActionExecutionFailure(IronicException):"},{"line_number":1101,"context_line":"    \"\"\"Raised when an inspection rule action fails during execution.\"\"\""},{"line_number":1102,"context_line":"    _msg_fmt \u003d _(\"Inspection rule action execution failed. \""},{"line_number":1103,"context_line":"                 \"Reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"d7937a9e_93feb369","line":1100,"updated":"2025-03-06 12:08:45.000000000","message":"I think these two exceptions should inherit HardwareInspectionFailure, wdyt?","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":true,"context_lines":[{"line_number":1097,"context_line":"    _msg_fmt \u003d _(\"Rule %(rule)s could not be found.\")"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"class RuleActionExecutionFailure(IronicException):"},{"line_number":1101,"context_line":"    \"\"\"Raised when an inspection rule action fails during execution.\"\"\""},{"line_number":1102,"context_line":"    _msg_fmt \u003d _(\"Inspection rule action execution failed. \""},{"line_number":1103,"context_line":"                 \"Reason: %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"a922279b_fb7dd519","line":1100,"in_reply_to":"d7937a9e_93feb369","updated":"2025-03-06 19:22:01.000000000","message":"Makes sense. Acknowledged!","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"d1e812567a1adb49ba05d9aa4322cb54e91d3ba9","unresolved":true,"context_lines":[{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class InspectionRuleExecutionFailure(HardwareInspectionFailure):"},{"line_number":1106,"context_line":"    \"\"\"Raised when an inspection rule action fails during execution.\"\"\""},{"line_number":1107,"context_line":"    _msg_fmt \u003d _(\"Inspection rule execution failed. Reason: %(reason)s\")"},{"line_number":1108,"context_line":""},{"line_number":1109,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ae4a3117_99d24cc6","line":1106,"updated":"2025-03-07 12:40:45.000000000","message":"nit: inspection rule execution failes","commit_id":"efe8f53c4a91678ad3b3a3c7fc1074ef44d480b9"}],"ironic/common/inspection_rules/actions.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":90,"context_line":"    VALID_LOG_LEVELS \u003d {\u0027debug\u0027, \u0027info\u0027, \u0027warning\u0027, \u0027error\u0027, \u0027critical\u0027}"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def __call__(self, task, msg, level\u003d\u0027info\u0027):"},{"line_number":93,"context_line":"        if level.lower() not in self.VALID_LOG_LEVELS:"},{"line_number":94,"context_line":"            raise ValueError("},{"line_number":95,"context_line":"                _(\"Invalid log level: %(level)s. Choose from %(levels)s\" %"},{"line_number":96,"context_line":"                  {\u0027level\u0027: level, \u0027levels\u0027: self.VALID_LOG_LEVELS}"}],"source_content_type":"text/x-python","patch_set":10,"id":"b3c165d5_a3a670d1","line":93,"updated":"2025-03-06 12:08:45.000000000","message":"If you want to support any case, you need to convert level to lower-case first, then do this check and getattr() below. Otherwise, the getattr() call will get a wrong level even if this check passes.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    VALID_LOG_LEVELS \u003d {\u0027debug\u0027, \u0027info\u0027, \u0027warning\u0027, \u0027error\u0027, \u0027critical\u0027}"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def __call__(self, task, msg, level\u003d\u0027info\u0027):"},{"line_number":93,"context_line":"        if level.lower() not in self.VALID_LOG_LEVELS:"},{"line_number":94,"context_line":"            raise ValueError("},{"line_number":95,"context_line":"                _(\"Invalid log level: %(level)s. Choose from %(levels)s\" %"},{"line_number":96,"context_line":"                  {\u0027level\u0027: level, \u0027levels\u0027: self.VALID_LOG_LEVELS}"}],"source_content_type":"text/x-python","patch_set":10,"id":"6356de4d_e2a5fb8d","line":93,"in_reply_to":"b3c165d5_a3a670d1","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":124,"context_line":"        except Exception as exc:"},{"line_number":125,"context_line":"            msg \u003d _(\"Failed to set attribute %(path)s \""},{"line_number":126,"context_line":"                    \"with value %(value)s: %(exc)s\" %"},{"line_number":127,"context_line":"                    {\u0027path\u0027: path, \u0027value\u0027: value, \u0027exc\u0027: exc})"},{"line_number":128,"context_line":"            LOG.error(msg)"},{"line_number":129,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"81aa8b1b_c8b3ca0b","line":127,"updated":"2025-03-06 12:08:45.000000000","message":"The _() part should not include the formatting argument, only the constant template (I mean, it has to end before %). Otherwise, the i18n tooling will not recognize the string to translate.\n\nSee line 149 for correct usage.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        except Exception as exc:"},{"line_number":125,"context_line":"            msg \u003d _(\"Failed to set attribute %(path)s \""},{"line_number":126,"context_line":"                    \"with value %(value)s: %(exc)s\" %"},{"line_number":127,"context_line":"                    {\u0027path\u0027: path, \u0027value\u0027: value, \u0027exc\u0027: exc})"},{"line_number":128,"context_line":"            LOG.error(msg)"},{"line_number":129,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"839f761e_1eb0b07e","line":127,"in_reply_to":"81aa8b1b_c8b3ca0b","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":221,"context_line":"            driver_utils.add_node_capability(task, name, value)"},{"line_number":222,"context_line":"        except Exception as exc:"},{"line_number":223,"context_line":"            msg \u003d _(\"Failed to set capability %(name)s: %(exc)s\" %"},{"line_number":224,"context_line":"                    {\u0027name\u0027: name, \u0027exc\u0027: exc})"},{"line_number":225,"context_line":"            raise exception.RuleActionExecutionFailure("},{"line_number":226,"context_line":"                reason\u003dmsg)"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"8f8fc397_dd629db8","line":224,"updated":"2025-03-06 12:08:45.000000000","message":"Same issue with _()","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":221,"context_line":"            driver_utils.add_node_capability(task, name, value)"},{"line_number":222,"context_line":"        except Exception as exc:"},{"line_number":223,"context_line":"            msg \u003d _(\"Failed to set capability %(name)s: %(exc)s\" %"},{"line_number":224,"context_line":"                    {\u0027name\u0027: name, \u0027exc\u0027: exc})"},{"line_number":225,"context_line":"            raise exception.RuleActionExecutionFailure("},{"line_number":226,"context_line":"                reason\u003dmsg)"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"c0b7850d_186f0932","line":224,"in_reply_to":"8f8fc397_dd629db8","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":233,"context_line":"            driver_utils.remove_node_capability(task, name)"},{"line_number":234,"context_line":"        except Exception as exc:"},{"line_number":235,"context_line":"            msg \u003d _(\"Failed to unset capability %(name)s: %(exc)s\" %"},{"line_number":236,"context_line":"                    {\u0027name\u0027: name, \u0027exc\u0027: exc})"},{"line_number":237,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"feb05898_34f8bf40","line":236,"updated":"2025-03-06 12:08:45.000000000","message":"Same","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            driver_utils.remove_node_capability(task, name)"},{"line_number":234,"context_line":"        except Exception as exc:"},{"line_number":235,"context_line":"            msg \u003d _(\"Failed to unset capability %(name)s: %(exc)s\" %"},{"line_number":236,"context_line":"                    {\u0027name\u0027: name, \u0027exc\u0027: exc})"},{"line_number":237,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"16be0e3b_e7110840","line":236,"in_reply_to":"feb05898_34f8bf40","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":247,"context_line":"            update_nested_dict(plugin_data, path, value)"},{"line_number":248,"context_line":"        except Exception as exc:"},{"line_number":249,"context_line":"            msg \u003d _(\"Failed to set plugin data at %(path)s: %(exc)s\" % {"},{"line_number":250,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc})"},{"line_number":251,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3e9e738e_60e833d8","line":250,"updated":"2025-03-06 12:08:45.000000000","message":"Same","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            update_nested_dict(plugin_data, path, value)"},{"line_number":248,"context_line":"        except Exception as exc:"},{"line_number":249,"context_line":"            msg \u003d _(\"Failed to set plugin data at %(path)s: %(exc)s\" % {"},{"line_number":250,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc})"},{"line_number":251,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"00145c20_37ae1373","line":250,"in_reply_to":"3e9e738e_60e833d8","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":379,"context_line":"            msg \u003d _(\"Failed to extend attribute %(path)s for port \""},{"line_number":380,"context_line":"                    \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":381,"context_line":"                                               \u0027port_id\u0027: port_id,"},{"line_number":382,"context_line":"                                               \u0027exc\u0027: str(exc)}"},{"line_number":383,"context_line":"            LOG.error(msg)"},{"line_number":384,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"640ac725_4a097e50","line":382,"updated":"2025-03-06 12:08:45.000000000","message":"nit: unnecessary str() (implied by %s)","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":379,"context_line":"            msg \u003d _(\"Failed to extend attribute %(path)s for port \""},{"line_number":380,"context_line":"                    \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":381,"context_line":"                                               \u0027port_id\u0027: port_id,"},{"line_number":382,"context_line":"                                               \u0027exc\u0027: str(exc)}"},{"line_number":383,"context_line":"            LOG.error(msg)"},{"line_number":384,"context_line":"            raise exception.RuleActionExecutionFailure(reason\u003dmsg)"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"c034df09_748c97e8","line":382,"in_reply_to":"640ac725_4a097e50","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"}],"ironic/common/inspection_rules/base.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        parameters \u003d list(signature.parameters.values())[1:]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        required_args \u003d [p.name for p in parameters"},{"line_number":40,"context_line":"                         if p.default is inspect.Parameter.empty]"},{"line_number":41,"context_line":"        optional_args \u003d [p.name for p in parameters"},{"line_number":42,"context_line":"                         if p.default is not inspect.Parameter.empty]"},{"line_number":43,"context_line":"        return required_args, optional_args"}],"source_content_type":"text/x-python","patch_set":10,"id":"47ab77ce_ed206545","line":40,"updated":"2025-03-06 12:08:45.000000000","message":"Nice!","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":42,"context_line":"                         if p.default is not inspect.Parameter.empty]"},{"line_number":43,"context_line":"        return required_args, optional_args"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _normalize_list_args(self, **kwargs):"},{"line_number":46,"context_line":"        \"\"\"Convert list arguments into dictionary format.\"\"\""},{"line_number":47,"context_line":"        arg_list \u003d kwargs[\u0027args\u0027]"},{"line_number":48,"context_line":"        if not isinstance(arg_list, list):"}],"source_content_type":"text/x-python","patch_set":10,"id":"a7f10931_606f63ed","line":45,"updated":"2025-03-06 12:08:45.000000000","message":"By the way, you don\u0027t use the rest of kwargs, why not just pass args alone?","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":true,"context_lines":[{"line_number":42,"context_line":"                         if p.default is not inspect.Parameter.empty]"},{"line_number":43,"context_line":"        return required_args, optional_args"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _normalize_list_args(self, **kwargs):"},{"line_number":46,"context_line":"        \"\"\"Convert list arguments into dictionary format.\"\"\""},{"line_number":47,"context_line":"        arg_list \u003d kwargs[\u0027args\u0027]"},{"line_number":48,"context_line":"        if not isinstance(arg_list, list):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fc741dcc_28985674","line":45,"in_reply_to":"a7f10931_606f63ed","updated":"2025-03-06 19:22:01.000000000","message":"We also use \u0027required_args\u0027 and \u0027optional_args\u0027","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c1ca1c987b009493eb7527540a3376cfe6623232","unresolved":true,"context_lines":[{"line_number":42,"context_line":"                         if p.default is not inspect.Parameter.empty]"},{"line_number":43,"context_line":"        return required_args, optional_args"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _normalize_list_args(self, **kwargs):"},{"line_number":46,"context_line":"        \"\"\"Convert list arguments into dictionary format.\"\"\""},{"line_number":47,"context_line":"        arg_list \u003d kwargs[\u0027args\u0027]"},{"line_number":48,"context_line":"        if not isinstance(arg_list, list):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9e5eef3b_8299c220","line":45,"in_reply_to":"fc741dcc_28985674","updated":"2025-03-10 15:16:08.000000000","message":"Okay, then it makes sense to just pass them explicitly?","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        # Initialize required context fields if needed"},{"line_number":55,"context_line":"        if self.REQUIRES_PLUGIN_DATA:"},{"line_number":56,"context_line":"            arg_list.append(\u0027{}\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        required_args \u003d kwargs[\u0027required_args\u0027]"},{"line_number":59,"context_line":"        optional_args \u003d kwargs[\u0027optional_args\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"897bdc15_ee34a86b","line":56,"updated":"2025-03-06 12:08:45.000000000","message":"Shouldn\u0027t it be a dictionary, not a string? Probably does not matter for validation, but I spent a few minutes here trying to understand the value.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        # Initialize required context fields if needed"},{"line_number":55,"context_line":"        if self.REQUIRES_PLUGIN_DATA:"},{"line_number":56,"context_line":"            arg_list.append(\u0027{}\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        required_args \u003d kwargs[\u0027required_args\u0027]"},{"line_number":59,"context_line":"        optional_args \u003d kwargs[\u0027optional_args\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"2eaaeda0_e8b2c358","line":56,"in_reply_to":"897bdc15_ee34a86b","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            missing \u003d [p for p in required_args[len(arg_list):]]"},{"line_number":63,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":64,"context_line":"                   \", \".join(missing))"},{"line_number":65,"context_line":"            raise ValueError(msg)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        normalized_args \u003d {name: arg_list[i]"},{"line_number":68,"context_line":"                           for i, name in enumerate(required_args)}"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa952ad0_f5bb9949","line":65,"updated":"2025-03-06 12:08:45.000000000","message":"By the way, this is a user-visible error, right? So it should be something inherited from HardwareInspectionFailure maybe?","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            missing \u003d [p for p in required_args[len(arg_list):]]"},{"line_number":63,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":64,"context_line":"                   \", \".join(missing))"},{"line_number":65,"context_line":"            raise ValueError(msg)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        normalized_args \u003d {name: arg_list[i]"},{"line_number":68,"context_line":"                           for i, name in enumerate(required_args)}"}],"source_content_type":"text/x-python","patch_set":10,"id":"505dce7b_7eaa7202","line":65,"in_reply_to":"fa952ad0_f5bb9949","updated":"2025-03-06 19:22:01.000000000","message":"Yeah. Since this code runs in the API, perhaps an IronicException.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        # If after normalization attempt, we still do not have a dictionary,"},{"line_number":91,"context_line":"        # then it was never a list, so, not a supported type."},{"line_number":92,"context_line":"        if isinstance(normalized_args, dict):"},{"line_number":93,"context_line":"            provided \u003d set(normalized_args.keys())"},{"line_number":94,"context_line":"            missing \u003d set(required_args) - provided"},{"line_number":95,"context_line":"            unexpected \u003d provided - (set(required_args) | set(optional_args))"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"981283bb_948ef39a","line":93,"updated":"2025-03-06 12:08:45.000000000","message":"nit: .keys() redundant (iterating a dict yields keys already)","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        # If after normalization attempt, we still do not have a dictionary,"},{"line_number":91,"context_line":"        # then it was never a list, so, not a supported type."},{"line_number":92,"context_line":"        if isinstance(normalized_args, dict):"},{"line_number":93,"context_line":"            provided \u003d set(normalized_args.keys())"},{"line_number":94,"context_line":"            missing \u003d set(required_args) - provided"},{"line_number":95,"context_line":"            unexpected \u003d provided - (set(required_args) | set(optional_args))"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3855eece_ed6b2e99","line":93,"in_reply_to":"981283bb_948ef39a","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":104,"context_line":"            if msg:"},{"line_number":105,"context_line":"                raise ValueError(\u0027; \u0027.join(msg))"},{"line_number":106,"context_line":"        else:"},{"line_number":107,"context_line":"            raise ValueError(_(\"args must be either a list or dictionary\"))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def interpolate_variables(value, node, inventory, plugin_data):"},{"line_number":110,"context_line":"        if isinstance(value, str):"}],"source_content_type":"text/x-python","patch_set":10,"id":"d0db17d3_2e3a0260","line":107,"updated":"2025-03-06 12:08:45.000000000","message":"Similarly, these two will be visible to a user creating a rule, right? If so, should be some ironic exceptions.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            if msg:"},{"line_number":105,"context_line":"                raise ValueError(\u0027; \u0027.join(msg))"},{"line_number":106,"context_line":"        else:"},{"line_number":107,"context_line":"            raise ValueError(_(\"args must be either a list or dictionary\"))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def interpolate_variables(value, node, inventory, plugin_data):"},{"line_number":110,"context_line":"        if isinstance(value, str):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7701470b_247620b7","line":107,"in_reply_to":"d0db17d3_2e3a0260","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        op \u003d operation.get(\u0027op\u0027)"},{"line_number":136,"context_line":"        if not op:"},{"line_number":137,"context_line":"            raise ValueError(_(\"Operation must contain \u0027op\u0027 key\"))"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        required_args, optional_args \u003d self._get_validation_signature()"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"546a1813_cbbca9b3","line":137,"updated":"2025-03-06 12:08:45.000000000","message":"Similarly, let\u0027s inspect the usage of ValueError. Unless we catch them somewhere up the stack, they should be Ironic exceptions.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        op \u003d operation.get(\u0027op\u0027)"},{"line_number":136,"context_line":"        if not op:"},{"line_number":137,"context_line":"            raise ValueError(_(\"Operation must contain \u0027op\u0027 key\"))"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        required_args, optional_args \u003d self._get_validation_signature()"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ab76d753_a82b8ef3","line":137,"in_reply_to":"546a1813_cbbca9b3","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c1ca1c987b009493eb7527540a3376cfe6623232","unresolved":true,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        return normalized_args"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def validate(self, **kwargs):"},{"line_number":79,"context_line":"        \"\"\"Validate args passed during creation."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        Default implementation checks for presence of required fields."}],"source_content_type":"text/x-python","patch_set":11,"id":"8face804_7759d0cf","line":78,"updated":"2025-03-10 15:16:08.000000000","message":"Similarly here: I think you only accept \"args\" here","commit_id":"efe8f53c4a91678ad3b3a3c7fc1074ef44d480b9"}],"ironic/common/inspection_rules/engine.py":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"453b2752076b8e213e7e6ce23c34f53c3604e01a","unresolved":true,"context_lines":[{"line_number":27,"context_line":"SENSITIVE_FIELDS \u003d [\u0027password\u0027, \u0027auth_token\u0027, \u0027bmc_password\u0027]"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class _MaskProxyList(list):"},{"line_number":31,"context_line":"    def __init__(self, original_list, wrapped_items):"},{"line_number":32,"context_line":"        self._original \u003d original_list"},{"line_number":33,"context_line":"        self._wrapped \u003d wrapped_items"}],"source_content_type":"text/x-python","patch_set":9,"id":"f2821db0_ea289270","line":30,"updated":"2025-02-26 19:27:15.000000000","message":"Can we a docstring on this?\n\nLets make sure, since this is a security sensitive thing, that we document how this is expected to work. Might wanna specifically mention that we\u0027re only going to mask values out of key:value fields (effectively meaning we only mask dictionaries)","commit_id":"2cffa83a7c4d1e8e5bcd5c8bd23dd7817e49bcba"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":27,"context_line":"SENSITIVE_FIELDS \u003d [\u0027password\u0027, \u0027auth_token\u0027, \u0027bmc_password\u0027]"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class _MaskProxyList(list):"},{"line_number":31,"context_line":"    def __init__(self, original_list, wrapped_items):"},{"line_number":32,"context_line":"        self._original \u003d original_list"},{"line_number":33,"context_line":"        self._wrapped \u003d wrapped_items"}],"source_content_type":"text/x-python","patch_set":9,"id":"25dce182_d7a71636","line":30,"in_reply_to":"f2821db0_ea289270","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"2cffa83a7c4d1e8e5bcd5c8bd23dd7817e49bcba"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"453b2752076b8e213e7e6ce23c34f53c3604e01a","unresolved":true,"context_lines":[{"line_number":306,"context_line":"        except exception.HardwareInspectionFailure:"},{"line_number":307,"context_line":"            raise"},{"line_number":308,"context_line":"        except exception.IronicException as e:"},{"line_number":309,"context_line":"            if rule.get(\u0027sensitive\u0027, False):"},{"line_number":310,"context_line":"                LOG.error(\"Error applying sensitive rule %(rule)s to node \""},{"line_number":311,"context_line":"                          \"%(node)s\", {\u0027rule\u0027: rule[\u0027uuid\u0027],"},{"line_number":312,"context_line":"                                       \u0027node\u0027: node.uuid})"}],"source_content_type":"text/x-python","patch_set":9,"id":"228bcdbf_b49998e7","line":309,"updated":"2025-02-26 19:27:15.000000000","message":"The spec is ambiguous; it says errors for sensitive rules should be terse, but I interpret that to mean terse-to-the-API-caller, not necessarily terse in the logs; especially given we have last-minute code to keep secrets out of the log.\n\nI don\u0027t think you should change this based on my question, but wanted to highlight the behavior to ensure it was intended.","commit_id":"2cffa83a7c4d1e8e5bcd5c8bd23dd7817e49bcba"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":306,"context_line":"        except exception.HardwareInspectionFailure:"},{"line_number":307,"context_line":"            raise"},{"line_number":308,"context_line":"        except exception.IronicException as e:"},{"line_number":309,"context_line":"            if rule.get(\u0027sensitive\u0027, False):"},{"line_number":310,"context_line":"                LOG.error(\"Error applying sensitive rule %(rule)s to node \""},{"line_number":311,"context_line":"                          \"%(node)s\", {\u0027rule\u0027: rule[\u0027uuid\u0027],"},{"line_number":312,"context_line":"                                       \u0027node\u0027: node.uuid})"}],"source_content_type":"text/x-python","patch_set":9,"id":"85b4a6aa_ce5f7466","line":309,"in_reply_to":"228bcdbf_b49998e7","updated":"2025-03-06 12:08:45.000000000","message":"As the author, I agree: the logging can and probably should be full in all cases.","commit_id":"2cffa83a7c4d1e8e5bcd5c8bd23dd7817e49bcba"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        except exception.HardwareInspectionFailure:"},{"line_number":307,"context_line":"            raise"},{"line_number":308,"context_line":"        except exception.IronicException as e:"},{"line_number":309,"context_line":"            if rule.get(\u0027sensitive\u0027, False):"},{"line_number":310,"context_line":"                LOG.error(\"Error applying sensitive rule %(rule)s to node \""},{"line_number":311,"context_line":"                          \"%(node)s\", {\u0027rule\u0027: rule[\u0027uuid\u0027],"},{"line_number":312,"context_line":"                                       \u0027node\u0027: node.uuid})"}],"source_content_type":"text/x-python","patch_set":9,"id":"2918a612_46fb1109","line":309,"in_reply_to":"85b4a6aa_ce5f7466","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"2cffa83a7c4d1e8e5bcd5c8bd23dd7817e49bcba"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":27,"context_line":"SENSITIVE_FIELDS \u003d [\u0027password\u0027, \u0027auth_token\u0027, \u0027bmc_password\u0027]"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class _MaskProxyList(list):"},{"line_number":31,"context_line":"    \"\"\"A proxy list that maintains original and masked versions of list items."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    This is an implementation detail of ShallowMaskDict to handle lists that"}],"source_content_type":"text/x-python","patch_set":10,"id":"e9497818_27ba2ef4","line":30,"updated":"2025-03-06 12:08:45.000000000","message":"nit: these two classes could be under ironic.common.inspection_rules.utils (to keep the size and the scope of the file manageable)","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":27,"context_line":"SENSITIVE_FIELDS \u003d [\u0027password\u0027, \u0027auth_token\u0027, \u0027bmc_password\u0027]"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class _MaskProxyList(list):"},{"line_number":31,"context_line":"    \"\"\"A proxy list that maintains original and masked versions of list items."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    This is an implementation detail of ShallowMaskDict to handle lists that"}],"source_content_type":"text/x-python","patch_set":10,"id":"a0b25d63_214c6607","line":30,"in_reply_to":"e9497818_27ba2ef4","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":38,"context_line":"    When items are accessed from this proxy, they come from the wrapped list"},{"line_number":39,"context_line":"    (with masking applied). When items are modified, the changes are applied to"},{"line_number":40,"context_line":"    both lists, ensuring the original data structure is kept in sync."},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":"    def __init__(self, original_list, wrapped_items):"},{"line_number":43,"context_line":"        self._original \u003d original_list"},{"line_number":44,"context_line":"        self._wrapped \u003d wrapped_items"}],"source_content_type":"text/x-python","patch_set":10,"id":"1c954240_c9e284b9","line":41,"updated":"2025-03-06 12:08:45.000000000","message":"It could be less confusing to have a ShallowMaskList that works similarly to ShallowMaskDict for a list. Its `__getitem__` would return a ShallowMaskList for inner lists and ShallowMaskDict for inner dicts.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        return repr(self._wrapped)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class ShallowMaskDict:"},{"line_number":68,"context_line":"    \"\"\"Dictionary wrapper to mask sensitive fields on the fly"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    This class provides a secure way to handle dictionaries containing"}],"source_content_type":"text/x-python","patch_set":10,"id":"18ff7708_f82a50b1","line":67,"updated":"2025-03-06 12:08:45.000000000","message":"A useful pattern to create objects that behave similarly to standard types is to inherit them from something here https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes. This way, you need to implement fewer methods yourself.\n\nIn this case, it would be collections.abc.MutableMapping and for my speculated ShallowMaskList - MutableSequence.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"a095fc30090f75e514560e873911d5ea48aeaca3","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        return repr(self._wrapped)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class ShallowMaskDict:"},{"line_number":68,"context_line":"    \"\"\"Dictionary wrapper to mask sensitive fields on the fly"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    This class provides a secure way to handle dictionaries containing"}],"source_content_type":"text/x-python","patch_set":10,"id":"a9ee01e7_18a32241","line":67,"in_reply_to":"13ddcb71_245a55eb","updated":"2025-03-06 20:35:36.000000000","message":"Today I learned! It totally makes sense though.","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        return repr(self._wrapped)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class ShallowMaskDict:"},{"line_number":68,"context_line":"    \"\"\"Dictionary wrapper to mask sensitive fields on the fly"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    This class provides a secure way to handle dictionaries containing"}],"source_content_type":"text/x-python","patch_set":10,"id":"13ddcb71_245a55eb","line":67,"in_reply_to":"18ff7708_f82a50b1","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":true,"context_lines":[{"line_number":193,"context_line":"                break"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    rules.sort(key\u003dlambda rule: rule[\u0027priority\u0027], reverse\u003dTrue)"},{"line_number":196,"context_line":"    LOG.debug(\"Applying %(count)d inspection rules to node %(node)s\","},{"line_number":197,"context_line":"              {\u0027count\u0027: len(rules), \u0027node\u0027: node.uuid})"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    result \u003d {\u0027plugin_data\u0027: plugin_data}"}],"source_content_type":"text/x-python","patch_set":11,"id":"8e261373_05eeac47","side":"PARENT","line":196,"range":{"start_line":196,"start_character":0,"end_line":196,"end_character":69},"updated":"2025-03-06 19:22:01.000000000","message":"`H904: String interpolation should be delayed to be handled by the logging code, rather than being done at the point of the logging call. Use \u0027,\u0027 instead of \u0027%\u0027.`\n\nI hope this formatting is also supported by Ironic.","commit_id":"3603e60c328cd9dd522b0080adacc86a1ea396fc"}],"ironic/common/inspection_rules/operators.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cbfb93fc854b0ae4df0bf50cbdde77c2dd2cfeea","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        if not isinstance(values, list):"},{"line_number":116,"context_line":"            msg \u003d _(\"Failed to check condition: \u0027%(op)s\u0027 on values: \""},{"line_number":117,"context_line":"                    \"%(values)s: Expected list for \u0027values\u0027, got: \""},{"line_number":118,"context_line":"                    \"%(invalid_type)s\" % {"},{"line_number":119,"context_line":"                        \u0027op\u0027: self.op.__name__, \u0027values\u0027: values,"},{"line_number":120,"context_line":"                        \"invalid_type\": type(values).__name__})"},{"line_number":121,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"454b3a14_64f52cda","line":118,"updated":"2025-03-06 12:08:45.000000000","message":"Same problem with _() as in a few places in actions (formatting under _())","commit_id":"86891153a4ff521377d24610cdf351359b367482"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        if not isinstance(values, list):"},{"line_number":116,"context_line":"            msg \u003d _(\"Failed to check condition: \u0027%(op)s\u0027 on values: \""},{"line_number":117,"context_line":"                    \"%(values)s: Expected list for \u0027values\u0027, got: \""},{"line_number":118,"context_line":"                    \"%(invalid_type)s\" % {"},{"line_number":119,"context_line":"                        \u0027op\u0027: self.op.__name__, \u0027values\u0027: values,"},{"line_number":120,"context_line":"                        \"invalid_type\": type(values).__name__})"},{"line_number":121,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"50e8252e_eeaf48cb","line":118,"in_reply_to":"454b3a14_64f52cda","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"86891153a4ff521377d24610cdf351359b367482"}],"ironic/common/inspection_rules/utils.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"05c227d4ec9404f539733829a3eea22dc55a828e","unresolved":true,"context_lines":[{"line_number":1,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":2,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":3,"context_line":"# Copyright 2011 Justin Santa Barbara"},{"line_number":4,"context_line":"# Copyright (c) 2012 NTT DOCOMO, INC."},{"line_number":5,"context_line":"# All Rights Reserved."},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":8,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":8,"id":"0cad3469_a78ea41f","line":5,"range":{"start_line":1,"start_character":0,"end_line":5,"end_character":22},"updated":"2025-02-26 14:35:11.000000000","message":"Did this entire file come with this header?","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":2,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":3,"context_line":"# Copyright 2011 Justin Santa Barbara"},{"line_number":4,"context_line":"# Copyright (c) 2012 NTT DOCOMO, INC."},{"line_number":5,"context_line":"# All Rights Reserved."},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":8,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":8,"id":"a41b6352_ecd6793d","line":5,"range":{"start_line":1,"start_character":0,"end_line":5,"end_character":22},"in_reply_to":"0cad3469_a78ea41f","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def __iter__(self):"},{"line_number":60,"context_line":"        for item in self._original:"},{"line_number":61,"context_line":"            yield self._mask_value(item)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __len__(self):"},{"line_number":64,"context_line":"        return len(self._original)"}],"source_content_type":"text/x-python","patch_set":13,"id":"23622bac_8514951f","line":61,"updated":"2025-03-11 14:39:05.000000000","message":"nit: you don\u0027t need to implement __iter__ - it\u0027s included automatically per https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"}],"ironic/common/inspection_rules/validation.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"05c227d4ec9404f539733829a3eea22dc55a828e","unresolved":true,"context_lines":[{"line_number":1,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":2,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":3,"context_line":"# Copyright 2011 Justin Santa Barbara"},{"line_number":4,"context_line":"# Copyright (c) 2012 NTT DOCOMO, INC."},{"line_number":5,"context_line":"# All Rights Reserved."},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":8,"id":"0b08eb4d_61448afc","line":4,"range":{"start_line":1,"start_character":0,"end_line":4,"end_character":37},"updated":"2025-02-26 14:35:11.000000000","message":"Wait a second, where did this code originally come from? This should only be stamped on to a file if it legtimiately has heritage tracing back to the exceptionally early days of OpenStack.\n\nAt this point, it seems unlikely to be appropriate to stamp this copyright on to files. Further background would be greatly appreciated. Thanks!","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"},{"author":{"_account_id":36770,"name":"cid","display_name":"cid","email":"cid@gr-oss.io","username":"cidelight","status":"@gr-oss upstream: Doing good IRONIC things..."},"change_message_id":"cace4453e75455ddd165d91141de6dedb3288ef9","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":2,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":3,"context_line":"# Copyright 2011 Justin Santa Barbara"},{"line_number":4,"context_line":"# Copyright (c) 2012 NTT DOCOMO, INC."},{"line_number":5,"context_line":"# All Rights Reserved."},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":8,"id":"2b5a5523_423ec1cf","line":4,"range":{"start_line":1,"start_character":0,"end_line":4,"end_character":37},"in_reply_to":"0b08eb4d_61448afc","updated":"2025-03-06 19:22:01.000000000","message":"Acknowledged","commit_id":"7ce41c2d8846f4a55705f9201a343fc10e50beb6"}],"ironic/tests/unit/common/test_inspection_rule.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":23,"context_line":"from ironic.tests.unit.objects import utils as obj_utils"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class TestApplyRules(db_base.DbTestCase):"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(TestApplyRules, self).setUp()"}],"source_content_type":"text/x-python","patch_set":13,"id":"01de4fae_7d06ad21","line":26,"updated":"2025-03-11 14:39:05.000000000","message":"nit: tests should be grouped into several test classes based on what they\u0027re testing.","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        self.sensitive_rule \u003d obj_utils.create_test_inspection_rule("},{"line_number":67,"context_line":"            self.context, sensitive\u003dTrue)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_set_mask_enabled(self):"},{"line_number":70,"context_line":"        \"\"\"Test that set_mask_enabled properly toggles masking.\"\"\""},{"line_number":71,"context_line":"        masked_dict \u003d utils.ShallowMaskDict("},{"line_number":72,"context_line":"            self.test_data, sensitive_fields\u003dself.sensitive_fields,"}],"source_content_type":"text/x-python","patch_set":13,"id":"ac27bc72_4a5caa75","line":69,"updated":"2025-03-11 14:39:05.000000000","message":"(cont) e.g. you could have a separate TestShallowMask","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":178,"context_line":"    @mock.patch.object(engine, \u0027get_built_in_rules\u0027, autospec\u003dTrue)"},{"line_number":179,"context_line":"    @mock.patch.object(engine, \u0027check_conditions\u0027, autospec\u003dTrue)"},{"line_number":180,"context_line":"    @mock.patch.object(engine, \u0027apply_actions\u0027, autospec\u003dTrue)"},{"line_number":181,"context_line":"    @mock.patch(\u0027ironic.objects.InspectionRule.list\u0027, autospec\u003dTrue)"},{"line_number":182,"context_line":"    def test_apply_rules_no_rules(self, mock_list, mock_apply_actions,"},{"line_number":183,"context_line":"                                  mock_check_conditions, mock_get_built_in):"},{"line_number":184,"context_line":"        mock_list.return_value \u003d []"}],"source_content_type":"text/x-python","patch_set":13,"id":"89e19b6f_e8cad388","line":181,"updated":"2025-03-11 14:39:05.000000000","message":"nit: if you did not have masking tests in the same test class, you could move these 4 @mock invocations to the class level since they are the same for all test methods.","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":220,"context_line":"        mock_get_built_in.assert_called_once()"},{"line_number":221,"context_line":"        mock_list.assert_called_once_with("},{"line_number":222,"context_line":"            context\u003dself.context,"},{"line_number":223,"context_line":"            filters\u003d{\u0027phase\u0027: \u0027main\u0027})"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    @mock.patch.object(engine, \u0027get_built_in_rules\u0027, autospec\u003dTrue)"},{"line_number":226,"context_line":"    @mock.patch.object(engine, \u0027check_conditions\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f459b22_417f4533","line":223,"updated":"2025-03-11 14:39:05.000000000","message":"maybe check that apply_actions has been called","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4c8cc37efea50493b4b658ba2132ca9b77916705","unresolved":true,"context_lines":[{"line_number":238,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":239,"context_line":"            engine.apply_rules(task, self.inventory, self.plugin_data, \u0027main\u0027)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self.assertEqual(2, mock_check_conditions.call_count)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    @mock.patch.object(engine, \u0027LOG\u0027, autospec\u003dTrue)"},{"line_number":244,"context_line":"    @mock.patch.object(engine, \u0027get_built_in_rules\u0027, autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"45799ba9_e12ebf44","line":241,"updated":"2025-03-11 14:39:05.000000000","message":"... and here that it has not been called?","commit_id":"de5988af29f3a7bba028f3cdf6bfb77b1622419f"}]}