)]}'
{"/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":"1a1e8017a38c5f7c3ff80166db8c50049247ce01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"d45abfa7_ea43cb8c","updated":"2025-02-07 22:27:51.000000000","message":"Overall, lgtm. Nothing stands out to me as problematic.","commit_id":"8a4091f7eec29075048ce8bfc438a26824996537"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"e8e448f5_65c6ddd3","updated":"2025-02-17 12:44:49.000000000","message":"Sorry, late to the party. A few quick comments on actions (will check the rest later). I\u0027d also prefer this change to come with unit tests.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"53f70426e24e00dc51519d6381c3a74b0ef5f227","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"cc4689f1_9cee53f5","updated":"2025-02-18 15:46:03.000000000","message":"Thank you for your work, and sorry for being late with the review; I\u0027ve left a few comments","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"375ae984c3bba10bef62ab24f525335a9416abde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"b5ec87ab_d604741c","updated":"2025-02-14 21:22:48.000000000","message":"recheck","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"}],"ironic/common/inspection_rules/actions.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                                            plugin_data)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        arg_values \u003d [processed_args[arg_name]"},{"line_number":92,"context_line":"                      for arg_name in self.get_arg_names()]"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        for optional_arg in self.OPTIONAL_ARGS:"},{"line_number":95,"context_line":"            arg_values.append(processed_args.get(optional_arg, False))"}],"source_content_type":"text/x-python","patch_set":21,"id":"1f7ab153_2ca5c155","line":92,"updated":"2025-02-17 12:44:49.000000000","message":"Do you expect complex logic in get_arg_names? I wonder what is preventing us from having REQUIRED_ARGS and OPTIONAL_ARGS on the class level.\n\nIn fact, you could use used Python\u0027s https://docs.python.org/3/library/inspect.html#inspect.signature to get the arguments without declaring them. Maybe it\u0027s a good idea to switch to that in the default implementation of get_arg_names before we call the API stable.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":104,"context_line":"    def get_arg_names(cls):"},{"line_number":105,"context_line":"        return [\u0027msg\u0027]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __call__(self, task, msg, level\u003d\u0027info\u0027):"},{"line_number":108,"context_line":"        getattr(LOG, level)(msg)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"8c804cc6_18351e9d","line":107,"updated":"2025-02-17 12:44:49.000000000","message":"It would be good to have a means to verify that the level is acceptable.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    def get_arg_names(cls):"},{"line_number":105,"context_line":"        return [\u0027msg\u0027]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __call__(self, task, msg, level\u003d\u0027info\u0027):"},{"line_number":108,"context_line":"        getattr(LOG, level)(msg)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"93e7864c_1f8ff3e3","line":107,"in_reply_to":"8c804cc6_18351e9d","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        return [\u0027msg\u0027]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def __call__(self, task, msg):"},{"line_number":118,"context_line":"        msg \u003d _(\u0027%(msg)s\u0027) % {\u0027msg\u0027: msg}"},{"line_number":119,"context_line":"        raise exception.HardwareInspectionFailure(error\u003dmsg)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"f6cbcf46_283e0f86","line":118,"updated":"2025-02-17 12:44:49.000000000","message":"nit: this is just str(msg)","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return [\u0027msg\u0027]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def __call__(self, task, msg):"},{"line_number":118,"context_line":"        msg \u003d _(\u0027%(msg)s\u0027) % {\u0027msg\u0027: msg}"},{"line_number":119,"context_line":"        raise exception.HardwareInspectionFailure(error\u003dmsg)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"080d2146_4167834e","line":118,"in_reply_to":"f6cbcf46_283e0f86","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":140,"context_line":"                setattr(task.node, attr_path_parts[0], base_attr)"},{"line_number":141,"context_line":"            task.node.save()"},{"line_number":142,"context_line":"        except Exception as exc:"},{"line_number":143,"context_line":"            msg \u003d (\"Failed to set attribute %(path)s \""},{"line_number":144,"context_line":"                   \"with value %(value)s: %(exc)s\" %"},{"line_number":145,"context_line":"                   {\u0027path\u0027: path, \u0027value\u0027: value, \u0027exc\u0027: exc})"},{"line_number":146,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"705ffbe2_2a1da41d","line":143,"updated":"2025-02-17 12:44:49.000000000","message":"nit: missing _() around a user-visible message","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                setattr(task.node, attr_path_parts[0], base_attr)"},{"line_number":141,"context_line":"            task.node.save()"},{"line_number":142,"context_line":"        except Exception as exc:"},{"line_number":143,"context_line":"            msg \u003d (\"Failed to set attribute %(path)s \""},{"line_number":144,"context_line":"                   \"with value %(value)s: %(exc)s\" %"},{"line_number":145,"context_line":"                   {\u0027path\u0027: path, \u0027value\u0027: value, \u0027exc\u0027: exc})"},{"line_number":146,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"19c829b8_6039c6ee","line":143,"in_reply_to":"705ffbe2_2a1da41d","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":144,"context_line":"                   \"with value %(value)s: %(exc)s\" %"},{"line_number":145,"context_line":"                   {\u0027path\u0027: path, \u0027value\u0027: value, \u0027exc\u0027: exc})"},{"line_number":146,"context_line":"            LOG.error(msg)"},{"line_number":147,"context_line":"            raise exception.InvalidParameterValue(msg)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class ExtendAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"763fea0c_e1828b65","line":147,"updated":"2025-02-17 12:44:49.000000000","message":"This is not necessarily an InvalidParameterValue, I\u0027d introduce a new class for failed actions.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                    current \u003d current.setdefault(part, {})"},{"line_number":169,"context_line":"                current \u003d current.setdefault(attr_path_parts[-1], [])"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            if not isinstance(current, list):"},{"line_number":172,"context_line":"                current \u003d []"},{"line_number":173,"context_line":"            if not unique or value not in current:"},{"line_number":174,"context_line":"                current.append(value)"}],"source_content_type":"text/x-python","patch_set":21,"id":"28628544_d3c38796","line":171,"updated":"2025-02-17 12:44:49.000000000","message":"We should not silently override a value that is not a list. If it\u0027s None - should, but if it\u0027s a string or something else, we should rather error out.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":181,"context_line":"        except Exception as exc:"},{"line_number":182,"context_line":"            msg \u003d (\"Failed to extend attribute %(path)s: %(exc)s\") % {"},{"line_number":183,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc}"},{"line_number":184,"context_line":"            raise exception.InvalidParameterValue(msg)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"class DelAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"70ec12ab_c563ff0f","line":184,"updated":"2025-02-17 12:44:49.000000000","message":"ditto about _() and exception class","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":206,"context_line":"        except Exception as exc:"},{"line_number":207,"context_line":"            msg \u003d (\"Failed to delete attribute at %(path)s: %(exc)s\") % {"},{"line_number":208,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc}"},{"line_number":209,"context_line":"            raise exception.InvalidParameterValue(msg)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"class AddTraitAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"b446ef78_27bba79b","line":209,"updated":"2025-02-17 12:44:49.000000000","message":"ditto (and everywhere below)","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def __call__(self, task, name, value):"},{"line_number":256,"context_line":"        try:"},{"line_number":257,"context_line":"            properties \u003d task.node.properties.copy()"},{"line_number":258,"context_line":"            capabilities \u003d properties.get(\u0027capabilities\u0027, \u0027\u0027)"},{"line_number":259,"context_line":"            caps \u003d dict(cap.split(\u0027:\u0027, 1)"},{"line_number":260,"context_line":"                        for cap in capabilities.split(\u0027,\u0027) if cap)"}],"source_content_type":"text/x-python","patch_set":21,"id":"68ab5b63_6557ae6d","line":257,"updated":"2025-02-17 12:44:49.000000000","message":"nit: please use functions from ironic.drivers.utils to manipulate capabilities","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def __call__(self, task, name, value):"},{"line_number":256,"context_line":"        try:"},{"line_number":257,"context_line":"            properties \u003d task.node.properties.copy()"},{"line_number":258,"context_line":"            capabilities \u003d properties.get(\u0027capabilities\u0027, \u0027\u0027)"},{"line_number":259,"context_line":"            caps \u003d dict(cap.split(\u0027:\u0027, 1)"},{"line_number":260,"context_line":"                        for cap in capabilities.split(\u0027,\u0027) if cap)"}],"source_content_type":"text/x-python","patch_set":21,"id":"90a4f08c_f435dcf9","line":257,"in_reply_to":"68ab5b63_6557ae6d","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def __call__(self, task, name):"},{"line_number":278,"context_line":"        try:"},{"line_number":279,"context_line":"            properties \u003d task.node.properties.copy()"},{"line_number":280,"context_line":"            capabilities \u003d properties.get(\u0027capabilities\u0027, \u0027\u0027)"},{"line_number":281,"context_line":"            caps \u003d dict(cap.split(\u0027:\u0027, 1)"},{"line_number":282,"context_line":"                        for cap in capabilities.split(\u0027,\u0027) if cap)"}],"source_content_type":"text/x-python","patch_set":21,"id":"4a515d58_82f55ac9","line":279,"updated":"2025-02-17 12:44:49.000000000","message":"Same","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":true,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def __call__(self, task, name):"},{"line_number":278,"context_line":"        try:"},{"line_number":279,"context_line":"            properties \u003d task.node.properties.copy()"},{"line_number":280,"context_line":"            capabilities \u003d properties.get(\u0027capabilities\u0027, \u0027\u0027)"},{"line_number":281,"context_line":"            caps \u003d dict(cap.split(\u0027:\u0027, 1)"},{"line_number":282,"context_line":"                        for cap in capabilities.split(\u0027,\u0027) if cap)"}],"source_content_type":"text/x-python","patch_set":21,"id":"69ef60d4_d8f15653","line":279,"in_reply_to":"4a515d58_82f55ac9","updated":"2025-02-18 21:38:15.000000000","message":"I don\u0027t think there\u0027s a function to remove a capability in ironic.drivers.utils","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2a53393c4ce93f2a7875a4ffabce0c29d7f9847f","unresolved":true,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def __call__(self, task, name):"},{"line_number":278,"context_line":"        try:"},{"line_number":279,"context_line":"            properties \u003d task.node.properties.copy()"},{"line_number":280,"context_line":"            capabilities \u003d properties.get(\u0027capabilities\u0027, \u0027\u0027)"},{"line_number":281,"context_line":"            caps \u003d dict(cap.split(\u0027:\u0027, 1)"},{"line_number":282,"context_line":"                        for cap in capabilities.split(\u0027,\u0027) if cap)"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ff1a45d_ca488e78","line":279,"in_reply_to":"69ef60d4_d8f15653","updated":"2025-02-20 16:14:25.000000000","message":"Indeed. It\u0027s better to add it there for the general reuse.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":302,"context_line":"    def __call__(self, task, path, value, plugin_data):"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            update_nested_dict(plugin_data, path, value)"},{"line_number":305,"context_line":"            return {\u0027plugin_data\u0027: plugin_data}"},{"line_number":306,"context_line":"        except Exception as exc:"},{"line_number":307,"context_line":"            msg \u003d (\"Failed to set plugin data at %(path)s: %(exc)s\" % {"},{"line_number":308,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":21,"id":"a11aa0e3_3c74dd30","line":305,"updated":"2025-02-17 12:44:49.000000000","message":"I don\u0027t think actions return anything?","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2a53393c4ce93f2a7875a4ffabce0c29d7f9847f","unresolved":true,"context_lines":[{"line_number":302,"context_line":"    def __call__(self, task, path, value, plugin_data):"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            update_nested_dict(plugin_data, path, value)"},{"line_number":305,"context_line":"            return {\u0027plugin_data\u0027: plugin_data}"},{"line_number":306,"context_line":"        except Exception as exc:"},{"line_number":307,"context_line":"            msg \u003d (\"Failed to set plugin data at %(path)s: %(exc)s\" % {"},{"line_number":308,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":21,"id":"31232cb5_891d3ab0","line":305,"in_reply_to":"8e3bc2c8_0b026114","updated":"2025-02-20 16:14:25.000000000","message":"You\u0027re modifying the dictionary already, no? Why do you also need to return it? I\u0027m quite sure we don\u0027t pass a copy around.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":true,"context_lines":[{"line_number":302,"context_line":"    def __call__(self, task, path, value, plugin_data):"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            update_nested_dict(plugin_data, path, value)"},{"line_number":305,"context_line":"            return {\u0027plugin_data\u0027: plugin_data}"},{"line_number":306,"context_line":"        except Exception as exc:"},{"line_number":307,"context_line":"            msg \u003d (\"Failed to set plugin data at %(path)s: %(exc)s\" % {"},{"line_number":308,"context_line":"                \u0027path\u0027: path, \u0027exc\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":21,"id":"8e3bc2c8_0b026114","line":305,"in_reply_to":"a11aa0e3_3c74dd30","updated":"2025-02-18 21:38:15.000000000","message":"When we modify the plugin_data, returning that copy to the `continue_inspection` function to possibly pass around and eventually persist it, is the only way I figured to always have the updated copy of the plugin_data (a.k.a the \u003caction word\u003ePluginDataAction actions to have worked).\n\nI\u0027m open to ideas!","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":325,"context_line":"                current \u003d []"},{"line_number":326,"context_line":"                update_nested_dict(plugin_data, path, current)"},{"line_number":327,"context_line":"            elif not isinstance(current, list):"},{"line_number":328,"context_line":"                current \u003d []"},{"line_number":329,"context_line":"                update_nested_dict(plugin_data, path, current)"},{"line_number":330,"context_line":"            if not unique or value not in current:"},{"line_number":331,"context_line":"                current.append(value)"}],"source_content_type":"text/x-python","patch_set":21,"id":"48202d0f_aae87df8","line":328,"updated":"2025-02-17 12:44:49.000000000","message":"This block is the same as lines 325-226, the conditions can be merged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":413,"context_line":"            msg \u003d (\"Failed to set attribute %(path)s for port \""},{"line_number":414,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":415,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":416,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":417,"context_line":"            LOG.warning(msg)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"3592ea82_7f6c96a3","line":416,"updated":"2025-02-17 12:44:49.000000000","message":"nit: str() implied by %s","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":413,"context_line":"            msg \u003d (\"Failed to set attribute %(path)s for port \""},{"line_number":414,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":415,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":416,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":417,"context_line":"            LOG.warning(msg)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"045dd311_f32ced4a","line":416,"in_reply_to":"3592ea82_7f6c96a3","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":414,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":415,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":416,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":417,"context_line":"            LOG.warning(msg)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"class ExtendPortAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"f3f2a26b_9af997a3","line":417,"updated":"2025-02-17 12:44:49.000000000","message":"The normal set-attribute raises an exception but this one only logs. I think it should also raise an error on a failure.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":415,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":416,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":417,"context_line":"            LOG.warning(msg)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"class ExtendPortAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"a4aa5658_53f2c5d5","line":417,"in_reply_to":"f3f2a26b_9af997a3","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":455,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":456,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":457,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":458,"context_line":"            LOG.warning(msg)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"class DelPortAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"871f5940_dbc1271d","line":458,"updated":"2025-02-17 12:44:49.000000000","message":"same","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":455,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":456,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":457,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":458,"context_line":"            LOG.warning(msg)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"class DelPortAttributeAction(ActionBase):"}],"source_content_type":"text/x-python","patch_set":21,"id":"50ed1361_2c5d2dc6","line":458,"in_reply_to":"871f5940_dbc1271d","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43564e73005b9327915a7bf7159298da5d221c3f","unresolved":true,"context_lines":[{"line_number":485,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":486,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":487,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":488,"context_line":"            LOG.warning(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"22a07d6b_f533773c","line":488,"updated":"2025-02-17 12:44:49.000000000","message":"same","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                   \"%(port_id)s: %(exc)s\") % {\u0027path\u0027: path,"},{"line_number":486,"context_line":"                                              \u0027port_id\u0027: port_id,"},{"line_number":487,"context_line":"                                              \u0027exc\u0027: str(exc)}"},{"line_number":488,"context_line":"            LOG.warning(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"8d41c5bc_9cbcf409","line":488,"in_reply_to":"22a07d6b_f533773c","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"}],"ironic/common/inspection_rules/base.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    @classmethod"},{"line_number":39,"context_line":"    @abc.abstractmethod"},{"line_number":40,"context_line":"    def get_arg_names(cls):"},{"line_number":41,"context_line":"        \"\"\"Return list of argument names in order expected.\"\"\""},{"line_number":42,"context_line":"        raise NotImplementedError"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"f2d44ab4_2b93dcc3","line":40,"updated":"2025-02-17 17:32:12.000000000","message":"See the previous file about get_arg_names","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":55,"context_line":"        # plugin_data is a required argument during validation but since"},{"line_number":56,"context_line":"        # it comes from the inspection data and added later, we need to"},{"line_number":57,"context_line":"        # make sure validation does not fail for that sake."},{"line_number":58,"context_line":"        if \u0027plugin-data\u0027 in op_name:"},{"line_number":59,"context_line":"            arg_list.append(\u0027{}\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        arg_names \u003d set(self.__class__.get_arg_names())"}],"source_content_type":"text/x-python","patch_set":21,"id":"cebf0a99_30a1cfcd","line":58,"updated":"2025-02-17 17:32:12.000000000","message":"Please don\u0027t hardcode specific operations or arguments like that, it will be extremely hard to remember how things work in a few months. Maybe we just need to make inventory a requirement and always present argument? It was not in the spec, I believe, but requiring this logic seems to justify adding it now.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":59,"context_line":"            arg_list.append(\u0027{}\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        arg_names \u003d set(self.__class__.get_arg_names())"},{"line_number":62,"context_line":"        if len(arg_list) \u003c len(arg_names):"},{"line_number":63,"context_line":"            missing \u003d arg_names[len(arg_list):]"},{"line_number":64,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":65,"context_line":"                   \", \".join(missing))"}],"source_content_type":"text/x-python","patch_set":21,"id":"2824adae_edb448c7","line":62,"updated":"2025-02-17 17:32:12.000000000","message":"If you go down the path of using inspect.Signature, you can use its bind() method to validate the arguments instead of the custom logic.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"53f70426e24e00dc51519d6381c3a74b0ef5f227","unresolved":true,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        arg_names \u003d set(self.__class__.get_arg_names())"},{"line_number":62,"context_line":"        if len(arg_list) \u003c len(arg_names):"},{"line_number":63,"context_line":"            missing \u003d arg_names[len(arg_list):]"},{"line_number":64,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":65,"context_line":"                   \", \".join(missing))"},{"line_number":66,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"0b4c3b0b_57d630e2","line":63,"updated":"2025-02-18 15:46:03.000000000","message":"Sets are not subscriptable in python, I think?","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":true,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        arg_names \u003d set(self.__class__.get_arg_names())"},{"line_number":62,"context_line":"        if len(arg_list) \u003c len(arg_names):"},{"line_number":63,"context_line":"            missing \u003d arg_names[len(arg_list):]"},{"line_number":64,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":65,"context_line":"                   \", \".join(missing))"},{"line_number":66,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":21,"id":"95e31118_f6bd8c0a","line":63,"in_reply_to":"0b4c3b0b_57d630e2","updated":"2025-02-18 21:38:15.000000000","message":"Good catch. That\u0027s no longer the case in the new follow-up patch.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":63,"context_line":"            missing \u003d arg_names[len(arg_list):]"},{"line_number":64,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":65,"context_line":"                   \", \".join(missing))"},{"line_number":66,"context_line":"            LOG.error(msg)"},{"line_number":67,"context_line":"            raise ValueError(msg)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        arg_list \u003d {name: arg_list[i] for i, name in enumerate(arg_names)}"}],"source_content_type":"text/x-python","patch_set":21,"id":"4f8a6600_24f47053","line":66,"updated":"2025-02-17 17:32:12.000000000","message":"It\u0027s unfortunate that we don\u0027t have a Node object in this function: log messages without a node UUID will be tricky to interpret. Maybe raise here but leave logging up to a higher level code?","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            missing \u003d arg_names[len(arg_list):]"},{"line_number":64,"context_line":"            msg \u003d (_(\"Not enough arguments provided. Missing: %s\"),"},{"line_number":65,"context_line":"                   \", \".join(missing))"},{"line_number":66,"context_line":"            LOG.error(msg)"},{"line_number":67,"context_line":"            raise ValueError(msg)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        arg_list \u003d {name: arg_list[i] for i, name in enumerate(arg_names)}"}],"source_content_type":"text/x-python","patch_set":21,"id":"44b1c517_a860d1ed","line":66,"in_reply_to":"4f8a6600_24f47053","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # Add optional args if they exist in the input"},{"line_number":72,"context_line":"        start_idx \u003d len(arg_names)"},{"line_number":73,"context_line":"        for i, opt_arg in enumerate(self.OPTIONAL_ARGS):"},{"line_number":74,"context_line":"            if start_idx + i \u003c len(arg_list):"},{"line_number":75,"context_line":"                arg_list[opt_arg] \u003d arg_list[start_idx + i]"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"01bcc6ad_786a786e","line":73,"updated":"2025-02-17 17:32:12.000000000","message":"Similarly, this can be done much more elegantly using inspect.Signature.bind","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":82,"context_line":"        Default implementation checks for presence of required fields."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :param args: args as a dictionary"},{"line_number":85,"context_line":"        :param kwargs: used for extensibility without breaking existing plugins"},{"line_number":86,"context_line":"        :raises: ValueError on validation failure"},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        required_args \u003d set(self.__class__.get_arg_names())"}],"source_content_type":"text/x-python","patch_set":21,"id":"41b0dbba_906f5029","line":85,"updated":"2025-02-17 17:32:12.000000000","message":"This was the case for Inspector, but it\u0027s not the case here: you\u0027re using kwargs to pass \u0027args\u0027 and \u0027op\u0027 and don\u0027t seem to use args at all. Could you clean up?","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"53f70426e24e00dc51519d6381c3a74b0ef5f227","unresolved":true,"context_lines":[{"line_number":82,"context_line":"        Default implementation checks for presence of required fields."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :param args: args as a dictionary"},{"line_number":85,"context_line":"        :param kwargs: used for extensibility without breaking existing plugins"},{"line_number":86,"context_line":"        :raises: ValueError on validation failure"},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        required_args \u003d set(self.__class__.get_arg_names())"}],"source_content_type":"text/x-python","patch_set":21,"id":"97ed3968_a45e90c4","line":85,"in_reply_to":"41b0dbba_906f5029","updated":"2025-02-18 15:46:03.000000000","message":"Similar for the above _normalize_list_args function","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        Default implementation checks for presence of required fields."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :param args: args as a dictionary"},{"line_number":85,"context_line":"        :param kwargs: used for extensibility without breaking existing plugins"},{"line_number":86,"context_line":"        :raises: ValueError on validation failure"},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        required_args \u003d set(self.__class__.get_arg_names())"}],"source_content_type":"text/x-python","patch_set":21,"id":"226f83f9_a800bf69","line":85,"in_reply_to":"97ed3968_a45e90c4","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"53f70426e24e00dc51519d6381c3a74b0ef5f227","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                           % \u0027, \u0027.join(unexpected))"},{"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":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":21,"id":"fdc8e944_48a8a1a0","line":106,"updated":"2025-02-18 15:46:03.000000000","message":"This ValueError will be raised even if normalized_args is a list","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                           % \u0027, \u0027.join(unexpected))"},{"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":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":21,"id":"264d53fe_e9900e12","line":106,"in_reply_to":"fdc8e944_48a8a1a0","updated":"2025-02-18 21:38:15.000000000","message":"Well, at this point, it should already be normalized (converted to a dict), so if it\u0027s still not a dict at this point, it must be of an unsupported type.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"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":"    @staticmethod"},{"line_number":110,"context_line":"    def interpolate_variables(value, node, inventory, plugin_data):"},{"line_number":111,"context_line":"        if isinstance(value, str):"},{"line_number":112,"context_line":"            try:"},{"line_number":113,"context_line":"                return value.format(node\u003dnode, inventory\u003dinventory,"}],"source_content_type":"text/x-python","patch_set":21,"id":"0c3c1d49_967b72b1","line":110,"updated":"2025-02-17 17:32:12.000000000","message":"Static methods are rarely better than just normal functions, this one can definitely be a function.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"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":"    @staticmethod"},{"line_number":110,"context_line":"    def interpolate_variables(value, node, inventory, plugin_data):"},{"line_number":111,"context_line":"        if isinstance(value, str):"},{"line_number":112,"context_line":"            try:"},{"line_number":113,"context_line":"                return value.format(node\u003dnode, inventory\u003dinventory,"}],"source_content_type":"text/x-python","patch_set":21,"id":"eb21494d_4e894811","line":110,"in_reply_to":"0c3c1d49_967b72b1","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        op \u003d operation.get(\u0027op\u0027)"},{"line_number":137,"context_line":"        if not op:"},{"line_number":138,"context_line":"            raise ValueError(\"Operation must contain \u0027op\u0027 key\")"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        op, invtd \u003d common_utils.parse_inverted_operator(op)"},{"line_number":141,"context_line":"        dict_args \u003d self._normalize_list_args(args\u003doperation.get(\u0027args\u0027, {}),"}],"source_content_type":"text/x-python","patch_set":21,"id":"4937a631_66e0e495","line":138,"updated":"2025-02-17 17:32:12.000000000","message":"Is this message supposed to be user-visible? If so, you need _()","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # plugin-data becomes available during inspection,"},{"line_number":145,"context_line":"        # we need to populate with the actual value."},{"line_number":146,"context_line":"        if \u0027plugin_data\u0027 in dict_args or \u0027plugin-data\u0027 in op:"},{"line_number":147,"context_line":"            dict_args[\u0027plugin_data\u0027] \u003d plugin_data"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":21,"id":"c5a78732_ec3ee6d1","line":146,"updated":"2025-02-17 17:32:12.000000000","message":"See above","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"}],"ironic/common/inspection_rules/engine.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":104,"context_line":"                             \u0027op\u0027: op, \u0027supported_ops\u0027: supported_ops})"},{"line_number":105,"context_line":"                raise ValueError(msg)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            result \u003d False"},{"line_number":108,"context_line":"            plugin \u003d operators.get_operator(op)"},{"line_number":109,"context_line":"            if \u0027loop\u0027 in condition:"},{"line_number":110,"context_line":"                result \u003d plugin()._check_with_loop(task, condition, inventory,"}],"source_content_type":"text/x-python","patch_set":21,"id":"f9576941_7df6e888","line":107,"updated":"2025-02-17 17:32:12.000000000","message":"nit: redundant instruction","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                             \u0027op\u0027: op, \u0027supported_ops\u0027: supported_ops})"},{"line_number":105,"context_line":"                raise ValueError(msg)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            result \u003d False"},{"line_number":108,"context_line":"            plugin \u003d operators.get_operator(op)"},{"line_number":109,"context_line":"            if \u0027loop\u0027 in condition:"},{"line_number":110,"context_line":"                result \u003d plugin()._check_with_loop(task, condition, inventory,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5cece433_b3581ad6","line":107,"in_reply_to":"f9576941_7df6e888","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"68cfcacfd3c5be19dcaa4866202a3c1634d2d5a9","unresolved":true,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"            plugin \u003d actions.get_action(op)"},{"line_number":143,"context_line":"            if \u0027loop\u0027 in action:"},{"line_number":144,"context_line":"                action_result \u003d plugin()._execute_with_loop("},{"line_number":145,"context_line":"                    task, action, inventory, result[\u0027plugin_data\u0027])"},{"line_number":146,"context_line":"            else:"},{"line_number":147,"context_line":"                action_result \u003d plugin()._execute_action("}],"source_content_type":"text/x-python","patch_set":21,"id":"bdbf6a86_7947ec49","line":144,"updated":"2025-02-24 18:36:46.000000000","message":"Methods starting with _ are private by convention and should not be used from outside the class.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"68cfcacfd3c5be19dcaa4866202a3c1634d2d5a9","unresolved":true,"context_lines":[{"line_number":144,"context_line":"                action_result \u003d plugin()._execute_with_loop("},{"line_number":145,"context_line":"                    task, action, inventory, result[\u0027plugin_data\u0027])"},{"line_number":146,"context_line":"            else:"},{"line_number":147,"context_line":"                action_result \u003d plugin()._execute_action("},{"line_number":148,"context_line":"                    task, action, inventory, result[\u0027plugin_data\u0027])"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            if action_result is not None and isinstance(action_result, dict):"}],"source_content_type":"text/x-python","patch_set":21,"id":"ea66988d_ecd94809","line":147,"updated":"2025-02-24 18:36:46.000000000","message":"same","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"53f70426e24e00dc51519d6381c3a74b0ef5f227","unresolved":true,"context_lines":[{"line_number":185,"context_line":"        inventory \u003d _mask_sensitive_data(inventory)"},{"line_number":186,"context_line":"        plugin_data \u003d _mask_sensitive_data(plugin_data)"},{"line_number":187,"context_line":"    elif mask_secrets \u003d\u003d \u0027sensitive\u0027:"},{"line_number":188,"context_line":"        # Mask secrets unless the rule is marked as sensitive"},{"line_number":189,"context_line":"        for rule in rules:"},{"line_number":190,"context_line":"            if not rule.get(\u0027sensitive\u0027, False):"},{"line_number":191,"context_line":"                inventory \u003d _mask_sensitive_data(inventory)"}],"source_content_type":"text/x-python","patch_set":21,"id":"a9b0de8c_9b87960b","line":188,"updated":"2025-02-18 15:46:03.000000000","message":"Mask secrets if the rule is marked as sensitive (not unless)","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":true,"context_lines":[{"line_number":185,"context_line":"        inventory \u003d _mask_sensitive_data(inventory)"},{"line_number":186,"context_line":"        plugin_data \u003d _mask_sensitive_data(plugin_data)"},{"line_number":187,"context_line":"    elif mask_secrets \u003d\u003d \u0027sensitive\u0027:"},{"line_number":188,"context_line":"        # Mask secrets unless the rule is marked as sensitive"},{"line_number":189,"context_line":"        for rule in rules:"},{"line_number":190,"context_line":"            if not rule.get(\u0027sensitive\u0027, False):"},{"line_number":191,"context_line":"                inventory \u003d _mask_sensitive_data(inventory)"}],"source_content_type":"text/x-python","patch_set":21,"id":"bf315c94_cb4adc20","line":188,"in_reply_to":"a9b0de8c_9b87960b","updated":"2025-02-18 21:38:15.000000000","message":"This logic is according to the spec: https://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/inspection-rules.html#:~:text\u003dallow%20secrets%20for%20rules%20marked%20as%20sensitive \n\nUnless I misinterpreted that :D.","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"159b973698b46dad42451e2b31951a90edaea2da","unresolved":true,"context_lines":[{"line_number":185,"context_line":"        inventory \u003d _mask_sensitive_data(inventory)"},{"line_number":186,"context_line":"        plugin_data \u003d _mask_sensitive_data(plugin_data)"},{"line_number":187,"context_line":"    elif mask_secrets \u003d\u003d \u0027sensitive\u0027:"},{"line_number":188,"context_line":"        # Mask secrets unless the rule is marked as sensitive"},{"line_number":189,"context_line":"        for rule in rules:"},{"line_number":190,"context_line":"            if not rule.get(\u0027sensitive\u0027, False):"},{"line_number":191,"context_line":"                inventory \u003d _mask_sensitive_data(inventory)"}],"source_content_type":"text/x-python","patch_set":21,"id":"91acc6c1_5c54cb8b","line":188,"in_reply_to":"bf315c94_cb4adc20","updated":"2025-02-19 11:08:31.000000000","message":"The logic is correct, its the comment I\u0027m referring to :)","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"}],"ironic/common/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":"dc9a88c57b8f735c3c7e477e57bc8bf7885fc499","unresolved":true,"context_lines":[{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"def parse_inverted_operator(op):"},{"line_number":1153,"context_line":"    \"\"\"Handle inverted operators.\"\"\""},{"line_number":1154,"context_line":"    op \u003d op.strip()"},{"line_number":1155,"context_line":"    if op.count(\u0027!\u0027) \u003e 1:"},{"line_number":1156,"context_line":"        msg \u003d _(\"Multiple exclamation marks are not allowed. \""}],"source_content_type":"text/x-python","patch_set":17,"id":"80060960_4a1304f4","line":1153,"range":{"start_line":1153,"start_character":33,"end_line":1153,"end_character":34},"updated":"2025-01-29 15:31:27.000000000","message":"Please enhance this docstring so internally we know more why this exists.","commit_id":"1695341e93a3a91c2b8facf465fb394255360beb"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"def parse_inverted_operator(op):"},{"line_number":1153,"context_line":"    \"\"\"Handle inverted operators.\"\"\""},{"line_number":1154,"context_line":"    op \u003d op.strip()"},{"line_number":1155,"context_line":"    if op.count(\u0027!\u0027) \u003e 1:"},{"line_number":1156,"context_line":"        msg \u003d _(\"Multiple exclamation marks are not allowed. \""}],"source_content_type":"text/x-python","patch_set":17,"id":"3b9ca0c4_402e2f51","line":1153,"range":{"start_line":1153,"start_character":33,"end_line":1153,"end_character":34},"in_reply_to":"80060960_4a1304f4","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"1695341e93a3a91c2b8facf465fb394255360beb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"94a7bb9c0aae3508c9e2dc20e8daa70e6455dae3","unresolved":true,"context_lines":[{"line_number":1149,"context_line":"                  {\u0027dest\u0027: dest, \u0027rec\u0027: out})"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"def parse_inverted_operator(op):"},{"line_number":1153,"context_line":"    \"\"\"Handle inverted operators.\"\"\""},{"line_number":1154,"context_line":"    op \u003d op.strip()"},{"line_number":1155,"context_line":"    if op.count(\u0027!\u0027) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":21,"id":"7bf6eb2a_67fd05f5","line":1152,"updated":"2025-02-17 17:32:12.000000000","message":"This is specific to inspection rules, I\u0027d keep it somewhere under ironic.common.inspection_rules","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"},{"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":"fbe2816d9f379b7e7e900124bdc4717ad3ef7430","unresolved":false,"context_lines":[{"line_number":1149,"context_line":"                  {\u0027dest\u0027: dest, \u0027rec\u0027: out})"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"def parse_inverted_operator(op):"},{"line_number":1153,"context_line":"    \"\"\"Handle inverted operators.\"\"\""},{"line_number":1154,"context_line":"    op \u003d op.strip()"},{"line_number":1155,"context_line":"    if op.count(\u0027!\u0027) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":21,"id":"60cc603b_9fb0e948","line":1152,"in_reply_to":"7bf6eb2a_67fd05f5","updated":"2025-02-18 21:38:15.000000000","message":"Acknowledged","commit_id":"15df33437bd4e9888f46d9b453adf03fb7c59ab6"}]}