)]}'
{"driver-requirements.txt":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":11,"context_line":"python-xclarityclient\u003e\u003d0.1.6"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# The Redfish hardware type uses the Sushy library"},{"line_number":14,"context_line":"sushy\u003e\u003d3.6.0"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Ansible-deploy interface"},{"line_number":17,"context_line":"ansible\u003e\u003d2.7"}],"source_content_type":"text/plain","patch_set":12,"id":"d7bdba8a_e5659a06","line":14,"updated":"2021-02-02 14:38:13.000000000","message":"Will need to be changed to 3.6.1 as far as I understand","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":11,"context_line":"python-xclarityclient\u003e\u003d0.1.6"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# The Redfish hardware type uses the Sushy library"},{"line_number":14,"context_line":"sushy\u003e\u003d3.6.0"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Ansible-deploy interface"},{"line_number":17,"context_line":"ansible\u003e\u003d2.7"}],"source_content_type":"text/plain","patch_set":12,"id":"f29f71c5_bd36d945","line":14,"in_reply_to":"d7bdba8a_e5659a06","updated":"2021-02-05 19:39:10.000000000","message":"Correct. 3.6.1 is published and I\u0027ll update this in the next patch set.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"682def467c97e8487a9af68d4931a228f753d0f5","unresolved":true,"context_lines":[{"line_number":11,"context_line":"python-xclarityclient\u003e\u003d0.1.6"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# The Redfish hardware type uses the Sushy library"},{"line_number":14,"context_line":"sushy\u003e\u003d3.6.1"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Ansible-deploy interface"},{"line_number":17,"context_line":"ansible\u003e\u003d2.7"}],"source_content_type":"text/plain","patch_set":16,"id":"88a09c9a_381763db","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":12},"updated":"2021-02-23 18:03:00.000000000","message":"todo: will need to update this once task monitor change is released","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"bc13d82fd1900987fff20990c99d2b8a48e864b0","unresolved":true,"context_lines":[{"line_number":11,"context_line":"python-xclarityclient\u003e\u003d0.1.6"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# The Redfish hardware type uses the Sushy library"},{"line_number":14,"context_line":"sushy\u003e\u003d3.6.1"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Ansible-deploy interface"},{"line_number":17,"context_line":"ansible\u003e\u003d2.7"}],"source_content_type":"text/plain","patch_set":16,"id":"72f9d4b5_576e0260","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":12},"in_reply_to":"88a09c9a_381763db","updated":"2021-02-23 22:10:11.000000000","message":"Ack","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"}],"ironic/conf/redfish.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":"bc38f3a135fd035ed9e0bbc4891497e52613ebea","unresolved":true,"context_lines":[{"line_number":99,"context_line":"               min\u003d0,"},{"line_number":100,"context_line":"               default\u003d60,"},{"line_number":101,"context_line":"               help\u003d_(\u0027Number of seconds to wait between checking for \u0027"},{"line_number":102,"context_line":"                      \u0027failed raid config tasks\u0027)),"},{"line_number":103,"context_line":"]"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9d5ea993_a6aaa463","line":102,"updated":"2021-02-08 13:25:51.000000000","message":"Nit: It might be good to indicate hw long it will take for the job to fail, or figure out how to fin that out.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"}],"ironic/drivers/modules/redfish/raid.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":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import math"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log"},{"line_number":18,"context_line":"from oslo_utils import importutils"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_b22f0da6","line":15,"updated":"2020-08-13 20:44:21.000000000","message":"For some reason, I\u0027m finding this amusing.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import math"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log"},{"line_number":18,"context_line":"from oslo_utils import importutils"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_61abae88","line":15,"in_reply_to":"9f560f44_b22f0da6","updated":"2020-08-19 23:29:44.000000000","message":"You know this is going to be an advanced module. :-)","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        \u0027raid_type\u0027: \u0027RAID6\u0027,"},{"line_number":61,"context_line":"        \u0027overhead\u0027: 2"},{"line_number":62,"context_line":"    },"},{"line_number":63,"context_line":"    \u00271+0\u0027: {"},{"line_number":64,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":65,"context_line":"        \u0027volume_type\u0027: \u0027SpannedMirrors\u0027,"},{"line_number":66,"context_line":"        \u0027raid_type\u0027: \u0027RAID10\u0027,"},{"line_number":67,"context_line":"        \u0027span_type\u0027: \u00271\u0027"},{"line_number":68,"context_line":"    },"},{"line_number":69,"context_line":"    \u00275+0\u0027: {"},{"line_number":70,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":71,"context_line":"        \u0027volume_type\u0027: \u0027SpannedStripesWithParity\u0027,"},{"line_number":72,"context_line":"        \u0027raid_type\u0027: \u0027RAID50\u0027,"},{"line_number":73,"context_line":"        \u0027span_type\u0027: \u00275\u0027"},{"line_number":74,"context_line":"    },"},{"line_number":75,"context_line":"    \u00276+0\u0027: {"},{"line_number":76,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":77,"context_line":"        \u0027volume_type\u0027: \u0027SpannedStripesWithParity\u0027,"},{"line_number":78,"context_line":"        \u0027raid_type\u0027: \u0027RAID60\u0027,"},{"line_number":79,"context_line":"        \u0027span_type\u0027: \u00276\u0027"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_125b5954","line":79,"range":{"start_line":63,"start_character":0,"end_line":79,"end_character":24},"updated":"2020-08-13 20:44:21.000000000","message":"I\u0027m just wondering if there should be an overhead here. I\u0027m guessing not given the type is spanned.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        \u0027raid_type\u0027: \u0027RAID6\u0027,"},{"line_number":61,"context_line":"        \u0027overhead\u0027: 2"},{"line_number":62,"context_line":"    },"},{"line_number":63,"context_line":"    \u00271+0\u0027: {"},{"line_number":64,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":65,"context_line":"        \u0027volume_type\u0027: \u0027SpannedMirrors\u0027,"},{"line_number":66,"context_line":"        \u0027raid_type\u0027: \u0027RAID10\u0027,"},{"line_number":67,"context_line":"        \u0027span_type\u0027: \u00271\u0027"},{"line_number":68,"context_line":"    },"},{"line_number":69,"context_line":"    \u00275+0\u0027: {"},{"line_number":70,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":71,"context_line":"        \u0027volume_type\u0027: \u0027SpannedStripesWithParity\u0027,"},{"line_number":72,"context_line":"        \u0027raid_type\u0027: \u0027RAID50\u0027,"},{"line_number":73,"context_line":"        \u0027span_type\u0027: \u00275\u0027"},{"line_number":74,"context_line":"    },"},{"line_number":75,"context_line":"    \u00276+0\u0027: {"},{"line_number":76,"context_line":"        \u0027type\u0027: \u0027spanned\u0027,"},{"line_number":77,"context_line":"        \u0027volume_type\u0027: \u0027SpannedStripesWithParity\u0027,"},{"line_number":78,"context_line":"        \u0027raid_type\u0027: \u0027RAID60\u0027,"},{"line_number":79,"context_line":"        \u0027span_type\u0027: \u00276\u0027"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_615c4e7e","line":79,"range":{"start_line":63,"start_character":0,"end_line":79,"end_character":24},"in_reply_to":"9f560f44_125b5954","updated":"2020-08-19 23:29:44.000000000","message":"Correct. For the spanned types (take, for example, type \u00276+0\u0027), when the logic needs to calculate the overhead, it uses the overhead from the corresponding span_type entry (RAID type \u00276\u0027 in this case). See _raid_level_overhead() around L#146-152.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    \"\"\"Convert size in logical_disks from gb to bytes\"\"\""},{"line_number":88,"context_line":"    for disk in logical_disks:"},{"line_number":89,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027 and \u0027physical_disks\u0027 not in disk:"},{"line_number":90,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":91,"context_line":"                _(\"create_configuration called with invalid \""},{"line_number":92,"context_line":"                  \"target_raid_configuration for node %(node_id)s. \""},{"line_number":93,"context_line":"                  \"\u0027physical_disks\u0027 is missing from logical_disk while \""},{"line_number":94,"context_line":"                  \"\u0027size_gb\u0027\u003d\u0027MAX\u0027 was requested: \""},{"line_number":95,"context_line":"                  \"%(logical_disk)s\") % {\u0027node_id\u0027: node.uuid,"},{"line_number":96,"context_line":"                                         \u0027logical_disk\u0027: disk})"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027:"},{"line_number":99,"context_line":"            disk[\u0027size_bytes\u0027] \u003d \u0027MAX\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_b526d7b9","line":96,"range":{"start_line":90,"start_character":0,"end_line":96,"end_character":63},"updated":"2020-08-13 20:44:21.000000000","message":"I guess the only thing that comes to mind here is that this is not strictly required.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    \"\"\"Convert size in logical_disks from gb to bytes\"\"\""},{"line_number":88,"context_line":"    for disk in logical_disks:"},{"line_number":89,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027 and \u0027physical_disks\u0027 not in disk:"},{"line_number":90,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":91,"context_line":"                _(\"create_configuration called with invalid \""},{"line_number":92,"context_line":"                  \"target_raid_configuration for node %(node_id)s. \""},{"line_number":93,"context_line":"                  \"\u0027physical_disks\u0027 is missing from logical_disk while \""},{"line_number":94,"context_line":"                  \"\u0027size_gb\u0027\u003d\u0027MAX\u0027 was requested: \""},{"line_number":95,"context_line":"                  \"%(logical_disk)s\") % {\u0027node_id\u0027: node.uuid,"},{"line_number":96,"context_line":"                                         \u0027logical_disk\u0027: disk})"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027:"},{"line_number":99,"context_line":"            disk[\u0027size_bytes\u0027] \u003d \u0027MAX\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_e1597e5a","line":96,"range":{"start_line":90,"start_character":0,"end_line":96,"end_character":63},"in_reply_to":"9f560f44_b526d7b9","updated":"2020-08-19 23:29:44.000000000","message":"Noted. The code here is largely based on the drac driver on the assumption that the logic should be very similar. I\u0027ll give some thought to removing this restriction. Perhaps in a follow-up?","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    :returns: a list of Drive objects from sushy"},{"line_number":111,"context_line":"    :raises: RedfishConnectionError when it fails to connect to Redfish"},{"line_number":112,"context_line":"    :raises: RedfishError if the System is not registered in Redfish"},{"line_number":113,"context_line":"    :raises: SushyError if there is an error getting the drives via sushy"},{"line_number":114,"context_line":"    \"\"\""},{"line_number":115,"context_line":"    system \u003d redfish_utils.get_system(node)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    disks \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_75beffff","line":114,"range":{"start_line":113,"start_character":13,"end_line":114,"end_character":7},"updated":"2020-08-13 20:44:21.000000000","message":"We likely shouldn\u0027t be raising the helper library exception to the conductor.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    :returns: a list of Drive objects from sushy"},{"line_number":111,"context_line":"    :raises: RedfishConnectionError when it fails to connect to Redfish"},{"line_number":112,"context_line":"    :raises: RedfishError if the System is not registered in Redfish"},{"line_number":113,"context_line":"    :raises: SushyError if there is an error getting the drives via sushy"},{"line_number":114,"context_line":"    \"\"\""},{"line_number":115,"context_line":"    system \u003d redfish_utils.get_system(node)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    disks \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_64573c04","line":114,"range":{"start_line":113,"start_character":13,"end_line":114,"end_character":7},"in_reply_to":"9f560f44_75beffff","updated":"2020-08-19 23:29:44.000000000","message":"Good catch. Yes, will raise RedfishError here instead.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    return disks, disk_to_controller"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"},{"line_number":137,"context_line":"    try:"},{"line_number":138,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":139,"context_line":"    except KeyError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_353ac7cc","line":136,"updated":"2020-08-13 20:44:21.000000000","message":"nit: would be good if there was a quick docstring, but not required.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    return disks, disk_to_controller"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"},{"line_number":137,"context_line":"    try:"},{"line_number":138,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":139,"context_line":"    except KeyError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_6cc0ac2b","line":136,"in_reply_to":"9f560f44_353ac7cc","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    try:"},{"line_number":138,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":139,"context_line":"    except KeyError:"},{"line_number":140,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":141,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":142,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":143,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":144,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":147,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_d5a8abdc","line":144,"range":{"start_line":140,"start_character":0,"end_line":144,"end_character":50},"updated":"2020-08-13 20:44:21.000000000","message":"nit: helper?","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    try:"},{"line_number":138,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":139,"context_line":"    except KeyError:"},{"line_number":140,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":141,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":142,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":143,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":144,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":147,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ccdc581a","line":144,"range":{"start_line":140,"start_character":0,"end_line":144,"end_character":50},"in_reply_to":"9f560f44_d5a8abdc","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":273,"context_line":"    try:"},{"line_number":274,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":275,"context_line":"    except KeyError:"},{"line_number":276,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":277,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":278,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":279,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":280,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":283,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_35ada7ea","line":280,"range":{"start_line":276,"start_character":0,"end_line":280,"end_character":50},"updated":"2020-08-13 20:44:21.000000000","message":"nit: helper?","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":273,"context_line":"    try:"},{"line_number":274,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":275,"context_line":"    except KeyError:"},{"line_number":276,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":277,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":278,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":279,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":280,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":283,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_acdf640a","line":280,"range":{"start_line":276,"start_character":0,"end_line":280,"end_character":50},"in_reply_to":"9f560f44_35ada7ea","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":294,"context_line":"    try:"},{"line_number":295,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":296,"context_line":"    except KeyError:"},{"line_number":297,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":298,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":299,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":300,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":301,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":304,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_15aa23d1","line":301,"range":{"start_line":297,"start_character":0,"end_line":301,"end_character":50},"updated":"2020-08-13 20:44:21.000000000","message":"nit: helper?","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":294,"context_line":"    try:"},{"line_number":295,"context_line":"        raid_level_info \u003d RAID_LEVELS[raid_level]"},{"line_number":296,"context_line":"    except KeyError:"},{"line_number":297,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":298,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":299,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":300,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":301,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    if raid_level_info[\u0027type\u0027] \u003d\u003d \u0027spanned\u0027:"},{"line_number":304,"context_line":"        if spans_count \u003c\u003d 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0ce770e6","line":301,"range":{"start_line":297,"start_character":0,"end_line":301,"end_character":50},"in_reply_to":"9f560f44_15aa23d1","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":326,"context_line":"        # largest even number less than disk_count"},{"line_number":327,"context_line":"        return (disks_count \u003e\u003e 1) \u003c\u003c 1"},{"line_number":328,"context_line":"    else:"},{"line_number":329,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":330,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":331,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":332,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":333,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_959e33b6","line":332,"range":{"start_line":329,"start_character":0,"end_line":332,"end_character":65},"updated":"2020-08-13 20:44:21.000000000","message":"This feels like it could almost be a helper, just given the couple different times I\u0027ve seen this.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":326,"context_line":"        # largest even number less than disk_count"},{"line_number":327,"context_line":"        return (disks_count \u003e\u003e 1) \u003c\u003c 1"},{"line_number":328,"context_line":"    else:"},{"line_number":329,"context_line":"        reason \u003d (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":330,"context_line":"                    \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":331,"context_line":"                  % {\u0027raid_level\u0027: raid_level,"},{"line_number":332,"context_line":"                     \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":333,"context_line":"        raise exception.RedfishError(error\u003dreason)"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ece95cb7","line":332,"range":{"start_line":329,"start_character":0,"end_line":332,"end_character":65},"in_reply_to":"9f560f44_959e33b6","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":696,"context_line":"            asynchronously or None if it is complete."},{"line_number":697,"context_line":"        \"\"\""},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        # TODO(billdodd): delete all RAID vols or just what Ironic knows about?"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":"        return self.post_delete_configuration(task)"},{"line_number":702,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_95ec732a","line":699,"range":{"start_line":699,"start_character":26,"end_line":699,"end_character":79},"updated":"2020-08-13 20:44:21.000000000","message":"This is a super good question. I don\u0027t know. I assume it should likely be all configurations, but it may just have to be up to the driver.\n\nThe reason for all is because we shouldn\u0027t potentially leave data around, and one means to \"destroy\" data is to rip apart raid configurations and recreate them.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":696,"context_line":"            asynchronously or None if it is complete."},{"line_number":697,"context_line":"        \"\"\""},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        # TODO(billdodd): delete all RAID vols or just what Ironic knows about?"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":"        return self.post_delete_configuration(task)"},{"line_number":702,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_6cd54ce5","line":699,"range":{"start_line":699,"start_character":26,"end_line":699,"end_character":79},"in_reply_to":"9f560f44_95ec732a","updated":"2020-08-19 23:29:44.000000000","message":"ack. Will keep this in mind as I work to complete the WIP.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        :param controllers: a list of dictionary containing the controller"},{"line_number":712,"context_line":"                            and new virtual disk resource."},{"line_number":713,"context_line":"        \"\"\""},{"line_number":714,"context_line":"        # TODO(billdodd): default implementation needed?"},{"line_number":715,"context_line":"        pass"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def post_delete_configuration(self, task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_756c3f9c","line":714,"updated":"2020-08-13 20:44:21.000000000","message":"I think as long as the base code calls it, it seems reasonable to not have a default implementation.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        :param controllers: a list of dictionary containing the controller"},{"line_number":712,"context_line":"                            and new virtual disk resource."},{"line_number":713,"context_line":"        \"\"\""},{"line_number":714,"context_line":"        # TODO(billdodd): default implementation needed?"},{"line_number":715,"context_line":"        pass"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def post_delete_configuration(self, task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_e4b86c2b","line":714,"in_reply_to":"9f560f44_756c3f9c","updated":"2020-08-19 23:29:44.000000000","message":"Agreed. It may turn out that there is some action needed for the default implementation. But if not, will leave it empty.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":23847,"name":"Richard G. Pioso","email":"richard.pioso@gmail.com","username":"richard.pioso"},"change_message_id":"f17a1c820a9cd301e43e6bd24134c6b59a435230","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_7aab4cd6","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"updated":"2020-10-05 22:43:12.000000000","message":"Could the workflow be arranged to support APPLY_TIME_IMMEDIATE, too. I think that should take precedence over APPLY_TIME_ON_RESET, because it would be faster since a reboot would not be needed.\n\nNote the reference to those constants. That follows the pattern used by the Redfish BIOS hardware interface implementation: https://opendev.org/openstack/ironic/src/commit/e7d08bcf7544eab5518a88108dd35fcf7f7d2ddf/ironic/drivers/modules/redfish/bios.py#L179-L183. It appears sushy would need to be modified to support that pattern.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":23847,"name":"Richard G. Pioso","email":"richard.pioso@gmail.com","username":"richard.pioso"},"change_message_id":"d84af5e5b67235716432308e06c3b1c711f6f3c9","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_2c07abcb","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"in_reply_to":"9f560f44_09ea99d7","updated":"2020-10-08 23:33:37.000000000","message":"I am not familiar with the reasoning. A possibility is that \"on reset\" is considered more human friendly than \u0027OnReset\u0027. It could be used in log messages and exceptions.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":23847,"name":"Richard G. Pioso","email":"richard.pioso@gmail.com","username":"richard.pioso"},"change_message_id":"b231e9ba1b96bb8ad1158732860339aae6c9a157","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_d94fe167","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"in_reply_to":"9f560f44_24b23b97","updated":"2020-10-07 23:34:28.000000000","message":"Without changing sushy, I agree it would not be feasible. And that may require deprecation. We would need to check with the core reviewers.\n\nIs there anything else preventing us from adopting that pattern? The use of constants and mappings is used throughout sushy.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"b9219c33e4d6813f551cadd590d0c1e338426d6c","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_24b23b97","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"in_reply_to":"9f560f44_7aab4cd6","updated":"2020-10-07 19:43:10.000000000","message":"Regarding the constants: I\u0027m not sure it\u0027s going to be feasible to use that pattern here. The supported_values field in OperationApplyTimeSupportField is simply a list field that returns the values in the list (e.g. [\"Immediate\", \"OnReset\"]), not a MappedListField that returns mapped values.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"a08dac10cfa1f375e7707303edcac94fb6c38b30","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_70a23141","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"in_reply_to":"9f560f44_7aab4cd6","updated":"2020-10-07 15:37:17.000000000","message":"Thanks for the input. I\u0027ll investigate and update.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"a864f655ecc958e139db465f59c5737e9063b6b7","unresolved":false,"context_lines":[{"line_number":561,"context_line":"               \u0027RAIDType\u0027: RAID_LEVELS[raid_level][\u0027raid_type\u0027],"},{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_09ea99d7","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":58},"in_reply_to":"9f560f44_d94fe167","updated":"2020-10-08 21:35:05.000000000","message":"Yes, the pattern can be adopted if we add a new property to OperationApplyTimeSupportField() that returns the mapped list (and deprecate the original one if needed). And add a new keyword param for apply_time to the create_volume() and delete_volume() methods. And maybe some other changes that I haven\u0027t realized yet. But it is doable.\n\nThis mapping between, for example, \"OnReset\" and \"on reset\" within the sushy library confuses me. I don\u0027t understand the value or purpose of it.\n\nI do understand the value of having a constant for these types of values. But it seems much simpler and straight forward to just set the constant to the schema-defined value, like:\n\nAPPLY_TIME_ON_RESET \u003d \u0027OnReset\u0027","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":23847,"name":"Richard G. Pioso","email":"richard.pioso@gmail.com","username":"richard.pioso"},"change_message_id":"f17a1c820a9cd301e43e6bd24134c6b59a435230","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            asynchronously or None if it is complete."},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"        # TODO(billdodd): delete all RAID vols or just what Ironic knows about?"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        return self.post_delete_configuration(task)"},{"line_number":713,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_7a3d0cbb","line":710,"updated":"2020-10-05 22:43:12.000000000","message":"My read of the documentation is that they should all be deleted. [1] While the specification does say much about this, it could be interpreted the same. [2] The idrac-wsman RAID hardware interface implementation deletes all RAID volumes and more.\n\n[1] https://docs.openstack.org/ironic/latest/admin/raid.html\n[2] https://opendev.org/openstack/ironic-specs/src/branch/master/specs/approved/ironic-generic-raid-interface.rst","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"a08dac10cfa1f375e7707303edcac94fb6c38b30","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            asynchronously or None if it is complete."},{"line_number":708,"context_line":"        \"\"\""},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"        # TODO(billdodd): delete all RAID vols or just what Ironic knows about?"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        return self.post_delete_configuration(task)"},{"line_number":713,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_10677526","line":710,"in_reply_to":"9f560f44_7a3d0cbb","updated":"2020-10-07 15:37:17.000000000","message":"Thanks for the insight.","commit_id":"9466ce5fd65b786c09584a5839ef43b72a77f36f"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"3cac538aa8cce801c5ac05162feb1ed9a8f2239a","unresolved":false,"context_lines":[{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_b777a0a0","line":565,"range":{"start_line":565,"start_character":49,"end_line":565,"end_character":58},"updated":"2020-11-06 16:23:15.000000000","message":"Where does it reset the system to apply the changes?\n\nRelated, looking around, this appears to be synchronous step. If system reset is necessary should it be async? I\u0027m thinking, at least for iDRAC, there are 2 scenarios - real time and with reboot. Could ironic or sushy determine which is the case and behave based on that?","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_667a2eba","line":565,"range":{"start_line":565,"start_character":49,"end_line":565,"end_character":58},"in_reply_to":"1f621f24_977f04df","updated":"2020-11-13 14:44:42.000000000","message":"great, in both cases the request is still async, meaning it will take more than couple of seconds. On iDRAC creating a volume takes around 2 minutes, deleting around 1.5 minutes.\n\nIf not introducing async operations using Tasks\u0026TaskService (at least for now), then need to increase sushy request timeout for these requests (currently 60s). I believe this shouldn\u0027t remain sync operation as it consumes a thread while waiting.\n\n(Recently TaskService was added for Firmware update patch [1][2])\n\n[1] https://review.opendev.org/#/c/745950/\n[2] https://review.opendev.org/#/c/749619/","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"90c3800dc3753913622da167aec44e62dff4bc31","unresolved":false,"context_lines":[{"line_number":562,"context_line":"               \u0027CapacityBytes\u0027: size_bytes}"},{"line_number":563,"context_line":"    if (apply_time_support and apply_time_support.supported_values"},{"line_number":564,"context_line":"            and \u0027OnReset\u0027 in apply_time_support.supported_values):"},{"line_number":565,"context_line":"        payload[\u0027@Redfish.OperationApplyTime\u0027] \u003d \u0027OnReset\u0027"},{"line_number":566,"context_line":"    if physical_disks:"},{"line_number":567,"context_line":"        payload[\u0027Links\u0027] \u003d {"},{"line_number":568,"context_line":"            \"Drives\": [{\"@odata.id\": _drive_path(storage, d)} for d in"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_977f04df","line":565,"range":{"start_line":565,"start_character":49,"end_line":565,"end_character":58},"in_reply_to":"1f621f24_b777a0a0","updated":"2020-11-06 17:19:44.000000000","message":"Right. The plan is to support both scenarios. Will check the OperationApplyTime support and use Immediate if available, else fall back to OnReset. I have submitted a patch set for sushy that I\u0027m waiting on to make related changes here.\n\nAnd there will need to be code added in this module to handle doing the reset for the OnReset case (it just hasn\u0027t been done yet in this WIP).","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":587,"context_line":"    :param disk_name: name of the virtual disk. (optional)"},{"line_number":588,"context_line":"    :param span_depth: Number of spans in virtual disk. (optional)"},{"line_number":589,"context_line":"    :param span_length: Number of disks per span. (optional)"},{"line_number":590,"context_line":"    :returns: a dictionary containing the commit_needed key with a boolean"},{"line_number":591,"context_line":"              value indicating whether a config job must be created for the"},{"line_number":592,"context_line":"              values to be applied."},{"line_number":593,"context_line":"    :raises: RedfishConnectionError when it fails to connect to Redfish"},{"line_number":594,"context_line":"    :raises: RedfishError if there is an error creating the virtual disk"},{"line_number":595,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_b11c7e78","line":592,"range":{"start_line":590,"start_character":14,"end_line":592,"end_character":35},"updated":"2020-11-13 14:44:42.000000000","message":"copy-paste?","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"05e4e55a0a5df17b80d37deed77e37cfe006cec3","unresolved":false,"context_lines":[{"line_number":587,"context_line":"    :param disk_name: name of the virtual disk. (optional)"},{"line_number":588,"context_line":"    :param span_depth: Number of spans in virtual disk. (optional)"},{"line_number":589,"context_line":"    :param span_length: Number of disks per span. (optional)"},{"line_number":590,"context_line":"    :returns: a dictionary containing the commit_needed key with a boolean"},{"line_number":591,"context_line":"              value indicating whether a config job must be created for the"},{"line_number":592,"context_line":"              values to be applied."},{"line_number":593,"context_line":"    :raises: RedfishConnectionError when it fails to connect to Redfish"},{"line_number":594,"context_line":"    :raises: RedfishError if there is an error creating the virtual disk"},{"line_number":595,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ae6d80f3_21c76192","line":592,"range":{"start_line":590,"start_character":14,"end_line":592,"end_character":35},"in_reply_to":"1f621f24_b11c7e78","updated":"2021-01-08 00:25:58.000000000","message":"done","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        disk_name\u003ddisk_name, span_length\u003dspan_length, span_depth\u003dspan_depth)"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"    try:"},{"line_number":612,"context_line":"        return volume_collection.create_volume(payload)"},{"line_number":613,"context_line":"    except sushy.exceptions.SushyError as exc:"},{"line_number":614,"context_line":"        LOG.error(\u0027Redfish driver failed to create virtual disk for node \u0027"},{"line_number":615,"context_line":"                  \u0027%(node_uuid)s. Reason: %(error)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_8c7b298a","line":612,"range":{"start_line":612,"start_character":7,"end_line":612,"end_character":55},"updated":"2020-11-13 14:44:42.000000000","message":"Have to check if something is returned.\nAgain for iDRAC, it is possible to submit a request, create a job, but job fails (Task\u0027s TaskStatus will be Critical with HTTP 200 response). From sushy code it appears that in that case None will be returned, from Ironic\u0027s point of view this is error.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"03749c7f2db25dbecbf5b7160aa6933bf5f70761","unresolved":true,"context_lines":[{"line_number":611,"context_line":"    try:"},{"line_number":612,"context_line":"        return volume_collection.create_volume(payload)"},{"line_number":613,"context_line":"    except sushy.exceptions.SushyError as exc:"},{"line_number":614,"context_line":"        LOG.error(\u0027Redfish driver failed to create virtual disk for node \u0027"},{"line_number":615,"context_line":"                  \u0027%(node_uuid)s. Reason: %(error)s.\u0027,"},{"line_number":616,"context_line":"                  {\u0027node_uuid\u0027: node.uuid,"},{"line_number":617,"context_line":"                   \u0027error\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":7,"id":"2b14ba25_65989d7a","line":614,"updated":"2020-12-09 10:15:42.000000000","message":"Unfortunately, there are some bugs in iDRAC related to Volume creation in Redfish. Until they are fixed (currently estimated June 2021) and for those running older versions, would it be possible to add a hook here to try to handle error in idrac-redfish implementation? All the bugs found can be worked around by changing payload and resubmitting. These errors are returned in synchronous response when submitting Volume request (that is, not outcome of async Task).\n\nCould add here a hook similar to post configuration that looks like this:\n\n  def create_error_handler(self, task, exc, volume_collection, payload)\n\nand expecting to return whatever volume_collection.create_volume() is returning (indicating success/failure) that is handled is `except` block - if error handler couldn\u0027t fix the error, then raising `RedfishError`, if error handler managed to work around it and submitted the request, no exception raised.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"05e4e55a0a5df17b80d37deed77e37cfe006cec3","unresolved":false,"context_lines":[{"line_number":611,"context_line":"    try:"},{"line_number":612,"context_line":"        return volume_collection.create_volume(payload)"},{"line_number":613,"context_line":"    except sushy.exceptions.SushyError as exc:"},{"line_number":614,"context_line":"        LOG.error(\u0027Redfish driver failed to create virtual disk for node \u0027"},{"line_number":615,"context_line":"                  \u0027%(node_uuid)s. Reason: %(error)s.\u0027,"},{"line_number":616,"context_line":"                  {\u0027node_uuid\u0027: node.uuid,"},{"line_number":617,"context_line":"                   \u0027error\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":7,"id":"1d30845a_d6f377b5","line":614,"in_reply_to":"2b14ba25_65989d7a","updated":"2021-01-08 00:25:58.000000000","message":"@Aija: Hook added. Please review and see if it meets your expectations.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"03749c7f2db25dbecbf5b7160aa6933bf5f70761","unresolved":true,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"        logical_disks_to_create \u003d _filter_logical_disks("},{"line_number":683,"context_line":"            logical_disks, create_root_volume, create_nonroot_volumes)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        controllers \u003d list()"},{"line_number":686,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":687,"context_line":"            controller \u003d dict()"}],"source_content_type":"text/x-python","patch_set":7,"id":"79d6e932_ea1b37ea","line":684,"updated":"2020-12-09 10:15:42.000000000","message":"There is a bug/missing feature in iDRAC for cases where physical disks are in non-RAID mode. In that case the volume creation will fail with error saying that some disks are not in correct state. In WS-MAN implementation physical disks are converted to RAID mode if needed.\n\nThere are no Redfish standard methods to convert to/from RAID, so one of the solutions for iDRAC could be that it converts that internally when receiving create Volume request. Still waiting from iDRAC team what their direction will be around this, but until this is fixed (that will take some time anyway), would it be possible to add here pre_create_configuration hook so that in idrac-redfish implementation OEM extensions are used to check and convert non-RAID physical disks? It could look like this:\n\n  def pre_create_configuration(self, task, logical_disks_to_create)\n\nThough have to think about how this will work with async operations (as converting will be long-ish running tasks).","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"05e4e55a0a5df17b80d37deed77e37cfe006cec3","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"        logical_disks_to_create \u003d _filter_logical_disks("},{"line_number":683,"context_line":"            logical_disks, create_root_volume, create_nonroot_volumes)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        controllers \u003d list()"},{"line_number":686,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":687,"context_line":"            controller \u003d dict()"}],"source_content_type":"text/x-python","patch_set":7,"id":"16b82bd3_140dd1d2","line":684,"in_reply_to":"79d6e932_ea1b37ea","updated":"2021-01-08 00:25:58.000000000","message":"@Aija: Hook added. Please review and see if it meets your expectations.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":683,"context_line":"            logical_disks, create_root_volume, create_nonroot_volumes)"},{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        controllers \u003d list()"},{"line_number":686,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":687,"context_line":"            controller \u003d dict()"},{"line_number":688,"context_line":"            resource \u003d create_virtual_disk("},{"line_number":689,"context_line":"                node,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_5149aab4","line":686,"range":{"start_line":686,"start_character":8,"end_line":686,"end_character":52},"updated":"2020-11-13 14:44:42.000000000","message":"just a note that applies to deleting volumes too - it creates/delete volumes one by one where in WS-MAN and looks like also ilo has one request for all logical disks. This could make the Redfish implementation slower compared to vendor protocols. And at least for iDRAC can\u0027t issue these requests in parallel as each request creates a new iDRAC job and can\u0027t have more than 1 job per subsystem.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        try:"},{"line_number":715,"context_line":"            for storage in system.storage.get_members():"},{"line_number":716,"context_line":"                for volume in storage.volumes.get_members():"},{"line_number":717,"context_line":"                    if volume.raid_type:"},{"line_number":718,"context_line":"                        vols_to_delete.append(volume)"},{"line_number":719,"context_line":"        except sushy.exceptions.SushyError as exc:"},{"line_number":720,"context_line":"            error_msg \u003d _(\u0027Cannot get the list of volumes to delete for node \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_51decad9","line":717,"range":{"start_line":717,"start_character":30,"end_line":717,"end_character":39},"updated":"2020-11-13 14:44:42.000000000","message":"raid_type is somewhat recent addition, safer to also check VolumeType !\u003d \u0027RawDevice\u0027\n\n(yes, iDRAC does not implement RAIDType yet)","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"03749c7f2db25dbecbf5b7160aa6933bf5f70761","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        try:"},{"line_number":715,"context_line":"            for storage in system.storage.get_members():"},{"line_number":716,"context_line":"                for volume in storage.volumes.get_members():"},{"line_number":717,"context_line":"                    if volume.raid_type:"},{"line_number":718,"context_line":"                        vols_to_delete.append(volume)"},{"line_number":719,"context_line":"        except sushy.exceptions.SushyError as exc:"},{"line_number":720,"context_line":"            error_msg \u003d _(\u0027Cannot get the list of volumes to delete for node \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a36678b_695a23b1","line":717,"range":{"start_line":717,"start_character":30,"end_line":717,"end_character":39},"in_reply_to":"1f621f24_51decad9","updated":"2020-12-09 10:15:42.000000000","message":"JFYI, RAIDType will be available in the next iDRAC version for 14G+; 13G will never get RAIDType support.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"6bf398af8b73ef7d838c4952f655517121b97d58","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        try:"},{"line_number":715,"context_line":"            for storage in system.storage.get_members():"},{"line_number":716,"context_line":"                for volume in storage.volumes.get_members():"},{"line_number":717,"context_line":"                    if volume.raid_type:"},{"line_number":718,"context_line":"                        vols_to_delete.append(volume)"},{"line_number":719,"context_line":"        except sushy.exceptions.SushyError as exc:"},{"line_number":720,"context_line":"            error_msg \u003d _(\u0027Cannot get the list of volumes to delete for node \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ad93dfb_7619670f","line":717,"range":{"start_line":717,"start_character":30,"end_line":717,"end_character":39},"in_reply_to":"7a36678b_695a23b1","updated":"2021-01-25 17:40:06.000000000","message":"Updated in patch set 12.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"abb31c401ebd47d655c24088c43c044527842e2c","unresolved":false,"context_lines":[{"line_number":715,"context_line":"            for storage in system.storage.get_members():"},{"line_number":716,"context_line":"                for volume in storage.volumes.get_members():"},{"line_number":717,"context_line":"                    if volume.raid_type:"},{"line_number":718,"context_line":"                        vols_to_delete.append(volume)"},{"line_number":719,"context_line":"        except sushy.exceptions.SushyError as exc:"},{"line_number":720,"context_line":"            error_msg \u003d _(\u0027Cannot get the list of volumes to delete for node \u0027"},{"line_number":721,"context_line":"                          \u0027%(node_uuid)s. Reason: %(error)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f621f24_0cac39eb","line":718,"range":{"start_line":718,"start_character":24,"end_line":718,"end_character":53},"updated":"2020-11-13 14:44:42.000000000","message":"similar to creating there is no error handling from Task that can fail too. In that case there will be no SushyError, but it\u0027s still an error - failure to delete config.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"03749c7f2db25dbecbf5b7160aa6933bf5f70761","unresolved":true,"context_lines":[{"line_number":722,"context_line":"                          {\u0027node_uuid\u0027: node.uuid, \u0027error\u0027: exc})"},{"line_number":723,"context_line":"            LOG.error(error_msg)"},{"line_number":724,"context_line":"            raise exception.RedfishError(error\u003dexc)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        # TODO(billdodd): add ApplyTime handling once sushy task 41087 merges"},{"line_number":727,"context_line":"        for volume in vols_to_delete:"},{"line_number":728,"context_line":"            volume.delete_volume()"}],"source_content_type":"text/x-python","patch_set":7,"id":"a062c9b7_7d78ea28","line":725,"updated":"2020-12-09 10:15:42.000000000","message":"Could add here hook for `pre_delete_configuration(self, task, vols_to_delete)` - so that idrac-redfish implementation can delete foreign configs via OEM extensions and do other processing if necessary? (Though, again, have to think how this works with async ops)","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"05e4e55a0a5df17b80d37deed77e37cfe006cec3","unresolved":false,"context_lines":[{"line_number":722,"context_line":"                          {\u0027node_uuid\u0027: node.uuid, \u0027error\u0027: exc})"},{"line_number":723,"context_line":"            LOG.error(error_msg)"},{"line_number":724,"context_line":"            raise exception.RedfishError(error\u003dexc)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        # TODO(billdodd): add ApplyTime handling once sushy task 41087 merges"},{"line_number":727,"context_line":"        for volume in vols_to_delete:"},{"line_number":728,"context_line":"            volume.delete_volume()"}],"source_content_type":"text/x-python","patch_set":7,"id":"1926b5ca_b98a0be6","line":725,"in_reply_to":"a062c9b7_7d78ea28","updated":"2021-01-08 00:25:58.000000000","message":"@Aija: Hook added. Please review and see if it meets your expectations.","commit_id":"a4e0a4095e7dbab06941f0c18d21eb5c5d4927ca"},{"author":{"_account_id":24828,"name":"Kaifeng Wang","email":"kaifeng.w@gmail.com","username":"wangkf"},"change_message_id":"da03e3016c876dc2361ce74083a8f217f319cca3","unresolved":true,"context_lines":[{"line_number":536,"context_line":"            filtered_disks.append(disk)"},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"    return filtered_disks"},{"line_number":539,"context_line":""},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"def _get_storage_controller(node, system, physical_disks):"},{"line_number":542,"context_line":"    collection \u003d system.storage"}],"source_content_type":"text/x-python","patch_set":9,"id":"8de1848a_2043a4b1","line":539,"updated":"2021-01-11 15:36:25.000000000","message":"There seems to be some similarity with vendor raid implementation, I wonder if we could abstract common bits into a module, since half part of the raid is not vendor-specific?","commit_id":"9b30b7a0e663078402eba365d59c72f0a42354fe"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"6bf398af8b73ef7d838c4952f655517121b97d58","unresolved":true,"context_lines":[{"line_number":536,"context_line":"            filtered_disks.append(disk)"},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"    return filtered_disks"},{"line_number":539,"context_line":""},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"def _get_storage_controller(node, system, physical_disks):"},{"line_number":542,"context_line":"    collection \u003d system.storage"}],"source_content_type":"text/x-python","patch_set":9,"id":"b2aff830_7a2ea8fa","line":539,"in_reply_to":"8de1848a_2043a4b1","updated":"2021-01-25 17:40:06.000000000","message":"Sorry I didn\u0027t respond to this sooner. I think the design and intention is that once this base Redfish RAID implementation is available, vendors can can use it as a base and extend as necessary to handle any vendor-specific requirements. See the extension points below on lines 742-807. Does that make sense?","commit_id":"9b30b7a0e663078402eba365d59c72f0a42354fe"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":94,"context_line":"def convert_drive_units(logical_disks, node):"},{"line_number":95,"context_line":"    \"\"\"Convert size in logical_disks from gb to bytes\"\"\""},{"line_number":96,"context_line":"    for disk in logical_disks:"},{"line_number":97,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027 and \u0027physical_disks\u0027 not in disk:"},{"line_number":98,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":99,"context_line":"                _(\"create_configuration called with invalid \""},{"line_number":100,"context_line":"                  \"target_raid_configuration for node %(node_id)s. \""}],"source_content_type":"text/x-python","patch_set":12,"id":"c0badee2_36368bbe","line":97,"updated":"2021-02-02 14:38:13.000000000","message":"Not sure I understand this limitation, at least a case of RAID1 on a pair of identicaly disks has to be supported. Otherwise even the basic usage of RAID will require knowing either the sizes or the vendor-specific physical disk IDs, which is not exactly cloud-friendly.\n\nI would allow size\u003dMAX if the machine has the right amount of disks with the same size to create all unspecified volumes.\n\nI\u0027m not putting -1 because of this suggestion since it can be fixed in a follow-up, but please do consider it.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":94,"context_line":"def convert_drive_units(logical_disks, node):"},{"line_number":95,"context_line":"    \"\"\"Convert size in logical_disks from gb to bytes\"\"\""},{"line_number":96,"context_line":"    for disk in logical_disks:"},{"line_number":97,"context_line":"        if disk[\u0027size_gb\u0027] \u003d\u003d \u0027MAX\u0027 and \u0027physical_disks\u0027 not in disk:"},{"line_number":98,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":99,"context_line":"                _(\"create_configuration called with invalid \""},{"line_number":100,"context_line":"                  \"target_raid_configuration for node %(node_id)s. \""}],"source_content_type":"text/x-python","patch_set":12,"id":"a0e10094_eac09a49","line":97,"in_reply_to":"c0badee2_36368bbe","updated":"2021-02-05 19:39:10.000000000","message":"Ack. Will look at this in follow-up.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        for storage in collection.get_members():"},{"line_number":129,"context_line":"            disks.extend(storage.drives)"},{"line_number":130,"context_line":"            controller \u003d storage.storage_controllers["},{"line_number":131,"context_line":"                0] if storage.storage_controllers else None"},{"line_number":132,"context_line":"            for drive in storage.drives:"},{"line_number":133,"context_line":"                disk_to_controller[drive] \u003d controller"},{"line_number":134,"context_line":"    except sushy.exceptions.SushyError as exc:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3b226a31_395f55ca","line":131,"updated":"2021-02-02 14:38:13.000000000","message":"nit: please avoid splitting [] operator like this, better use brackets2","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        for storage in collection.get_members():"},{"line_number":129,"context_line":"            disks.extend(storage.drives)"},{"line_number":130,"context_line":"            controller \u003d storage.storage_controllers["},{"line_number":131,"context_line":"                0] if storage.storage_controllers else None"},{"line_number":132,"context_line":"            for drive in storage.drives:"},{"line_number":133,"context_line":"                disk_to_controller[drive] \u003d controller"},{"line_number":134,"context_line":"    except sushy.exceptions.SushyError as exc:"}],"source_content_type":"text/x-python","patch_set":12,"id":"38aefe2e_b85419e6","line":131,"in_reply_to":"3b226a31_395f55ca","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    :param raid_level: RAID level of the virtual disk"},{"line_number":147,"context_line":"    :return: the error message text"},{"line_number":148,"context_line":"    \"\"\""},{"line_number":149,"context_line":"    return (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":150,"context_line":"              \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":151,"context_line":"            % {\u0027raid_level\u0027: raid_level,"},{"line_number":152,"context_line":"               \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"}],"source_content_type":"text/x-python","patch_set":12,"id":"2754dbdf_598737d5","line":149,"updated":"2021-02-02 14:38:13.000000000","message":"nit: why not raise right here","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    :param raid_level: RAID level of the virtual disk"},{"line_number":147,"context_line":"    :return: the error message text"},{"line_number":148,"context_line":"    \"\"\""},{"line_number":149,"context_line":"    return (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":150,"context_line":"              \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":151,"context_line":"            % {\u0027raid_level\u0027: raid_level,"},{"line_number":152,"context_line":"               \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"}],"source_content_type":"text/x-python","patch_set":12,"id":"8b4f3d7c_2c9db448","line":149,"in_reply_to":"2754dbdf_598737d5","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    return (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":150,"context_line":"              \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":151,"context_line":"            % {\u0027raid_level\u0027: raid_level,"},{"line_number":152,"context_line":"               \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"}],"source_content_type":"text/x-python","patch_set":12,"id":"e5aa7f09_d7a97511","line":152,"updated":"2021-02-02 14:38:13.000000000","message":"nit: \u0027, \u0027.join(RAID_LEVELS)","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    return (_(\u0027RAID level %(raid_level)s is not supported by the \u0027"},{"line_number":150,"context_line":"              \u0027driver. Supported RAID levels: %(supported_raid_levels)s\u0027)"},{"line_number":151,"context_line":"            % {\u0027raid_level\u0027: raid_level,"},{"line_number":152,"context_line":"               \u0027supported_raid_levels\u0027: list(RAID_LEVELS)})"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"}],"source_content_type":"text/x-python","patch_set":12,"id":"c97e5464_00ae55ee","line":152,"in_reply_to":"e5aa7f09_d7a97511","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"},{"line_number":156,"context_line":"    \"\"\"Calculate the drive overhead for the given RAID level"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param raid_level: RAID level of the virtual disk"},{"line_number":159,"context_line":"    :param spans_count: number of spans for the virtual disk"}],"source_content_type":"text/x-python","patch_set":12,"id":"fae434db_547d0fca","line":156,"updated":"2021-02-02 14:38:13.000000000","message":"Would be cool to expand what \"drive overhead\" actually means and how it is calculated.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"def _raid_level_overhead(raid_level, spans_count\u003d1):"},{"line_number":156,"context_line":"    \"\"\"Calculate the drive overhead for the given RAID level"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    :param raid_level: RAID level of the virtual disk"},{"line_number":159,"context_line":"    :param spans_count: number of spans for the virtual disk"}],"source_content_type":"text/x-python","patch_set":12,"id":"d94adee7_cdab1174","line":156,"in_reply_to":"fae434db_547d0fca","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"def _max_volume_size_bytes(raid_level, physical_disks, free_space_bytes,"},{"line_number":180,"context_line":"                           spans_count\u003d1, stripe_size_kb\u003d64 * units.Ki):"},{"line_number":181,"context_line":"    # restrict the size to the smallest available space"},{"line_number":182,"context_line":"    free_spaces \u003d [free_space_bytes[disk] for disk in physical_disks]"},{"line_number":183,"context_line":"    size_kb \u003d min(free_spaces) // units.Ki"}],"source_content_type":"text/x-python","patch_set":12,"id":"1bfed71e_c0cd7cc1","line":180,"updated":"2021-02-02 14:38:13.000000000","message":"These functions are copied from iDRAC, right? Maybe time to create ironic.drivers.modules.raid_utils?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"def _max_volume_size_bytes(raid_level, physical_disks, free_space_bytes,"},{"line_number":180,"context_line":"                           spans_count\u003d1, stripe_size_kb\u003d64 * units.Ki):"},{"line_number":181,"context_line":"    # restrict the size to the smallest available space"},{"line_number":182,"context_line":"    free_spaces \u003d [free_space_bytes[disk] for disk in physical_disks]"},{"line_number":183,"context_line":"    size_kb \u003d min(free_spaces) // units.Ki"}],"source_content_type":"text/x-python","patch_set":12,"id":"a2bfba3a_7ec5ac7b","line":180,"in_reply_to":"1bfed71e_c0cd7cc1","updated":"2021-02-05 19:39:10.000000000","message":"Ack. If it\u0027s OK, would like to look at this as a follow-up.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # NOTE(ifarkas): using math.floor so we get a volume size that does not"},{"line_number":186,"context_line":"    #                exceed the available space"},{"line_number":187,"context_line":"    stripes_per_disk \u003d int(math.floor(float(size_kb) / stripe_size_kb))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    disks_count \u003d len(physical_disks)"},{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"}],"source_content_type":"text/x-python","patch_set":12,"id":"8daa1936_69445d8a","line":187,"updated":"2021-02-02 14:38:13.000000000","message":"Should stripe_size_kb be configurable? Sounds like something an advanced operator may want to configure, but I\u0027m not a RAID expert.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":true,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # NOTE(ifarkas): using math.floor so we get a volume size that does not"},{"line_number":186,"context_line":"    #                exceed the available space"},{"line_number":187,"context_line":"    stripes_per_disk \u003d int(math.floor(float(size_kb) / stripe_size_kb))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    disks_count \u003d len(physical_disks)"},{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"}],"source_content_type":"text/x-python","patch_set":12,"id":"95c614cc_e61b59a1","line":187,"in_reply_to":"8daa1936_69445d8a","updated":"2021-02-05 19:39:10.000000000","message":"That makes sense. I started adding a config variable for this. But then I got confused with the units here. \n\nstripe_size_kb\u003d64 * units.Ki\n\nA stripe size of 64 KB would be typical, I think. But the default value being set for stripe_size_kb is 65536. But I don\u0027t think the intention is really 65536 KB, is it?\n\n(I\u0027m clearly no RAID expert.)\n\nIf any reviewers have insight, please speak up.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":187,"context_line":"    stripes_per_disk \u003d int(math.floor(float(size_kb) / stripe_size_kb))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    disks_count \u003d len(physical_disks)"},{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    max_volume_size_bytes \u003d int("},{"line_number":193,"context_line":"        stripes_per_disk * stripe_size_kb"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f5a1390_c3297a44","line":190,"updated":"2021-02-02 14:38:13.000000000","message":"Should raise if disks_count \u003c\u003d overhead_disks_count","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":187,"context_line":"    stripes_per_disk \u003d int(math.floor(float(size_kb) / stripe_size_kb))"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    disks_count \u003d len(physical_disks)"},{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    max_volume_size_bytes \u003d int("},{"line_number":193,"context_line":"        stripes_per_disk * stripe_size_kb"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ccfadd7_15d95244","line":190,"in_reply_to":"1f5a1390_c3297a44","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    max_volume_size_bytes \u003d int("},{"line_number":193,"context_line":"        stripes_per_disk * stripe_size_kb"},{"line_number":194,"context_line":"        * (disks_count - overhead_disks_count) * units.Ki)"},{"line_number":195,"context_line":"    return max_volume_size_bytes"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"67322a75_e586924e","line":193,"updated":"2021-02-02 14:38:13.000000000","message":"nit: Why cannot we use size_kb here? I assume we need a whole number of stipes, maybe leave a comment explaining it (or better a docstring for this function)?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":true,"context_lines":[{"line_number":190,"context_line":"    overhead_disks_count \u003d _raid_level_overhead(raid_level, spans_count)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    max_volume_size_bytes \u003d int("},{"line_number":193,"context_line":"        stripes_per_disk * stripe_size_kb"},{"line_number":194,"context_line":"        * (disks_count - overhead_disks_count) * units.Ki)"},{"line_number":195,"context_line":"    return max_volume_size_bytes"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"b149e734_3fbcd3ec","line":193,"in_reply_to":"67322a75_e586924e","updated":"2021-02-05 19:39:10.000000000","message":"Good questions. Currently I don\u0027t have any good answers. As mentioned earlier, a lot of these calculation details were copied from drac/raid.py.\n\nAnother plea for any RAID experts to chime if with any insight.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":232,"context_line":""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"def _calculate_volume_props(logical_disk, physical_disks, free_space_bytes,"},{"line_number":235,"context_line":"                            disk_to_controller):"},{"line_number":236,"context_line":"    # TODO(billdodd): match e.g. {\u0027size\u0027: \u0027\u003e 100\u0027} -\u003e oslo_utils.specs_matcher"},{"line_number":237,"context_line":"    selected_disks \u003d [disk for disk in physical_disks"},{"line_number":238,"context_line":"                      if disk.identity in logical_disk[\u0027physical_disks\u0027]]"}],"source_content_type":"text/x-python","patch_set":12,"id":"0885b2a7_b036193d","line":235,"updated":"2021-02-02 14:38:13.000000000","message":"nit: docstring please","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":232,"context_line":""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"def _calculate_volume_props(logical_disk, physical_disks, free_space_bytes,"},{"line_number":235,"context_line":"                            disk_to_controller):"},{"line_number":236,"context_line":"    # TODO(billdodd): match e.g. {\u0027size\u0027: \u0027\u003e 100\u0027} -\u003e oslo_utils.specs_matcher"},{"line_number":237,"context_line":"    selected_disks \u003d [disk for disk in physical_disks"},{"line_number":238,"context_line":"                      if disk.identity in logical_disk[\u0027physical_disks\u0027]]"}],"source_content_type":"text/x-python","patch_set":12,"id":"ffc91c95_35293655","line":235,"in_reply_to":"0885b2a7_b036193d","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        logical_disk[\u0027raid_level\u0027], len(selected_disks))"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    if len(selected_disks) % spans_count !\u003d 0:"},{"line_number":244,"context_line":"        error_msg \u003d _(\u0027invalid number of physical disks was provided\u0027)"},{"line_number":245,"context_line":"        raise exception.RedfishError(error\u003derror_msg)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    disks_per_span \u003d len(selected_disks) / spans_count"}],"source_content_type":"text/x-python","patch_set":12,"id":"5bd5f1ee_28c9601e","line":244,"updated":"2021-02-02 14:38:13.000000000","message":"Let\u0027s add some details here, especially since span_count is something we calculate","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        logical_disk[\u0027raid_level\u0027], len(selected_disks))"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    if len(selected_disks) % spans_count !\u003d 0:"},{"line_number":244,"context_line":"        error_msg \u003d _(\u0027invalid number of physical disks was provided\u0027)"},{"line_number":245,"context_line":"        raise exception.RedfishError(error\u003derror_msg)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    disks_per_span \u003d len(selected_disks) / spans_count"}],"source_content_type":"text/x-python","patch_set":12,"id":"76b48ef6_51ca0e65","line":244,"in_reply_to":"5bd5f1ee_28c9601e","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        logical_disk[\u0027size_bytes\u0027] \u003d max_volume_size_bytes"},{"line_number":270,"context_line":"    elif max_volume_size_bytes \u003c logical_disk[\u0027size_bytes\u0027]:"},{"line_number":271,"context_line":"        if max_volume_size_bytes \u003d\u003d 0:"},{"line_number":272,"context_line":"            error_msg \u003d _(\u0027not enough physical disk space for the logical \u0027"},{"line_number":273,"context_line":"                          \u0027disk\u0027)"},{"line_number":274,"context_line":"            raise exception.RedfishError(error\u003derror_msg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"2e208821_1b43084d","line":271,"updated":"2021-02-02 14:38:13.000000000","message":"I\u0027m not sure I understand why this condition is needed, I think we should raise either way","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        logical_disk[\u0027size_bytes\u0027] \u003d max_volume_size_bytes"},{"line_number":270,"context_line":"    elif max_volume_size_bytes \u003c logical_disk[\u0027size_bytes\u0027]:"},{"line_number":271,"context_line":"        if max_volume_size_bytes \u003d\u003d 0:"},{"line_number":272,"context_line":"            error_msg \u003d _(\u0027not enough physical disk space for the logical \u0027"},{"line_number":273,"context_line":"                          \u0027disk\u0027)"},{"line_number":274,"context_line":"            raise exception.RedfishError(error\u003derror_msg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"70dd9b36_70ef8a3c","line":271,"in_reply_to":"2e208821_1b43084d","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    elif max_volume_size_bytes \u003c logical_disk[\u0027size_bytes\u0027]:"},{"line_number":271,"context_line":"        if max_volume_size_bytes \u003d\u003d 0:"},{"line_number":272,"context_line":"            error_msg \u003d _(\u0027not enough physical disk space for the logical \u0027"},{"line_number":273,"context_line":"                          \u0027disk\u0027)"},{"line_number":274,"context_line":"            raise exception.RedfishError(error\u003derror_msg)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    disk_usage \u003d _volume_usage_per_disk_bytes(logical_disk, selected_disks,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a7049ee_f9507a92","line":273,"updated":"2021-02-02 14:38:13.000000000","message":"let\u0027s mention how much we need and how much we have","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    elif max_volume_size_bytes \u003c logical_disk[\u0027size_bytes\u0027]:"},{"line_number":271,"context_line":"        if max_volume_size_bytes \u003d\u003d 0:"},{"line_number":272,"context_line":"            error_msg \u003d _(\u0027not enough physical disk space for the logical \u0027"},{"line_number":273,"context_line":"                          \u0027disk\u0027)"},{"line_number":274,"context_line":"            raise exception.RedfishError(error\u003derror_msg)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    disk_usage \u003d _volume_usage_per_disk_bytes(logical_disk, selected_disks,"}],"source_content_type":"text/x-python","patch_set":12,"id":"cee8ba0f_5c267f7b","line":273,"in_reply_to":"3a7049ee_f9507a92","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                                        candidate_free_space_bytes,"},{"line_number":408,"context_line":"                                        disk_to_controller)"},{"line_number":409,"context_line":"            except exception.RedfishError:"},{"line_number":410,"context_line":"                continue"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            if len(logical_disks) \u003e 0:"},{"line_number":413,"context_line":"                result, candidate_free_space_bytes \u003d ("}],"source_content_type":"text/x-python","patch_set":12,"id":"ec845b8a_40cd674a","line":410,"updated":"2021-02-02 14:38:13.000000000","message":"maybe LOG.debug here? otherwise it will be too hard to understand why something went wrong.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                                        candidate_free_space_bytes,"},{"line_number":408,"context_line":"                                        disk_to_controller)"},{"line_number":409,"context_line":"            except exception.RedfishError:"},{"line_number":410,"context_line":"                continue"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            if len(logical_disks) \u003e 0:"},{"line_number":413,"context_line":"                result, candidate_free_space_bytes \u003d ("}],"source_content_type":"text/x-python","patch_set":12,"id":"e9097d5c_3c2166dc","line":410,"in_reply_to":"ec845b8a_40cd674a","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            if len(logical_disks) \u003e 0:"},{"line_number":413,"context_line":"                result, candidate_free_space_bytes \u003d ("},{"line_number":414,"context_line":"                    _assign_disks_to_volume(logical_disks,"},{"line_number":415,"context_line":"                                            physical_disks_by_type,"},{"line_number":416,"context_line":"                                            candidate_free_space_bytes,"},{"line_number":417,"context_line":"                                            disk_to_controller))"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa704c7c_fb6046d7","line":414,"updated":"2021-02-02 14:38:13.000000000","message":"nit: This can raise, maybe handle it?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            if len(logical_disks) \u003e 0:"},{"line_number":413,"context_line":"                result, candidate_free_space_bytes \u003d ("},{"line_number":414,"context_line":"                    _assign_disks_to_volume(logical_disks,"},{"line_number":415,"context_line":"                                            physical_disks_by_type,"},{"line_number":416,"context_line":"                                            candidate_free_space_bytes,"},{"line_number":417,"context_line":"                                            disk_to_controller))"}],"source_content_type":"text/x-python","patch_set":12,"id":"34c8b2a3_73d02951","line":414,"in_reply_to":"fa704c7c_fb6046d7","updated":"2021-02-05 19:39:10.000000000","message":"Done.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d90ab96cd4fff4bccffc5f743417f7ed1786c442","unresolved":false,"context_lines":[{"line_number":647,"context_line":"        \"\"\""},{"line_number":648,"context_line":"        return redfish_utils.COMMON_PROPERTIES.copy()"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"    @base.clean_step(priority\u003d0, abortable\u003dFalse, argsinfo\u003d{"},{"line_number":651,"context_line":"        \u0027create_root_volume\u0027: {"},{"line_number":652,"context_line":"            \u0027description\u0027: ("},{"line_number":653,"context_line":"                \u0027This specifies whether to create the root volume. \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"f916a769_d2e19627","line":650,"updated":"2021-02-02 14:38:13.000000000","message":"RAID deploy steps are a bit different, they use apply_configuration (which is delete+create together)","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":647,"context_line":"        \"\"\""},{"line_number":648,"context_line":"        return redfish_utils.COMMON_PROPERTIES.copy()"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"    @base.clean_step(priority\u003d0, abortable\u003dFalse, argsinfo\u003d{"},{"line_number":651,"context_line":"        \u0027create_root_volume\u0027: {"},{"line_number":652,"context_line":"            \u0027description\u0027: ("},{"line_number":653,"context_line":"                \u0027This specifies whether to create the root volume. \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"e1676050_57c0fe2c","line":650,"range":{"start_line":650,"start_character":4,"end_line":650,"end_character":20},"updated":"2021-01-28 19:40:31.000000000","message":"are there any plans to make these deploy_step-s too?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":647,"context_line":"        \"\"\""},{"line_number":648,"context_line":"        return redfish_utils.COMMON_PROPERTIES.copy()"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"    @base.clean_step(priority\u003d0, abortable\u003dFalse, argsinfo\u003d{"},{"line_number":651,"context_line":"        \u0027create_root_volume\u0027: {"},{"line_number":652,"context_line":"            \u0027description\u0027: ("},{"line_number":653,"context_line":"                \u0027This specifies whether to create the root volume. \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"dd2d76c6_99627757","line":650,"range":{"start_line":650,"start_character":4,"end_line":650,"end_character":20},"in_reply_to":"4228e2f2_836db9dc","updated":"2021-02-05 19:39:10.000000000","message":"OK. I think I resolved this correctly in patch set 13. Let me know if there is still a problem in that patch.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":647,"context_line":"        \"\"\""},{"line_number":648,"context_line":"        return redfish_utils.COMMON_PROPERTIES.copy()"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"    @base.clean_step(priority\u003d0, abortable\u003dFalse, argsinfo\u003d{"},{"line_number":651,"context_line":"        \u0027create_root_volume\u0027: {"},{"line_number":652,"context_line":"            \u0027description\u0027: ("},{"line_number":653,"context_line":"                \u0027This specifies whether to create the root volume. \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"4228e2f2_836db9dc","line":650,"range":{"start_line":650,"start_character":4,"end_line":650,"end_character":20},"in_reply_to":"e1676050_57c0fe2c","updated":"2021-01-30 00:52:12.000000000","message":"Hmm. I just copied what I saw being used for create_configuration() in the other raid impls (ilo, drac, etc.). And in those impls I do not see the @base.deploy_step annotation.\n\nI do see the @base.deploy_step annotation used on the apply_configuration() and delete_configuration() methods in those other impls, so I will add those. (I missed that originally.)\n\nShould create_configuration() have the @base.deploy_step annotation? If so, why don\u0027t the other raid impls have it?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":647,"context_line":"        \"\"\""},{"line_number":648,"context_line":"        return redfish_utils.COMMON_PROPERTIES.copy()"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"    @base.clean_step(priority\u003d0, abortable\u003dFalse, argsinfo\u003d{"},{"line_number":651,"context_line":"        \u0027create_root_volume\u0027: {"},{"line_number":652,"context_line":"            \u0027description\u0027: ("},{"line_number":653,"context_line":"                \u0027This specifies whether to create the root volume. \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"8bd78524_68907673","line":650,"in_reply_to":"f916a769_d2e19627","updated":"2021-02-05 19:39:10.000000000","message":"OK. I think I resolved this correctly in patch set 13. Let me know if there is still a problem in that patch.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        reboot_required \u003d False"},{"line_number":701,"context_line":"        raid_configs \u003d list()"},{"line_number":702,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":703,"context_line":"            raid_config \u003d dict()"},{"line_number":704,"context_line":"            resource \u003d create_virtual_disk("},{"line_number":705,"context_line":"                task,"}],"source_content_type":"text/x-python","patch_set":12,"id":"602e935d_80fd0714","line":702,"range":{"start_line":702,"start_character":8,"end_line":702,"end_character":52},"updated":"2021-01-28 19:40:31.000000000","message":"same applies to deleting - looping through and submitting all tasks async will not work with iDRAC as in iDRAC this will result in several jobs and there can be only 1 job per subsystem. It will submit first job that will start running and all the other will fail. In iDRAC UI this can be solved by adding all disks to pending operation and then in the end create 1 job to cover all disks, but there is nothing like that in Redfish. To get iDRAC work with Redfish would have to submit one job, wait till it finishes and then submit remaining 1 by 1 until done.\n\nI don\u0027t know about other vendors, but would be useful to find it out.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7e1f7b64b36d6bdb6b39460e91c865961daeb6f3","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        reboot_required \u003d False"},{"line_number":701,"context_line":"        raid_configs \u003d list()"},{"line_number":702,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":703,"context_line":"            raid_config \u003d dict()"},{"line_number":704,"context_line":"            resource \u003d create_virtual_disk("},{"line_number":705,"context_line":"                task,"}],"source_content_type":"text/x-python","patch_set":12,"id":"293554fa_4d66e4b7","line":702,"range":{"start_line":702,"start_character":8,"end_line":702,"end_character":52},"in_reply_to":"2125a32f_0647276d","updated":"2021-02-01 23:58:06.000000000","message":"Good points. Thanks for the update. I\u0027ll focus on the other issues for now.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        reboot_required \u003d False"},{"line_number":701,"context_line":"        raid_configs \u003d list()"},{"line_number":702,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":703,"context_line":"            raid_config \u003d dict()"},{"line_number":704,"context_line":"            resource \u003d create_virtual_disk("},{"line_number":705,"context_line":"                task,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9d54fed7_b8543af0","line":702,"range":{"start_line":702,"start_character":8,"end_line":702,"end_character":52},"in_reply_to":"602e935d_80fd0714","updated":"2021-01-30 00:52:12.000000000","message":"That complicates things. :-(\nThanks for pointing this out. I think you mentioned it before, but it didn\u0027t sink in.\nI\u0027ll ponder how to try to resolve.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"7666ff719a0df5c17a40a48bf7d0209b20dfc66b","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        reboot_required \u003d False"},{"line_number":701,"context_line":"        raid_configs \u003d list()"},{"line_number":702,"context_line":"        for logical_disk in logical_disks_to_create:"},{"line_number":703,"context_line":"            raid_config \u003d dict()"},{"line_number":704,"context_line":"            resource \u003d create_virtual_disk("},{"line_number":705,"context_line":"                task,"}],"source_content_type":"text/x-python","patch_set":12,"id":"2125a32f_0647276d","line":702,"range":{"start_line":702,"start_character":8,"end_line":702,"end_character":52},"in_reply_to":"9d54fed7_b8543af0","updated":"2021-02-01 18:10:32.000000000","message":"yup, and let\u0027s not concern ourselves with that for now. With current implementation, if Immediate tasks are sync (that is, ironic waits for Task to complete inside sushy before posting the next one), this shouldn\u0027t be an issue, because from surveyed controllers all but 1 support Immediate. The one that does not support, BOSS-S1, allows creating only 1 disk, so no looping.\n\nIf for some reason Immediate tasks can not remain \u0027sync\u0027 (e.g., they are running for too long), then need to return to this issue. Also need to find out if other vendors behave similarly or this is only iDRAC thing.\n\nI\u0027m resolving this for now.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":721,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d resource.resource_name"},{"line_number":722,"context_line":"            else:"},{"line_number":723,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d None"},{"line_number":724,"context_line":"            raid_configs.append(raid_config)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        driver_internal_info \u003d node.driver_internal_info"},{"line_number":727,"context_line":"        driver_internal_info[\u0027raid_configs\u0027] \u003d raid_configs"}],"source_content_type":"text/x-python","patch_set":12,"id":"d76b3702_b6eafb6f","line":724,"range":{"start_line":724,"start_character":12,"end_line":724,"end_character":44},"updated":"2021-01-28 19:40:31.000000000","message":"are sync operations added here too? Should they be there as they are done?","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":721,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d resource.resource_name"},{"line_number":722,"context_line":"            else:"},{"line_number":723,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d None"},{"line_number":724,"context_line":"            raid_configs.append(raid_config)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        driver_internal_info \u003d node.driver_internal_info"},{"line_number":727,"context_line":"        driver_internal_info[\u0027raid_configs\u0027] \u003d raid_configs"}],"source_content_type":"text/x-python","patch_set":12,"id":"dff9f617_eb803727","line":724,"range":{"start_line":724,"start_character":12,"end_line":724,"end_character":44},"in_reply_to":"965553ae_964615b2","updated":"2021-02-05 19:39:10.000000000","message":"In my comment immediately above, it should have said: \"... it is recognized that they are already done and NO action is needed.\"\n\nI decided to keep all the config operations in the list here since I pass it to post_create_configuration() and some vendor implementations may have need to see or take action on them (even the sync ones).\n\nAnd as I mentioned earlier, in _check_node_raid_config(), any operations that are no longer in progress are removed from the list.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":721,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d resource.resource_name"},{"line_number":722,"context_line":"            else:"},{"line_number":723,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d None"},{"line_number":724,"context_line":"            raid_configs.append(raid_config)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        driver_internal_info \u003d node.driver_internal_info"},{"line_number":727,"context_line":"        driver_internal_info[\u0027raid_configs\u0027] \u003d raid_configs"}],"source_content_type":"text/x-python","patch_set":12,"id":"965553ae_964615b2","line":724,"range":{"start_line":724,"start_character":12,"end_line":724,"end_character":44},"in_reply_to":"d76b3702_b6eafb6f","updated":"2021-01-30 00:52:12.000000000","message":"They do get added to the list here. But when the list is processed, it is recognized that they are already done and action is needed.\n\nBut perhaps not adding them to the list here would be better/simpler.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":733,"context_line":"            reboot\u003dreboot_required,"},{"line_number":734,"context_line":"            skip_current_step\u003dTrue,"},{"line_number":735,"context_line":"            polling\u003dTrue)"},{"line_number":736,"context_line":"        if reboot_required:"},{"line_number":737,"context_line":"            return_state \u003d deploy_utils.get_async_step_return_state(task.node)"},{"line_number":738,"context_line":"            manager_utils.node_power_action(task, states.REBOOT)"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"68fdeed7_14c46ba2","line":736,"range":{"start_line":736,"start_character":8,"end_line":736,"end_character":27},"updated":"2021-01-28 19:40:31.000000000","message":"if rebooting, need also preserve boot device to boot back into IPA, see e.g., [1]. Same for delete_configuration.\n\n[1] https://opendev.org/openstack/ironic/src/branch/master/ironic/drivers/modules/redfish/bios.py#L217-L218","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":733,"context_line":"            reboot\u003dreboot_required,"},{"line_number":734,"context_line":"            skip_current_step\u003dTrue,"},{"line_number":735,"context_line":"            polling\u003dTrue)"},{"line_number":736,"context_line":"        if reboot_required:"},{"line_number":737,"context_line":"            return_state \u003d deploy_utils.get_async_step_return_state(task.node)"},{"line_number":738,"context_line":"            manager_utils.node_power_action(task, states.REBOOT)"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bf4307aa_65e23420","line":736,"range":{"start_line":736,"start_character":8,"end_line":736,"end_character":27},"in_reply_to":"68fdeed7_14c46ba2","updated":"2021-01-30 00:52:12.000000000","message":"OK, will investigate.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":733,"context_line":"            reboot\u003dreboot_required,"},{"line_number":734,"context_line":"            skip_current_step\u003dTrue,"},{"line_number":735,"context_line":"            polling\u003dTrue)"},{"line_number":736,"context_line":"        if reboot_required:"},{"line_number":737,"context_line":"            return_state \u003d deploy_utils.get_async_step_return_state(task.node)"},{"line_number":738,"context_line":"            manager_utils.node_power_action(task, states.REBOOT)"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"eec1682d_39abbcbc","line":736,"range":{"start_line":736,"start_character":8,"end_line":736,"end_character":27},"in_reply_to":"bf4307aa_65e23420","updated":"2021-02-05 19:39:10.000000000","message":"Done","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":776,"context_line":"        for volume, controller_name in vols_to_delete:"},{"line_number":777,"context_line":"            raid_config \u003d dict()"},{"line_number":778,"context_line":"            apply_time \u003d None"},{"line_number":779,"context_line":"            apply_time_support \u003d volume.operation_apply_time_support"},{"line_number":780,"context_line":"            if (apply_time_support"},{"line_number":781,"context_line":"                    and apply_time_support.mapped_supported_values):"},{"line_number":782,"context_line":"                supported_values \u003d apply_time_support.mapped_supported_values"}],"source_content_type":"text/x-python","patch_set":12,"id":"f3c5f5a5_1748a80c","line":779,"range":{"start_line":779,"start_character":33,"end_line":779,"end_character":68},"updated":"2021-01-28 19:40:31.000000000","message":"this will always be empty, the property is on volume collection, not individual volume","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        for volume, controller_name in vols_to_delete:"},{"line_number":777,"context_line":"            raid_config \u003d dict()"},{"line_number":778,"context_line":"            apply_time \u003d None"},{"line_number":779,"context_line":"            apply_time_support \u003d volume.operation_apply_time_support"},{"line_number":780,"context_line":"            if (apply_time_support"},{"line_number":781,"context_line":"                    and apply_time_support.mapped_supported_values):"},{"line_number":782,"context_line":"                supported_values \u003d apply_time_support.mapped_supported_values"}],"source_content_type":"text/x-python","patch_set":12,"id":"c2b5070a_d28e8487","line":779,"range":{"start_line":779,"start_character":33,"end_line":779,"end_character":68},"in_reply_to":"e1aefae9_2746479c","updated":"2021-02-05 19:39:10.000000000","message":"Done","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":776,"context_line":"        for volume, controller_name in vols_to_delete:"},{"line_number":777,"context_line":"            raid_config \u003d dict()"},{"line_number":778,"context_line":"            apply_time \u003d None"},{"line_number":779,"context_line":"            apply_time_support \u003d volume.operation_apply_time_support"},{"line_number":780,"context_line":"            if (apply_time_support"},{"line_number":781,"context_line":"                    and apply_time_support.mapped_supported_values):"},{"line_number":782,"context_line":"                supported_values \u003d apply_time_support.mapped_supported_values"}],"source_content_type":"text/x-python","patch_set":12,"id":"e1aefae9_2746479c","line":779,"range":{"start_line":779,"start_character":33,"end_line":779,"end_character":68},"in_reply_to":"f3c5f5a5_1748a80c","updated":"2021-01-30 00:52:12.000000000","message":"Ah, indeed you are correct. I had misinterpreted the spec.\nWill fix.\n\n(The spec is not as clear as it could be. I brought this up in the Redfish Forum Tools Task Force call today and they are going to look into making this more clear in the spec.)","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":true,"context_lines":[{"line_number":784,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_IMMEDIATE"},{"line_number":785,"context_line":"                elif sushy.APPLY_TIME_ON_RESET in supported_values:"},{"line_number":786,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_ON_RESET"},{"line_number":787,"context_line":"            resource \u003d volume.delete_volume(apply_time\u003dapply_time)"},{"line_number":788,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027delete_volume\u0027"},{"line_number":789,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":790,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"}],"source_content_type":"text/x-python","patch_set":12,"id":"930e0c48_f83f2ce2","line":787,"range":{"start_line":787,"start_character":23,"end_line":787,"end_character":66},"updated":"2021-01-28 19:40:31.000000000","message":"should have checked this with sushy patch, but in async case this fails with \n\n  File \"/opt/stack/sushy/sushy/resources/system/storage/volume.py\", line 156, in delete_volume\n    return (TaskMonitor(self, r.headers.get(\u0027location\u0027))\n  File \"/opt/stack/sushy/sushy/resources/task_monitor.py\", line 41, in __init__\n    super(TaskMonitor, self).__init__(connector, path, redfish_version)\n  File \"/opt/stack/sushy/sushy/resources/base.py\", line 528, in __init__\n    self.refresh(json_doc\u003djson_doc)\n  File \"/opt/stack/sushy/sushy/resources/base.py\", line 599, in refresh\n    self._json \u003d self._reader.get_data().json_doc\n  File \"/opt/stack/sushy/sushy/resources/base.py\", line 405, in get_data\n    data \u003d self._conn.get(path\u003dself._path)\n  AttributeError: \u0027Volume\u0027 object has no attribute \u0027get\u0027\n\nbecause there is TaskMonitor(self, r.headers..) instead of TaskMonitor(self._conn, r.headers..), same for initialize_volume","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":true,"context_lines":[{"line_number":784,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_IMMEDIATE"},{"line_number":785,"context_line":"                elif sushy.APPLY_TIME_ON_RESET in supported_values:"},{"line_number":786,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_ON_RESET"},{"line_number":787,"context_line":"            resource \u003d volume.delete_volume(apply_time\u003dapply_time)"},{"line_number":788,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027delete_volume\u0027"},{"line_number":789,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":790,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"}],"source_content_type":"text/x-python","patch_set":12,"id":"e42c6ba0_7ed665ca","line":787,"range":{"start_line":787,"start_character":23,"end_line":787,"end_character":66},"in_reply_to":"930e0c48_f83f2ce2","updated":"2021-01-30 00:52:12.000000000","message":"Sigh. Thank you for catching this. I will fix.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"7ad27203cd575d98d3bd2380136d3c492ed31d61","unresolved":false,"context_lines":[{"line_number":784,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_IMMEDIATE"},{"line_number":785,"context_line":"                elif sushy.APPLY_TIME_ON_RESET in supported_values:"},{"line_number":786,"context_line":"                    apply_time \u003d sushy.APPLY_TIME_ON_RESET"},{"line_number":787,"context_line":"            resource \u003d volume.delete_volume(apply_time\u003dapply_time)"},{"line_number":788,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027delete_volume\u0027"},{"line_number":789,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":790,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"}],"source_content_type":"text/x-python","patch_set":12,"id":"466e0ca0_f1fd2199","line":787,"range":{"start_line":787,"start_character":23,"end_line":787,"end_character":66},"in_reply_to":"e42c6ba0_7ed665ca","updated":"2021-02-05 19:39:10.000000000","message":"Done","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"42bf89e4bc5dfab94963ed47456cf8a72f9ab0c4","unresolved":false,"context_lines":[{"line_number":814,"context_line":"        return self.post_delete_configuration("},{"line_number":815,"context_line":"            task, raid_configs, return_state\u003dreturn_state)"},{"line_number":816,"context_line":""},{"line_number":817,"context_line":"    def volume_create_error_handler(self, task, exc, volume_collection,"},{"line_number":818,"context_line":"                                    payload):"},{"line_number":819,"context_line":"        \"\"\"Handle error from failed VolumeCollection.create_volume()"},{"line_number":820,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"371b1c8f_0b21b383","line":817,"updated":"2021-01-28 19:40:31.000000000","message":"just a comment, with recent discussion around generic redfish driver and vendor implementations, not sure if this and other hooks will be needed. There is alternative being proposed to handle vendor differences, e.g., [1][2]\n\n[1] https://review.opendev.org/c/openstack/ironic/+/772899\n[2] https://review.opendev.org/c/openstack/ironic/+/772239","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"af677752e7b037d8b4fc734ef20678a57d5d3dfb","unresolved":false,"context_lines":[{"line_number":814,"context_line":"        return self.post_delete_configuration("},{"line_number":815,"context_line":"            task, raid_configs, return_state\u003dreturn_state)"},{"line_number":816,"context_line":""},{"line_number":817,"context_line":"    def volume_create_error_handler(self, task, exc, volume_collection,"},{"line_number":818,"context_line":"                                    payload):"},{"line_number":819,"context_line":"        \"\"\"Handle error from failed VolumeCollection.create_volume()"},{"line_number":820,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"85bffa17_c6ea0a41","line":817,"in_reply_to":"371b1c8f_0b21b383","updated":"2021-01-30 00:52:12.000000000","message":"Thanks for the references. I\u0027ll read them.","commit_id":"5156a38e2bfdb2f4e6c09032140516ce22358def"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"bc38f3a135fd035ed9e0bbc4891497e52613ebea","unresolved":true,"context_lines":[{"line_number":495,"context_line":"    for disk in physical_disks:"},{"line_number":496,"context_line":"        # calculate free disk space"},{"line_number":497,"context_line":"        # TODO(billdodd): this won\u0027t be true if the drive is being used;"},{"line_number":498,"context_line":"        #     any mechanism available to adjust/calculate free space?"},{"line_number":499,"context_line":"        free_space_bytes[disk] \u003d disk.capacity_bytes"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        disk_type \u003d (disk.media_type, disk.protocol, disk.capacity_bytes)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fe2da0d8_bc0ab74e","line":498,"updated":"2021-02-08 13:25:51.000000000","message":"being used by another raid set? I think all that can be done is to iterate back over it on a second pass... I think.\n\nI guess the bigger question is what risk is there here of someone doign something bad.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"e29d1fa26c807450edd8229d96fbbbab8980eaa0","unresolved":false,"context_lines":[{"line_number":495,"context_line":"    for disk in physical_disks:"},{"line_number":496,"context_line":"        # calculate free disk space"},{"line_number":497,"context_line":"        # TODO(billdodd): this won\u0027t be true if the drive is being used;"},{"line_number":498,"context_line":"        #     any mechanism available to adjust/calculate free space?"},{"line_number":499,"context_line":"        free_space_bytes[disk] \u003d disk.capacity_bytes"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        disk_type \u003d (disk.media_type, disk.protocol, disk.capacity_bytes)"}],"source_content_type":"text/x-python","patch_set":13,"id":"45d319cc_e4bb0775","line":498,"in_reply_to":"fe2da0d8_bc0ab74e","updated":"2021-02-15 02:31:00.000000000","message":"I don\u0027t think there is much risk that the drive would be being used by another raid set (or another type of volume). And from what I gather to talking to our DMTF Storage expert, there\u0027s not a good way to tell if some drive slices are allocated to one Volume and some are available to be used in a new volume. I\u0027ll update this to a NOTE to investigate in future if/when Redfish (or SNIA) better define this case.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"bc38f3a135fd035ed9e0bbc4891497e52613ebea","unresolved":true,"context_lines":[{"line_number":748,"context_line":"        logical_disks \u003d node.target_raid_config[\u0027logical_disks\u0027]"},{"line_number":749,"context_line":"        convert_drive_units(logical_disks, node)"},{"line_number":750,"context_line":"        physical_disks, disk_to_controller \u003d get_physical_disks(node)"},{"line_number":751,"context_line":"        # TODO(billdodd): filter out physical disks that are already in use?"},{"line_number":752,"context_line":"        #                 filter out disks with HotSpareType !\u003d \"None\"?"},{"line_number":753,"context_line":"        logical_disks \u003d _find_configuration(logical_disks, physical_disks,"},{"line_number":754,"context_line":"                                            disk_to_controller)"}],"source_content_type":"text/x-python","patch_set":13,"id":"889c9f48_696dfe51","line":751,"updated":"2021-02-08 13:25:51.000000000","message":"I\u0027d definitely raise an error or something in this case.  Or maybe consider it. Handling or filtering out means we\u0027re supporting it somehow if someone doesn\u0027t properly update their raid config or does it manually and then we ought to fail  with delete_existing.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"e29d1fa26c807450edd8229d96fbbbab8980eaa0","unresolved":true,"context_lines":[{"line_number":748,"context_line":"        logical_disks \u003d node.target_raid_config[\u0027logical_disks\u0027]"},{"line_number":749,"context_line":"        convert_drive_units(logical_disks, node)"},{"line_number":750,"context_line":"        physical_disks, disk_to_controller \u003d get_physical_disks(node)"},{"line_number":751,"context_line":"        # TODO(billdodd): filter out physical disks that are already in use?"},{"line_number":752,"context_line":"        #                 filter out disks with HotSpareType !\u003d \"None\"?"},{"line_number":753,"context_line":"        logical_disks \u003d _find_configuration(logical_disks, physical_disks,"},{"line_number":754,"context_line":"                                            disk_to_controller)"}],"source_content_type":"text/x-python","patch_set":13,"id":"a4e134c3_5bb9304b","line":751,"in_reply_to":"889c9f48_696dfe51","updated":"2021-02-15 02:31:00.000000000","message":"I agree it needs to be considered. Leaving this for now as a follow-up.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"e32e4cfd21293d54549e11c48b9e63b01b2976f7","unresolved":true,"context_lines":[{"line_number":774,"context_line":"                error_handler\u003dself.volume_create_error_handler)"},{"line_number":775,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027create_volume\u0027"},{"line_number":776,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d logical_disk.get(\u0027controller\u0027)"},{"line_number":777,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"},{"line_number":778,"context_line":"            if resource is not None:"},{"line_number":779,"context_line":"                # resource_type will be either \u0027volume\u0027 or \u0027task_monitor\u0027"},{"line_number":780,"context_line":"                if resource.resource_name \u003d\u003d \u0027task_monitor\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"13b35a07_49df207d","line":777,"range":{"start_line":777,"start_character":38,"end_line":777,"end_character":46},"updated":"2021-02-10 15:18:06.000000000","message":"storing here task_monitor is causing serialization issue when saving to DB. (firmware upgrade steps saved only task monitor uri)","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"18e68de662d30d77692330bf6fe07cafa8623750","unresolved":true,"context_lines":[{"line_number":774,"context_line":"                error_handler\u003dself.volume_create_error_handler)"},{"line_number":775,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027create_volume\u0027"},{"line_number":776,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d logical_disk.get(\u0027controller\u0027)"},{"line_number":777,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"},{"line_number":778,"context_line":"            if resource is not None:"},{"line_number":779,"context_line":"                # resource_type will be either \u0027volume\u0027 or \u0027task_monitor\u0027"},{"line_number":780,"context_line":"                if resource.resource_name \u003d\u003d \u0027task_monitor\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"994393ba_551d87c1","line":777,"range":{"start_line":777,"start_character":38,"end_line":777,"end_character":46},"in_reply_to":"13b35a07_49df207d","updated":"2021-02-10 17:23:15.000000000","message":"I can update this to store the TaskMonitor URI instead of the resource. This means we\u0027ll need a general way to construct a TaskMonitor instance.\n\nThe firmware update code added a get_task_monitor() method to the UpdateService class to perform this function. But since we\u0027re trying to reach a common TaskMonitor model for sushy and Ironic, I think the get_task_monitor() method should be in the System class. (Not all systems will have an UpdateService.)\n\nAija - do you think adding a get_task_monitor() method to System could be done as part of the TaskMonitor refactor?","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"416d6714642ef08973b4caa16d7d91e5c623e641","unresolved":true,"context_lines":[{"line_number":774,"context_line":"                error_handler\u003dself.volume_create_error_handler)"},{"line_number":775,"context_line":"            raid_config[\u0027operation\u0027] \u003d \u0027create_volume\u0027"},{"line_number":776,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d logical_disk.get(\u0027controller\u0027)"},{"line_number":777,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"},{"line_number":778,"context_line":"            if resource is not None:"},{"line_number":779,"context_line":"                # resource_type will be either \u0027volume\u0027 or \u0027task_monitor\u0027"},{"line_number":780,"context_line":"                if resource.resource_name \u003d\u003d \u0027task_monitor\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"911def18_a009e5da","line":777,"range":{"start_line":777,"start_character":38,"end_line":777,"end_character":46},"in_reply_to":"994393ba_551d87c1","updated":"2021-02-10 18:08:49.000000000","message":"done, see updated patch (moved to sushy root)","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"e32e4cfd21293d54549e11c48b9e63b01b2976f7","unresolved":true,"context_lines":[{"line_number":855,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":856,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"},{"line_number":857,"context_line":"            if resource is not None:"},{"line_number":858,"context_line":"                # resource_type will be either \u0027volume\u0027 or \u0027task_monitor\u0027"},{"line_number":859,"context_line":"                if resource.resource_name \u003d\u003d \u0027task_monitor\u0027:"},{"line_number":860,"context_line":"                    reboot_required \u003d True"},{"line_number":861,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d resource.resource_name"}],"source_content_type":"text/x-python","patch_set":13,"id":"661b900e_bfd4c42a","line":858,"range":{"start_line":858,"start_character":37,"end_line":858,"end_character":55},"updated":"2021-02-10 15:18:06.000000000","message":"nit: it will be either None or \u0027task_monitor\u0027 as deleting is not returning volume in any case","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"e29d1fa26c807450edd8229d96fbbbab8980eaa0","unresolved":false,"context_lines":[{"line_number":855,"context_line":"            raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":856,"context_line":"            raid_config[\u0027resource\u0027] \u003d resource"},{"line_number":857,"context_line":"            if resource is not None:"},{"line_number":858,"context_line":"                # resource_type will be either \u0027volume\u0027 or \u0027task_monitor\u0027"},{"line_number":859,"context_line":"                if resource.resource_name \u003d\u003d \u0027task_monitor\u0027:"},{"line_number":860,"context_line":"                    reboot_required \u003d True"},{"line_number":861,"context_line":"                raid_config[\u0027resource_type\u0027] \u003d resource.resource_name"}],"source_content_type":"text/x-python","patch_set":13,"id":"c41e87fe_30962328","line":858,"range":{"start_line":858,"start_character":37,"end_line":858,"end_character":55},"in_reply_to":"661b900e_bfd4c42a","updated":"2021-02-15 02:31:00.000000000","message":"Done","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"e32e4cfd21293d54549e11c48b9e63b01b2976f7","unresolved":false,"context_lines":[{"line_number":1063,"context_line":"            ext_info \u003d error.get(\u0027@Message.ExtendedInfo\u0027, [{}])"},{"line_number":1064,"context_line":"            messages \u003d [m.get(\u0027Message\u0027) for m in ext_info if \u0027Message\u0027 in m]"},{"line_number":1065,"context_line":"            if not messages and message:"},{"line_number":1066,"context_line":"                messages \u003d [message]"},{"line_number":1067,"context_line":"            return messages"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _raid_config_in_progress(self, task, raid_config):"}],"source_content_type":"text/x-python","patch_set":13,"id":"e964a86c_d8ba0da9","line":1066,"updated":"2021-02-10 15:18:06.000000000","message":"feels like sushy needs some abstraction for error to avoid doing this here in Ironic and everywhere else where errors could be returned. That is, for sync operations return either resource or error. Smth to improve later.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"e29d1fa26c807450edd8229d96fbbbab8980eaa0","unresolved":false,"context_lines":[{"line_number":1063,"context_line":"            ext_info \u003d error.get(\u0027@Message.ExtendedInfo\u0027, [{}])"},{"line_number":1064,"context_line":"            messages \u003d [m.get(\u0027Message\u0027) for m in ext_info if \u0027Message\u0027 in m]"},{"line_number":1065,"context_line":"            if not messages and message:"},{"line_number":1066,"context_line":"                messages \u003d [message]"},{"line_number":1067,"context_line":"            return messages"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _raid_config_in_progress(self, task, raid_config):"}],"source_content_type":"text/x-python","patch_set":13,"id":"f8283747_cca6515f","line":1066,"in_reply_to":"01b43bf8_8f3928cd","updated":"2021-02-15 02:31:00.000000000","message":"At least for now, I\u0027d like to leave this as-is. I\u0027m purposely only using the TaskMonitor here and not a Task. This is because the TaskMonitor is mandated by the Redfish spec for async operations and the Task being returned in the 202 response is not mandated.\n\nHence I\u0027m using the \u0027@Message.ExtendedInfo\u0027 from the failed response rather than the messages in the Task resource.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"f14a3e4501621071d2300d3b5588a3f2a08e8f6f","unresolved":false,"context_lines":[{"line_number":1063,"context_line":"            ext_info \u003d error.get(\u0027@Message.ExtendedInfo\u0027, [{}])"},{"line_number":1064,"context_line":"            messages \u003d [m.get(\u0027Message\u0027) for m in ext_info if \u0027Message\u0027 in m]"},{"line_number":1065,"context_line":"            if not messages and message:"},{"line_number":1066,"context_line":"                messages \u003d [message]"},{"line_number":1067,"context_line":"            return messages"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _raid_config_in_progress(self, task, raid_config):"}],"source_content_type":"text/x-python","patch_set":13,"id":"01b43bf8_8f3928cd","line":1066,"in_reply_to":"e964a86c_d8ba0da9","updated":"2021-02-12 12:39:37.000000000","message":"I guess this can go away with switching to refactored TaskMonitor and result handling can be similar/same as for firmware upgrade [1]\n\n[1] https://opendev.org/openstack/ironic/src/branch/master/ironic/drivers/modules/redfish/management.py#L996","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"46e40c43f6c38be5c0a101f3acb67c03e47a0091","unresolved":false,"context_lines":[{"line_number":1063,"context_line":"            ext_info \u003d error.get(\u0027@Message.ExtendedInfo\u0027, [{}])"},{"line_number":1064,"context_line":"            messages \u003d [m.get(\u0027Message\u0027) for m in ext_info if \u0027Message\u0027 in m]"},{"line_number":1065,"context_line":"            if not messages and message:"},{"line_number":1066,"context_line":"                messages \u003d [message]"},{"line_number":1067,"context_line":"            return messages"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def _raid_config_in_progress(self, task, raid_config):"}],"source_content_type":"text/x-python","patch_set":13,"id":"b21d0694_2c77a1c3","line":1066,"in_reply_to":"f8283747_cca6515f","updated":"2021-02-16 08:57:33.000000000","message":"ok for now, this is something that could be part of TaskMonitor itself on sushy side to handle both having task and error as response. Having to handle HTTP codes and JSON response on ironic side feels a bit too low level.","commit_id":"6451d37b6b75bc68b3b7cde7a2c31981b78baaee"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"46e40c43f6c38be5c0a101f3acb67c03e47a0091","unresolved":true,"context_lines":[{"line_number":852,"context_line":"            resource \u003d volume.delete(apply_time\u003dapply_time)"},{"line_number":853,"context_line":"            # only save the async tasks (task_monitors) in raid_config"},{"line_number":854,"context_line":"            if (resource is not None"},{"line_number":855,"context_line":"                    and resource.resource_name \u003d\u003d \u0027task_monitor\u0027):"},{"line_number":856,"context_line":"                raid_config[\u0027operation\u0027] \u003d \u0027delete\u0027"},{"line_number":857,"context_line":"                raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":858,"context_line":"                raid_config[\u0027task_monitor_uri\u0027] \u003d resource.path"}],"source_content_type":"text/x-python","patch_set":14,"id":"b2186470_dabcbfe5","line":855,"range":{"start_line":855,"start_character":32,"end_line":855,"end_character":46},"updated":"2021-02-16 08:57:33.000000000","message":"one thing that was not taken from resource\u0027s TaskMonitor was it inheriting from Resource, because TaskMonitor is not resource like others in Redfish spec (e.g., it\u0027s not listed in resource list), so this wouldn\u0027t work, there is no resource_name and also no path (line 858). For path there is `task_monitor_uri` for `resource_name` have to figure out another way. Thoughts?","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"5884b3a4d0b415c6ebe71fe26990d6bf8c512a64","unresolved":false,"context_lines":[{"line_number":852,"context_line":"            resource \u003d volume.delete(apply_time\u003dapply_time)"},{"line_number":853,"context_line":"            # only save the async tasks (task_monitors) in raid_config"},{"line_number":854,"context_line":"            if (resource is not None"},{"line_number":855,"context_line":"                    and resource.resource_name \u003d\u003d \u0027task_monitor\u0027):"},{"line_number":856,"context_line":"                raid_config[\u0027operation\u0027] \u003d \u0027delete\u0027"},{"line_number":857,"context_line":"                raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":858,"context_line":"                raid_config[\u0027task_monitor_uri\u0027] \u003d resource.path"}],"source_content_type":"text/x-python","patch_set":14,"id":"4957f872_e580087c","line":855,"range":{"start_line":855,"start_character":32,"end_line":855,"end_character":46},"in_reply_to":"1fb9e118_b736f201","updated":"2021-02-19 17:48:46.000000000","message":"Got it. The changes you made look good to me.","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"a23a35dccd8d3d9473a986ac1195e70686acd159","unresolved":false,"context_lines":[{"line_number":852,"context_line":"            resource \u003d volume.delete(apply_time\u003dapply_time)"},{"line_number":853,"context_line":"            # only save the async tasks (task_monitors) in raid_config"},{"line_number":854,"context_line":"            if (resource is not None"},{"line_number":855,"context_line":"                    and resource.resource_name \u003d\u003d \u0027task_monitor\u0027):"},{"line_number":856,"context_line":"                raid_config[\u0027operation\u0027] \u003d \u0027delete\u0027"},{"line_number":857,"context_line":"                raid_config[\u0027raid_controller\u0027] \u003d controller_name"},{"line_number":858,"context_line":"                raid_config[\u0027task_monitor_uri\u0027] \u003d resource.path"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fb9e118_b736f201","line":855,"range":{"start_line":855,"start_character":32,"end_line":855,"end_character":46},"in_reply_to":"b2186470_dabcbfe5","updated":"2021-02-19 11:28:51.000000000","message":"Done","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"46e40c43f6c38be5c0a101f3acb67c03e47a0091","unresolved":true,"context_lines":[{"line_number":1074,"context_line":"                     {\u0027operation\u0027: raid_config[\u0027operation\u0027],"},{"line_number":1075,"context_line":"                      \u0027node_uuid\u0027: task.node.uuid})"},{"line_number":1076,"context_line":"            return False"},{"line_number":1077,"context_line":"        if task_monitor.check_is_processing:"},{"line_number":1078,"context_line":"            LOG.debug(\u0027RAID %(operation)s task %(task_mon)s to node \u0027"},{"line_number":1079,"context_line":"                      \u0027%(node_uuid)s still in progress\u0027,"},{"line_number":1080,"context_line":"                      {\u0027operation\u0027: raid_config[\u0027operation\u0027],"}],"source_content_type":"text/x-python","patch_set":14,"id":"6805e9b8_18964b3d","line":1077,"range":{"start_line":1077,"start_character":24,"end_line":1077,"end_character":43},"updated":"2021-02-16 08:57:33.000000000","message":"here can use `is_processing` that will already have the latest status since instance was fetched on line 1069. If keeping this, then 2 requests are made on line 1069 and here. This can help if task has completed within line 1067 and 1077 that should be rare.\n\n`check_is_processing` was built for connector to loop until done for blocking requests otherwise would have to call 2 methods (`refresh` and then `is_processing`).","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"a23a35dccd8d3d9473a986ac1195e70686acd159","unresolved":false,"context_lines":[{"line_number":1074,"context_line":"                     {\u0027operation\u0027: raid_config[\u0027operation\u0027],"},{"line_number":1075,"context_line":"                      \u0027node_uuid\u0027: task.node.uuid})"},{"line_number":1076,"context_line":"            return False"},{"line_number":1077,"context_line":"        if task_monitor.check_is_processing:"},{"line_number":1078,"context_line":"            LOG.debug(\u0027RAID %(operation)s task %(task_mon)s to node \u0027"},{"line_number":1079,"context_line":"                      \u0027%(node_uuid)s still in progress\u0027,"},{"line_number":1080,"context_line":"                      {\u0027operation\u0027: raid_config[\u0027operation\u0027],"}],"source_content_type":"text/x-python","patch_set":14,"id":"e52ed253_5aefb699","line":1077,"range":{"start_line":1077,"start_character":24,"end_line":1077,"end_character":43},"in_reply_to":"6805e9b8_18964b3d","updated":"2021-02-19 11:28:51.000000000","message":"Done","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"5884b3a4d0b415c6ebe71fe26990d6bf8c512a64","unresolved":false,"context_lines":[{"line_number":1074,"context_line":"                     {\u0027operation\u0027: raid_config[\u0027operation\u0027],"},{"line_number":1075,"context_line":"                      \u0027node_uuid\u0027: task.node.uuid})"},{"line_number":1076,"context_line":"            return False"},{"line_number":1077,"context_line":"        if task_monitor.check_is_processing:"},{"line_number":1078,"context_line":"            LOG.debug(\u0027RAID %(operation)s task %(task_mon)s to node \u0027"},{"line_number":1079,"context_line":"                      \u0027%(node_uuid)s still in progress\u0027,"},{"line_number":1080,"context_line":"                      {\u0027operation\u0027: raid_config[\u0027operation\u0027],"}],"source_content_type":"text/x-python","patch_set":14,"id":"204d155a_258ff6cc","line":1077,"range":{"start_line":1077,"start_character":24,"end_line":1077,"end_character":43},"in_reply_to":"e52ed253_5aefb699","updated":"2021-02-19 17:48:46.000000000","message":"Thanks for pointing this out. I missed that the call to get_task_monitor() was doing a refresh(). But I see it now. Looks good.","commit_id":"0793604878f6b45b7dd33298137d45aa392f8893"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"682def467c97e8487a9af68d4931a228f753d0f5","unresolved":true,"context_lines":[{"line_number":979,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":980,"context_line":"                                          purpose\u003dlock_purpose,"},{"line_number":981,"context_line":"                                          shared\u003dTrue) as task:"},{"line_number":982,"context_line":"                    if not isinstance(task.driver.management, RedfishRAID):"},{"line_number":983,"context_line":"                        continue"},{"line_number":984,"context_line":""},{"line_number":985,"context_line":"                    raid_configs \u003d driver_internal_info.get("}],"source_content_type":"text/x-python","patch_set":16,"id":"d3aa4307_35b8bdc6","line":982,"range":{"start_line":982,"start_character":38,"end_line":982,"end_character":60},"updated":"2021-02-23 18:03:00.000000000","message":"should be task.driver.raid here","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"bc13d82fd1900987fff20990c99d2b8a48e864b0","unresolved":false,"context_lines":[{"line_number":979,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":980,"context_line":"                                          purpose\u003dlock_purpose,"},{"line_number":981,"context_line":"                                          shared\u003dTrue) as task:"},{"line_number":982,"context_line":"                    if not isinstance(task.driver.management, RedfishRAID):"},{"line_number":983,"context_line":"                        continue"},{"line_number":984,"context_line":""},{"line_number":985,"context_line":"                    raid_configs \u003d driver_internal_info.get("}],"source_content_type":"text/x-python","patch_set":16,"id":"ed1c22d0_4b3fbf0a","line":982,"range":{"start_line":982,"start_character":38,"end_line":982,"end_character":60},"in_reply_to":"d3aa4307_35b8bdc6","updated":"2021-02-23 22:10:11.000000000","message":"Done","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"682def467c97e8487a9af68d4931a228f753d0f5","unresolved":true,"context_lines":[{"line_number":1027,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":1028,"context_line":"                                          purpose\u003dlock_purpose,"},{"line_number":1029,"context_line":"                                          shared\u003dTrue) as task:"},{"line_number":1030,"context_line":"                    if not isinstance(task.driver.management,"},{"line_number":1031,"context_line":"                                      RedfishRAID):"},{"line_number":1032,"context_line":"                        continue"},{"line_number":1033,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"8136bb33_8646d8d4","line":1030,"range":{"start_line":1030,"start_character":38,"end_line":1030,"end_character":60},"updated":"2021-02-23 18:03:00.000000000","message":"ditto","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"bc13d82fd1900987fff20990c99d2b8a48e864b0","unresolved":false,"context_lines":[{"line_number":1027,"context_line":"                with task_manager.acquire(context, node_uuid,"},{"line_number":1028,"context_line":"                                          purpose\u003dlock_purpose,"},{"line_number":1029,"context_line":"                                          shared\u003dTrue) as task:"},{"line_number":1030,"context_line":"                    if not isinstance(task.driver.management,"},{"line_number":1031,"context_line":"                                      RedfishRAID):"},{"line_number":1032,"context_line":"                        continue"},{"line_number":1033,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"934b86d5_b3047257","line":1030,"range":{"start_line":1030,"start_character":38,"end_line":1030,"end_character":60},"in_reply_to":"8136bb33_8646d8d4","updated":"2021-02-23 22:10:11.000000000","message":"Done","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b6b9d9673ba6f8235873a4cae54727cd2782072c","unresolved":true,"context_lines":[{"line_number":157,"context_line":"    \"\"\"Calculate the drive overhead for the given RAID level"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    Drive overhead is the number of additional drives required to hold the"},{"line_number":160,"context_line":"    the redundant data needed for mirrored volumes and the parity checksums"},{"line_number":161,"context_line":"    for volumes with parity."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    :param raid_level: RAID level of the virtual disk"}],"source_content_type":"text/x-python","patch_set":19,"id":"1a7bca68_9669c13b","line":160,"range":{"start_line":160,"start_character":4,"end_line":160,"end_character":7},"updated":"2021-03-08 10:25:56.000000000","message":"nit: remove \u0027the\u0027","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b6b9d9673ba6f8235873a4cae54727cd2782072c","unresolved":true,"context_lines":[{"line_number":228,"context_line":"    :param raid_level: RAID level of the virtual disk."},{"line_number":229,"context_line":"    :param disks_count: number of physical disks used for the virtual disk."},{"line_number":230,"context_line":"    :returns: number of spans."},{"line_number":231,"context_line":"    \"\"\""},{"line_number":232,"context_line":"    if raid_level in [\u00270\u0027, \u00271\u0027, \u00275\u0027, \u00276\u0027]:"},{"line_number":233,"context_line":"        return 1"},{"line_number":234,"context_line":"    elif raid_level in [\u00275+0\u0027, \u00276+0\u0027]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a549792_5e68a2d6","line":231,"updated":"2021-03-08 10:25:56.000000000","message":"nit: miss raises","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"02d81274038927a559ad1ae6a1fcf01815467dea","unresolved":true,"context_lines":[{"line_number":972,"context_line":"        fields \u003d [\u0027driver_internal_info\u0027]"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"},{"line_number":975,"context_line":"        for (node_uuid, driver, conductor_group,"},{"line_number":976,"context_line":"             driver_internal_info) in node_list:"},{"line_number":977,"context_line":"            try:"},{"line_number":978,"context_line":"                lock_purpose \u003d \u0027checking async RAID config failed.\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"0c4cef3a_8a747795","line":975,"updated":"2021-03-08 14:39:48.000000000","message":"We should likely be checking conductor_group here.","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"7e0b3594dd6f074a589a628e13bf75bff9dd30c3","unresolved":true,"context_lines":[{"line_number":972,"context_line":"        fields \u003d [\u0027driver_internal_info\u0027]"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"},{"line_number":975,"context_line":"        for (node_uuid, driver, conductor_group,"},{"line_number":976,"context_line":"             driver_internal_info) in node_list:"},{"line_number":977,"context_line":"            try:"},{"line_number":978,"context_line":"                lock_purpose \u003d \u0027checking async RAID config failed.\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"1d529348_b886b405","line":975,"in_reply_to":"0c4cef3a_8a747795","updated":"2021-03-09 16:44:29.000000000","message":"ditto","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"02d81274038927a559ad1ae6a1fcf01815467dea","unresolved":true,"context_lines":[{"line_number":1020,"context_line":"        fields \u003d [\u0027driver_internal_info\u0027]"},{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"},{"line_number":1023,"context_line":"        for (node_uuid, driver, conductor_group,"},{"line_number":1024,"context_line":"             driver_internal_info) in node_list:"},{"line_number":1025,"context_line":"            try:"},{"line_number":1026,"context_line":"                lock_purpose \u003d \u0027checking async RAID config tasks.\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"a29255b3_04ea5e3b","line":1023,"updated":"2021-03-08 14:39:48.000000000","message":"Looks like we\u0027re not accounting for conductor group and thus looping through everything. We should consider it and ensure that we\u0027re not working on another conductor\u0027s nodes somehow. It can be changed in follow-up.","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"7e0b3594dd6f074a589a628e13bf75bff9dd30c3","unresolved":true,"context_lines":[{"line_number":1020,"context_line":"        fields \u003d [\u0027driver_internal_info\u0027]"},{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"        node_list \u003d manager.iter_nodes(fields\u003dfields, filters\u003dfilters)"},{"line_number":1023,"context_line":"        for (node_uuid, driver, conductor_group,"},{"line_number":1024,"context_line":"             driver_internal_info) in node_list:"},{"line_number":1025,"context_line":"            try:"},{"line_number":1026,"context_line":"                lock_purpose \u003d \u0027checking async RAID config tasks.\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"04258181_343944ba","line":1023,"in_reply_to":"a29255b3_04ea5e3b","updated":"2021-03-09 16:44:29.000000000","message":"isn\u0027t iter_nodes accounting for that[1]? If not, then it should be done somewhere centralized, otherwise this is the pattern used in all(most?) periodics and doing it separately every time is very repetitive. In any case if additional changes are needed, they need to be consistently applied to all periodics and scope of the change is much bigger than just these RAID periodics.\n\n[1] https://opendev.org/openstack/ironic/src/commit/4a3bbf6d28eeede0bab36cf7ed412192c2300a1f/ironic/conductor/base_manager.py#L425","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"}],"ironic/drivers/redfish.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":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    @property"},{"line_number":60,"context_line":"    def supported_raid_interfaces(self):"},{"line_number":61,"context_line":"        \"\"\"List of supported raid interfaces.\"\"\""},{"line_number":62,"context_line":"        return [redfish_raid.RedfishRAID, noop.NoRAID]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_b50377be","line":62,"updated":"2020-08-13 20:44:21.000000000","message":"Would be awesome to allow the agent to be invoked as well since adding this without incorporating what is in the generic hardware interface means that redfish users also loose functionality.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    @property"},{"line_number":60,"context_line":"    def supported_raid_interfaces(self):"},{"line_number":61,"context_line":"        \"\"\"List of supported raid interfaces.\"\"\""},{"line_number":62,"context_line":"        return [redfish_raid.RedfishRAID, noop.NoRAID]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0fc139e5","line":62,"in_reply_to":"9f560f44_b50377be","updated":"2020-08-19 23:29:44.000000000","message":"Thanks for pointing this out. Will add agent.AgentRAID.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"}],"ironic/tests/unit/drivers/modules/redfish/test_raid.py":[{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"c6c5d3fdab089add3710d3dd435538eadfcc330e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import mock"},{"line_number":16,"context_line":"from oslo_utils import importutils"},{"line_number":17,"context_line":"from oslo_utils import units"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_2ea9f823","line":15,"updated":"2020-07-17 15:55:39.000000000","message":"please don\u0027t import mock, use unittest mock","commit_id":"cae878059979fe2ed9256ad0056dd349803227ba"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"220ed71b1682d78ccce596377dea656ba042ce3b","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import mock"},{"line_number":16,"context_line":"from oslo_utils import importutils"},{"line_number":17,"context_line":"from oslo_utils import units"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_69b92aeb","line":15,"in_reply_to":"bf51134e_2ea9f823","updated":"2020-07-17 16:17:56.000000000","message":"Done","commit_id":"cae878059979fe2ed9256ad0056dd349803227ba"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"682def467c97e8487a9af68d4931a228f753d0f5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"0b3c4b18_52048fbe","updated":"2021-02-23 18:03:00.000000000","message":"should improve coverage, currently it\u0027s at 69% and there are some methods entirely missing from coverage","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"bc13d82fd1900987fff20990c99d2b8a48e864b0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"a039d95d_0395a4af","in_reply_to":"0b3c4b18_52048fbe","updated":"2021-02-23 22:10:11.000000000","message":"Agreed. I\u0027ll add this to the list for follow-up.","commit_id":"4c2c72de538973e80481b5add7477204a0e785ba"}],"releasenotes/notes/redfish-raid-interface-e362e7a0e7140962.yaml":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ec6827741cb641ed99d514ee42ba0a37471fbd43","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds ``raid`` interface to the ``redfish`` hardware type."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9f560f44_15f4e3ce","line":4,"updated":"2020-08-13 20:44:21.000000000","message":"Perhaps?\n\nAdds a ``redfish`` native ``raid_interface` to the ``redfish`` hardware type.","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"3438bafd6889e1168ad110caba71c3507446b05f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds ``raid`` interface to the ``redfish`` hardware type."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9f560f44_2ce194fd","line":4,"in_reply_to":"9f560f44_15f4e3ce","updated":"2020-08-19 23:29:44.000000000","message":"Done","commit_id":"e9feb54b2676ece3b4519aadc9dcb6396802afdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"6d6f075a91c065c37a505c777adbe1b9e06364ce","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a ``redfish`` native ``raid_interface`` to the ``redfish`` hardware type."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"967956f7_4b075e4f","line":4,"updated":"2021-03-01 14:44:34.000000000","message":"Any thoughts on adding a note, to a separate feature section just noting that the common raid cases have been tested, but highly specific raid configurations may not work as desired given the variable configuration capability which exists.","commit_id":"e64a586a18259abc0fb488cfbf55bee09c8f3d79"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"e5e159e635f7c0e0a8f58a1e8bbde04ce39de01f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a ``redfish`` native ``raid_interface`` to the ``redfish`` hardware type."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"f067e5ec_d1a49f70","line":4,"in_reply_to":"8e00d4cd_e8bfc200","updated":"2021-03-03 16:49:24.000000000","message":"Updated in patch set 19.","commit_id":"e64a586a18259abc0fb488cfbf55bee09c8f3d79"},{"author":{"_account_id":28128,"name":"Bill Dodd","email":"billdodd@gmail.com","username":"billdodd"},"change_message_id":"fdbe0b710050c5a8b3f4d9a97f08d2d595123159","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a ``redfish`` native ``raid_interface`` to the ``redfish`` hardware type."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"8e00d4cd_e8bfc200","line":4,"in_reply_to":"967956f7_4b075e4f","updated":"2021-03-02 21:10:14.000000000","message":"Good idea. I\u0027ll add a note to that effect in the next patch set (when I update the sushy version prereq).","commit_id":"e64a586a18259abc0fb488cfbf55bee09c8f3d79"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b6b9d9673ba6f8235873a4cae54727cd2782072c","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a ``redfish`` native ``raid_interface`` to the ``redfish`` hardware type."},{"line_number":5,"context_line":"  - |"},{"line_number":6,"context_line":"    Note that common RAID cases have been tested, but cases that are more complex or"},{"line_number":7,"context_line":"    rely on vendor-specific implementation details may not work as desired due to"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"82d5c92a_cfe04e1d","line":4,"updated":"2021-03-08 10:25:56.000000000","message":"nit: I believe we need to respect max 80 columns for release notes too, even if not enforced","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"02d81274038927a559ad1ae6a1fcf01815467dea","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds a ``redfish`` native ``raid_interface`` to the ``redfish`` hardware type."},{"line_number":5,"context_line":"  - |"},{"line_number":6,"context_line":"    Note that common RAID cases have been tested, but cases that are more complex or"},{"line_number":7,"context_line":"    rely on vendor-specific implementation details may not work as desired due to"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"ac92d389_4ea8133a","line":4,"in_reply_to":"82d5c92a_cfe04e1d","updated":"2021-03-08 14:39:48.000000000","message":"It is a nice to have.","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b6b9d9673ba6f8235873a4cae54727cd2782072c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"e37207ce_ff6b6acd","line":9,"updated":"2021-03-08 10:25:56.000000000","message":"nit: we should add a link to the story","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"02d81274038927a559ad1ae6a1fcf01815467dea","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"5e561b81_fc36a4b0","line":9,"in_reply_to":"e37207ce_ff6b6acd","updated":"2021-03-08 14:39:48.000000000","message":"++","commit_id":"d7f4da23764e2b8a3ce784ff2bcf047227af7064"}]}