)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"5df56e71ed546a1d631421f0beb7fab7dcaa6895","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"693f3eff_c22abefc","updated":"2022-01-12 08:55:33.000000000","message":"recheck","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"8e14080c51a44c017433fdb73191b84760b2caac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a6dfbcc9_0ac4c634","updated":"2022-01-11 16:36:35.000000000","message":"recheck","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"74b9497d_6be66176","updated":"2022-01-13 09:12:40.000000000","message":"thanks for the review, comments inline.","commit_id":"287f58c7df4704b0ab07349597d57db5c1a7a987"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"cc60fc6b64ebe7598cdd86850d5bb1edcd0012fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"34976746_48ccc5ca","updated":"2022-01-13 16:42:50.000000000","message":"recheck","commit_id":"b824ea7fa8874e63cfe11bc82ce0dc049680344f"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"fb3b6e65ea7ba8dd80f94ad1674674bc1ebc8356","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a3570c23_2b6db32c","updated":"2022-01-13 12:35:25.000000000","message":"recheck","commit_id":"b824ea7fa8874e63cfe11bc82ce0dc049680344f"}],"ironic/drivers/modules/redfish/firmware_utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"_UPDATE_FIRMWARE_FIELDS \u003d [\u0027url\u0027, \u0027wait\u0027]"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def validate_update_firmware_args(firmware_images):"}],"source_content_type":"text/x-python","patch_set":3,"id":"07e7fea9_9d9aadae","line":21,"updated":"2022-01-12 11:53:31.000000000","message":"can be a frozenset - see below","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"_UPDATE_FIRMWARE_FIELDS \u003d [\u0027url\u0027, \u0027wait\u0027]"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def validate_update_firmware_args(firmware_images):"}],"source_content_type":"text/x-python","patch_set":3,"id":"c10fd5e1_7584cdba","line":21,"in_reply_to":"07e7fea9_9d9aadae","updated":"2022-01-13 09:12:40.000000000","message":"N/A","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    if not isinstance(firmware_images, list):"},{"line_number":47,"context_line":"        msg \u003d (_(\"update_firmware argument %(firmware_images)s is not a \""},{"line_number":48,"context_line":"                 \"list.\") % {\u0027firmware_images\u0027: firmware_images})"},{"line_number":49,"context_line":"        LOG.error(msg)"},{"line_number":50,"context_line":"        raise exception.InvalidParameterValue(msg)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    errors \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"d8a41944_475ca3f4","line":49,"updated":"2022-01-12 11:53:31.000000000","message":"When logging anything, we should include a node UUID. or leave the logging up to the calling code?\n\nGiven that the result of the validation is immediately returned as a result of the clean step, I\u0027m not convinced the logging here is needed at all.","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    if not isinstance(firmware_images, list):"},{"line_number":47,"context_line":"        msg \u003d (_(\"update_firmware argument %(firmware_images)s is not a \""},{"line_number":48,"context_line":"                 \"list.\") % {\u0027firmware_images\u0027: firmware_images})"},{"line_number":49,"context_line":"        LOG.error(msg)"},{"line_number":50,"context_line":"        raise exception.InvalidParameterValue(msg)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    errors \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"ae2a14f2_0eafc308","line":49,"in_reply_to":"d8a41944_475ca3f4","updated":"2022-01-13 09:12:40.000000000","message":"agree, removed","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    errors \u003d []"},{"line_number":53,"context_line":"    for item in firmware_images:"},{"line_number":54,"context_line":"        for key in item.keys():"},{"line_number":55,"context_line":"            # Check that all keys are known fields"},{"line_number":56,"context_line":"            if key not in _UPDATE_FIRMWARE_FIELDS:"},{"line_number":57,"context_line":"                errors.append(_(\"Field \u0027%(key)s\u0027 not expected in %(item)s. \""}],"source_content_type":"text/x-python","patch_set":3,"id":"3554c486_242ab8e4","line":54,"updated":"2022-01-12 11:53:31.000000000","message":"nit: keys() not needed","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    errors \u003d []"},{"line_number":53,"context_line":"    for item in firmware_images:"},{"line_number":54,"context_line":"        for key in item.keys():"},{"line_number":55,"context_line":"            # Check that all keys are known fields"},{"line_number":56,"context_line":"            if key not in _UPDATE_FIRMWARE_FIELDS:"},{"line_number":57,"context_line":"                errors.append(_(\"Field \u0027%(key)s\u0027 not expected in %(item)s. \""}],"source_content_type":"text/x-python","patch_set":3,"id":"e87a86a4_9d18ccbb","line":54,"in_reply_to":"3554c486_242ab8e4","updated":"2022-01-13 09:12:40.000000000","message":"N/A","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    for item in firmware_images:"},{"line_number":54,"context_line":"        for key in item.keys():"},{"line_number":55,"context_line":"            # Check that all keys are known fields"},{"line_number":56,"context_line":"            if key not in _UPDATE_FIRMWARE_FIELDS:"},{"line_number":57,"context_line":"                errors.append(_(\"Field \u0027%(key)s\u0027 not expected in %(item)s. \""},{"line_number":58,"context_line":"                              \"Known fields are: %(fields)s\") %"},{"line_number":59,"context_line":"                              {\u0027key\u0027: key, \u0027item\u0027: item,"}],"source_content_type":"text/x-python","patch_set":3,"id":"77b6f804_8f8e67fb","line":56,"updated":"2022-01-12 11:53:31.000000000","message":"nit: you could do\n\n unexpected \u003d set(item) - _UPDATE_FIRMWARE_FIELDS","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    for item in firmware_images:"},{"line_number":54,"context_line":"        for key in item.keys():"},{"line_number":55,"context_line":"            # Check that all keys are known fields"},{"line_number":56,"context_line":"            if key not in _UPDATE_FIRMWARE_FIELDS:"},{"line_number":57,"context_line":"                errors.append(_(\"Field \u0027%(key)s\u0027 not expected in %(item)s. \""},{"line_number":58,"context_line":"                              \"Known fields are: %(fields)s\") %"},{"line_number":59,"context_line":"                              {\u0027key\u0027: key, \u0027item\u0027: item,"}],"source_content_type":"text/x-python","patch_set":3,"id":"88974ecc_f9fa4ef0","line":56,"in_reply_to":"77b6f804_8f8e67fb","updated":"2022-01-13 09:12:40.000000000","message":"N/A","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                              {\u0027key\u0027: key, \u0027item\u0027: item,"},{"line_number":60,"context_line":"                               \u0027fields\u0027: \", \".join(_UPDATE_FIRMWARE_FIELDS)})"},{"line_number":61,"context_line":"        # Check that mandatory fields provided"},{"line_number":62,"context_line":"        if item.get(\u0027url\u0027) is None:"},{"line_number":63,"context_line":"            errors.append(_(\"Item %(item)s is missing mandatory field \u0027url\u0027\") %"},{"line_number":64,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":65,"context_line":"        # Check data types"}],"source_content_type":"text/x-python","patch_set":3,"id":"f7f83809_aef5e822","line":62,"updated":"2022-01-12 11:53:31.000000000","message":"Looks like you could use JSON schema instead of this function. Have you considered it?","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                              {\u0027key\u0027: key, \u0027item\u0027: item,"},{"line_number":60,"context_line":"                               \u0027fields\u0027: \", \".join(_UPDATE_FIRMWARE_FIELDS)})"},{"line_number":61,"context_line":"        # Check that mandatory fields provided"},{"line_number":62,"context_line":"        if item.get(\u0027url\u0027) is None:"},{"line_number":63,"context_line":"            errors.append(_(\"Item %(item)s is missing mandatory field \u0027url\u0027\") %"},{"line_number":64,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":65,"context_line":"        # Check data types"}],"source_content_type":"text/x-python","patch_set":3,"id":"d2552933_fe02db67","line":62,"in_reply_to":"f7f83809_aef5e822","updated":"2022-01-13 09:12:40.000000000","message":"yes, I did, but for some reason gravitated towards this as ilo firmware did this way. Anyway, switched to JSON schema as this was unnecessary boilerplate. I think, in longer term should have this as part of argsinfo validation, currently, support for more complex args as this, is lacking.","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            errors.append(_(\"Item %(item)s does not have string for \u0027url\u0027\") %"},{"line_number":68,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if item.get(\u0027wait\u0027) and not isinstance(item.get(\u0027wait\u0027), int):"},{"line_number":71,"context_line":"            errors.append(_(\"Item %(item)s does not have integer for \u0027wait\u0027\") %"},{"line_number":72,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"75c9ecea_36c2c35b","line":70,"updated":"2022-01-12 11:53:31.000000000","message":"nit: this will allow \"\" and False to pass","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            errors.append(_(\"Item %(item)s does not have string for \u0027url\u0027\") %"},{"line_number":68,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if item.get(\u0027wait\u0027) and not isinstance(item.get(\u0027wait\u0027), int):"},{"line_number":71,"context_line":"            errors.append(_(\"Item %(item)s does not have integer for \u0027wait\u0027\") %"},{"line_number":72,"context_line":"                          {\u0027item\u0027: item})"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"aa455a11_aecbba52","line":70,"in_reply_to":"75c9ecea_36c2c35b","updated":"2022-01-13 09:12:40.000000000","message":"should be fixed with switch to schema","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    if errors:"},{"line_number":75,"context_line":"        msg \u003d (_(\"update_firmware argument is not valid. Errors: %(errors)s\") %"},{"line_number":76,"context_line":"               {\u0027errors\u0027: \", \".join(errors)})"},{"line_number":77,"context_line":"        LOG.error(msg)"},{"line_number":78,"context_line":"        raise exception.InvalidParameterValue(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c9179363_b1c2088b","line":77,"updated":"2022-01-12 11:53:31.000000000","message":"same","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    if errors:"},{"line_number":75,"context_line":"        msg \u003d (_(\"update_firmware argument is not valid. Errors: %(errors)s\") %"},{"line_number":76,"context_line":"               {\u0027errors\u0027: \", \".join(errors)})"},{"line_number":77,"context_line":"        LOG.error(msg)"},{"line_number":78,"context_line":"        raise exception.InvalidParameterValue(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1d6d462b_f33e65af","line":77,"in_reply_to":"c9179363_b1c2088b","updated":"2022-01-13 09:12:40.000000000","message":"done","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"}],"ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6640cd0c555c817eac28a012814d4cee663f8810","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class FirmwareUtilsTestCase(base.TestCase):"},{"line_number":20,"context_line":"    def setUp(self):"},{"line_number":21,"context_line":"        super(FirmwareUtilsTestCase, self).setUp()"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def test_validate_update_firmware_args(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"c39ccbb1_9c44078c","line":20,"updated":"2022-01-12 11:53:31.000000000","message":"nit: redundant setUp","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0b0f6cea88ba3a8690508932d09271376927169c","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class FirmwareUtilsTestCase(base.TestCase):"},{"line_number":20,"context_line":"    def setUp(self):"},{"line_number":21,"context_line":"        super(FirmwareUtilsTestCase, self).setUp()"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def test_validate_update_firmware_args(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"cab288e8_fbef3de6","line":20,"in_reply_to":"c39ccbb1_9c44078c","updated":"2022-01-13 09:12:40.000000000","message":"done","commit_id":"a142305d50252a2460b313d21e1d4cf95a3741a4"}]}
