)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"2b00f05fd6a68a3853a06eade5c967246627cabd","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Expose GPT partitioning fixing method"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As suggested by TheJulia, this code needs to be"},{"line_number":10,"context_line":"exposed and used in ironic-python-agent so that"},{"line_number":11,"context_line":"GPT partition table is properly set when image"},{"line_number":12,"context_line":"is written on disk with bigger extend."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_cdaa1618","line":9,"range":{"start_line":9,"start_character":26,"end_line":9,"end_character":35},"updated":"2018-05-22 14:17:06.000000000","message":"In the commit message the code you are referring to is not that well visible. Can you please be more explicit and may be focus on the desired change in behaviour?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"4985daa3d3fdf5b1f61a13959628bc733025629e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Expose GPT partitioning fixing method"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As suggested by TheJulia, this code needs to be"},{"line_number":10,"context_line":"exposed and used in ironic-python-agent so that"},{"line_number":11,"context_line":"GPT partition table is properly set when image"},{"line_number":12,"context_line":"is written on disk with bigger extend."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_0e69bf8a","line":9,"range":{"start_line":9,"start_character":26,"end_line":9,"end_character":35},"in_reply_to":"5f7c97a3_3f66702b","updated":"2018-06-08 16:09:13.000000000","message":"Sounds good to me! Except that I\u0027d put in the quotes `_fix_gpt_structs` for readability.\n\nBut is not it `fix_gpt_partition` function we are talking about here?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"179dada40818b58c237720be9b068043ac46b053","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Expose GPT partitioning fixing method"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As suggested by TheJulia, this code needs to be"},{"line_number":10,"context_line":"exposed and used in ironic-python-agent so that"},{"line_number":11,"context_line":"GPT partition table is properly set when image"},{"line_number":12,"context_line":"is written on disk with bigger extend."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_3f66702b","line":9,"range":{"start_line":9,"start_character":26,"end_line":9,"end_character":35},"in_reply_to":"5f7c97a3_cdaa1618","updated":"2018-06-06 07:28:22.000000000","message":"Would you rather see something like\n\nAs suggested by TheJulia, the _fix_gpt_structs code\nwhich is currently not accessible/callable from outside of ironic-lib\nshould be exposed so that other code like ironic-python-agent\ncan use it such as https://review.openstack.org/#/c/568524/\n?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"af8585556162e7f48191cf9004752d344599ce67","unresolved":false,"context_lines":[{"line_number":11,"context_line":"should be exposed so that other code like ironic-python-agent"},{"line_number":12,"context_line":"can use it such as https://review.openstack.org/#/c/568524/"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patchset now exposes this code as `fix_gpt_structs`"},{"line_number":15,"context_line":"(no leading underscore)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Icd824982d23966605fb179270bbf428cab23a41d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5f7c97a3_f2ff9c63","line":14,"updated":"2018-07-03 08:42:52.000000000","message":"well, fix_gpt_partition actually","commit_id":"a8097877830233035441aba9e0106fc5913399c2"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"0b531c8c6edacb37171add75f0303a297e88e17e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"should be exposed so that other code like ironic-python-agent"},{"line_number":12,"context_line":"can use it such as https://review.openstack.org/#/c/568524/"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patchset now exposes this code as `fix_gpt_structs`"},{"line_number":15,"context_line":"(no leading underscore)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Icd824982d23966605fb179270bbf428cab23a41d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5f7c97a3_b5d4ae4c","line":14,"in_reply_to":"5f7c97a3_f2ff9c63","updated":"2018-07-03 09:03:41.000000000","message":"Oops, good catch, I fixed it in the back ports to queens, pike and coat\nSorry about this","commit_id":"a8097877830233035441aba9e0106fc5913399c2"}],"ironic_lib/disk_utils.py":[{"author":{"_account_id":23630,"name":"Tovin Seven","email":"tovin07@gmail.com","username":"tovin07"},"change_message_id":"ee3e6a5ca9ca43e16155e93adc411536e5fea98e","unresolved":false,"context_lines":[{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device, node_uuid)"},{"line_number":737,"context_line":"        else:"},{"line_number":738,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_fad4ea49","line":736,"range":{"start_line":736,"start_character":78,"end_line":736,"end_character":81},"updated":"2018-05-15 09:03:06.000000000","message":"pep8","commit_id":"6b0b29f5a5791f8a6f1e0de7be56a124a5eaa905"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"8cd946f75aa88c36c43ee807891d5cc1831a6d61","unresolved":false,"context_lines":[{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device, node_uuid)"},{"line_number":737,"context_line":"        else:"},{"line_number":738,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":739,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_3a9d8265","line":736,"range":{"start_line":736,"start_character":78,"end_line":736,"end_character":81},"in_reply_to":"5f7c97a3_fad4ea49","updated":"2018-05-15 09:10:24.000000000","message":"Done","commit_id":"6b0b29f5a5791f8a6f1e0de7be56a124a5eaa905"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"2b00f05fd6a68a3853a06eade5c967246627cabd","unresolved":false,"context_lines":[{"line_number":722,"context_line":"def fix_gpt_partition(node_uuid, device, is_config_drive\u003dNone):"},{"line_number":723,"context_line":"    \"\"\"Fix GPT partition"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"    Exposing GPT partition fixing code for ironic-python-agent"},{"line_number":726,"context_line":"    when config drive is not used."},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"    :param node_uuid: UUID of the Node."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_edf7d258","line":725,"range":{"start_line":725,"start_character":4,"end_line":725,"end_character":38},"updated":"2018-05-22 14:17:06.000000000","message":"Could we re-phrase it somehow to make a bit clearer? I\u0027d focus on the functionality of the function rather than referring to its callers.","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"4985daa3d3fdf5b1f61a13959628bc733025629e","unresolved":false,"context_lines":[{"line_number":722,"context_line":"def fix_gpt_partition(node_uuid, device, is_config_drive\u003dNone):"},{"line_number":723,"context_line":"    \"\"\"Fix GPT partition"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"    Exposing GPT partition fixing code for ironic-python-agent"},{"line_number":726,"context_line":"    when config drive is not used."},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"    :param node_uuid: UUID of the Node."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2e97db47","line":725,"range":{"start_line":725,"start_character":4,"end_line":725,"end_character":38},"in_reply_to":"5f7c97a3_7f02a80d","updated":"2018-06-08 16:09:13.000000000","message":"Sounds good, though I\u0027d focus on the functionality of the function rather than on the change. What if:\n\n\"Fix GPT table information when image is written to a disk which has a bigger extend (e.g. 30GB image written on a 60Gb physical disk).\"\n\nIt is quite obvious that this function is not private so it can be called by external modules.","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"179dada40818b58c237720be9b068043ac46b053","unresolved":false,"context_lines":[{"line_number":722,"context_line":"def fix_gpt_partition(node_uuid, device, is_config_drive\u003dNone):"},{"line_number":723,"context_line":"    \"\"\"Fix GPT partition"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"    Exposing GPT partition fixing code for ironic-python-agent"},{"line_number":726,"context_line":"    when config drive is not used."},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"    :param node_uuid: UUID of the Node."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_7f02a80d","line":725,"range":{"start_line":725,"start_character":4,"end_line":725,"end_character":38},"in_reply_to":"5f7c97a3_edf7d258","updated":"2018-06-06 07:28:22.000000000","message":"GPT partition code which fixes the GPT table informations when image is written to a disk which has a bigger extend (ex. 30GB image written on a 60Gb physical disk).  This code can now be called by external modules/code","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"2b00f05fd6a68a3853a06eade5c967246627cabd","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    :raises: InstanceDeployFailure if exception is caught."},{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_0d032e44","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"updated":"2018-05-22 14:17:06.000000000","message":"Do we have to branch based on `is_config_drive` equals `None` or `False`? If that is the case, then I\u0027d really be looking for a plausible explanation in the comment why we resort to such a fragile approach.","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"179dada40818b58c237720be9b068043ac46b053","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    :raises: InstanceDeployFailure if exception is caught."},{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_bf5a804d","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"in_reply_to":"5f7c97a3_0d032e44","updated":"2018-06-06 07:28:22.000000000","message":"I am not sure what you really mean here and what makes you think this is fragile.\nWith my current knowledge, I just tried to implement something which would not\nbreak the existing workflow but I may definitely be wrong or used non proper way to do it","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"5dd840f5da52d38d54b270b7fe4043de1a4b9b1f","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    :raises: InstanceDeployFailure if exception is caught."},{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8c185bd3","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"in_reply_to":"5f7c97a3_4ea2579c","updated":"2018-06-11 10:55:19.000000000","message":"Ok I see you point here. However you have to consider the following assessment I am making. When code is called without prior knowledge\nis_config_drive is set to None which implies the code needs to determine whether or not a config drive is used. In the other case where the calling code knows about this it has properly set is_config_drive to True or False accordingly.\nThis being said, moving the is None test to a not is test will definitely not produce the same behaviour AFAIK","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"6fab1a6764054eb1d23eb4171d8db72c527644a6","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    :raises: InstanceDeployFailure if exception is caught."},{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_3edc7643","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"in_reply_to":"5f7c97a3_8c185bd3","updated":"2018-06-14 12:11:41.000000000","message":"In other words, the `is_config_drive` variable has three distinctive states - None, True and False.\n\nPerhaps an explicit comment on this in the code could be helpful?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"4985daa3d3fdf5b1f61a13959628bc733025629e","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    :raises: InstanceDeployFailure if exception is caught."},{"line_number":733,"context_line":"    \"\"\""},{"line_number":734,"context_line":"    try:"},{"line_number":735,"context_line":"        if is_config_drive is None:"},{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_4ea2579c","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"in_reply_to":"5f7c97a3_bf5a804d","updated":"2018-06-08 16:09:13.000000000","message":"I mean, could we just do:\n\n    if not is_config_drive:\n        ...\n\nOr this would break something? Between `if var is None` versus `if not var` the latter looks clearer and more Pythonic.","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"2b00f05fd6a68a3853a06eade5c967246627cabd","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"},{"line_number":739,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":740,"context_line":""},{"line_number":741,"context_line":"        if disk_is_gpt_partitioned:"},{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8dc65e16","line":739,"range":{"start_line":739,"start_character":38,"end_line":739,"end_character":53},"updated":"2018-05-22 14:17:06.000000000","message":"To me, the code would be easier to read if it would less indirect e.g.:\n\n    disk_is_gpt_partitioned \u003d True","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"6fab1a6764054eb1d23eb4171d8db72c527644a6","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"},{"line_number":739,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":740,"context_line":""},{"line_number":741,"context_line":"        if disk_is_gpt_partitioned:"},{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_fee11efd","line":739,"range":{"start_line":739,"start_character":38,"end_line":739,"end_character":53},"in_reply_to":"5f7c97a3_2c090783","updated":"2018-06-14 12:11:41.000000000","message":"Done","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"4985daa3d3fdf5b1f61a13959628bc733025629e","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"},{"line_number":739,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":740,"context_line":""},{"line_number":741,"context_line":"        if disk_is_gpt_partitioned:"},{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8eadcf85","line":739,"range":{"start_line":739,"start_character":38,"end_line":739,"end_character":53},"in_reply_to":"5f7c97a3_7a3076e9","updated":"2018-06-08 16:09:13.000000000","message":"I mean here you assign the value of `is_config_drive` because you checked elsewhere that it evaluates to `True`. So why not just assign `True` right away?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"5dd840f5da52d38d54b270b7fe4043de1a4b9b1f","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"},{"line_number":739,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":740,"context_line":""},{"line_number":741,"context_line":"        if disk_is_gpt_partitioned:"},{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2c090783","line":739,"range":{"start_line":739,"start_character":38,"end_line":739,"end_character":53},"in_reply_to":"5f7c97a3_7a3076e9","updated":"2018-06-11 10:55:19.000000000","message":"See explanation on previous comment above therefore it should stay the way it is","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"179dada40818b58c237720be9b068043ac46b053","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            disk_is_gpt_partitioned \u003d _is_disk_gpt_partitioned(device,"},{"line_number":737,"context_line":"                                                               node_uuid)"},{"line_number":738,"context_line":"        else:"},{"line_number":739,"context_line":"            disk_is_gpt_partitioned \u003d is_config_drive"},{"line_number":740,"context_line":""},{"line_number":741,"context_line":"        if disk_is_gpt_partitioned:"},{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_7a3076e9","line":739,"range":{"start_line":739,"start_character":38,"end_line":739,"end_character":53},"in_reply_to":"5f7c97a3_8dc65e16","updated":"2018-06-06 07:28:22.000000000","message":"Will need to look back at this in details to understand your call here","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"2b00f05fd6a68a3853a06eade5c967246627cabd","unresolved":false,"context_lines":[{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"},{"line_number":743,"context_line":"            if is_config_drive:"},{"line_number":744,"context_line":"                create_option \u003d \u00270:-%dMB:0\u0027 % MAX_CONFIG_DRIVE_SIZE_MB"},{"line_number":745,"context_line":"                utils.execute(\u0027sgdisk\u0027, \u0027-n\u0027, create_option, device,"},{"line_number":746,"context_line":"                              run_as_root\u003dTrue)"},{"line_number":747,"context_line":"    except Exception as e:"},{"line_number":748,"context_line":"        msg \u003d (_(\u0027Failed to fix GPT partition on disk %(disk)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_6dd3c2d6","line":745,"range":{"start_line":745,"start_character":16,"end_line":745,"end_character":29},"updated":"2018-05-22 14:17:06.000000000","message":"Is it deliberate not to check outcome of the call?","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"179dada40818b58c237720be9b068043ac46b053","unresolved":false,"context_lines":[{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"},{"line_number":743,"context_line":"            if is_config_drive:"},{"line_number":744,"context_line":"                create_option \u003d \u00270:-%dMB:0\u0027 % MAX_CONFIG_DRIVE_SIZE_MB"},{"line_number":745,"context_line":"                utils.execute(\u0027sgdisk\u0027, \u0027-n\u0027, create_option, device,"},{"line_number":746,"context_line":"                              run_as_root\u003dTrue)"},{"line_number":747,"context_line":"    except Exception as e:"},{"line_number":748,"context_line":"        msg \u003d (_(\u0027Failed to fix GPT partition on disk %(disk)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_ba0dae36","line":745,"range":{"start_line":745,"start_character":16,"end_line":745,"end_character":29},"in_reply_to":"5f7c97a3_6dd3c2d6","updated":"2018-06-06 07:28:22.000000000","message":"AFAIK, this was not tested either before and from what I saw it is supposed to raise exception if some failure occurs","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"4985daa3d3fdf5b1f61a13959628bc733025629e","unresolved":false,"context_lines":[{"line_number":742,"context_line":"            _fix_gpt_structs(device, node_uuid)"},{"line_number":743,"context_line":"            if is_config_drive:"},{"line_number":744,"context_line":"                create_option \u003d \u00270:-%dMB:0\u0027 % MAX_CONFIG_DRIVE_SIZE_MB"},{"line_number":745,"context_line":"                utils.execute(\u0027sgdisk\u0027, \u0027-n\u0027, create_option, device,"},{"line_number":746,"context_line":"                              run_as_root\u003dTrue)"},{"line_number":747,"context_line":"    except Exception as e:"},{"line_number":748,"context_line":"        msg \u003d (_(\u0027Failed to fix GPT partition on disk %(disk)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2ebefbad","line":745,"range":{"start_line":745,"start_character":16,"end_line":745,"end_character":29},"in_reply_to":"5f7c97a3_ba0dae36","updated":"2018-06-08 16:09:13.000000000","message":"Ah, OK then!","commit_id":"ef7979b6411edcfdcca0956f1a5f8cdb3f4e0004"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"a5105b97d755735d7af3173ca83922732d6743b5","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        else:"},{"line_number":792,"context_line":"            cur_parts \u003d set(part[\u0027number\u0027] for part in list_partitions(device))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            if _is_disk_gpt_partitioned(device, node_uuid):"},{"line_number":795,"context_line":"                fix_gpt_partition(node_uuid, device, True)"},{"line_number":796,"context_line":"            else:"},{"line_number":797,"context_line":"                # Check if the disk has 4 partitions. The MBR based disk"},{"line_number":798,"context_line":"                # cannot have more than 4 partitions."}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_413ce591","line":795,"range":{"start_line":794,"start_character":0,"end_line":795,"end_character":58},"updated":"2018-06-27 19:46:39.000000000","message":"I believe we need to copy these lines to around line 790 because if the drive already has aconfig drive, we ought to still fix the partition...\n\nWe also likely need to add some unit tests :(","commit_id":"fe7c5551e6b68a92eeaf0684b610a8ac21049ac6"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"215b10e377f0ff5466c61fc64d900a84946d4c3f","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        else:"},{"line_number":792,"context_line":"            cur_parts \u003d set(part[\u0027number\u0027] for part in list_partitions(device))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            if _is_disk_gpt_partitioned(device, node_uuid):"},{"line_number":795,"context_line":"                fix_gpt_partition(node_uuid, device, True)"},{"line_number":796,"context_line":"            else:"},{"line_number":797,"context_line":"                # Check if the disk has 4 partitions. The MBR based disk"},{"line_number":798,"context_line":"                # cannot have more than 4 partitions."}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_e27078a7","line":795,"range":{"start_line":794,"start_character":0,"end_line":795,"end_character":58},"in_reply_to":"5f7c97a3_3f5f6b10","updated":"2018-06-28 16:48:20.000000000","message":"I agree with Julia here. This code is just going to be called when there is a config drive, but we can have gpt partitions that need fixing, that are not going to need a config drive.\nSo the proposal should be splitting the method in 2: fix_gpt_partitions and create_config_drive_partition.\nThen, from the deploy_disk_image from ironic, we could call fix_gpt_partitions without depending on configdrive:\n\ndef deploy_disk_image(..):\n   ...\n   disk_utils.fix_gpt_partitions\n   if configdrive:\n       disk_utils.create_config_drive_partition","commit_id":"fe7c5551e6b68a92eeaf0684b610a8ac21049ac6"},{"author":{"_account_id":19578,"name":"Olivier Bourdon","email":"olivierbourdon38@gmail.com","username":"olivierbourdon38"},"change_message_id":"4f02468a92196d1192788005d874f977a9e8ec68","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        else:"},{"line_number":792,"context_line":"            cur_parts \u003d set(part[\u0027number\u0027] for part in list_partitions(device))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            if _is_disk_gpt_partitioned(device, node_uuid):"},{"line_number":795,"context_line":"                fix_gpt_partition(node_uuid, device, True)"},{"line_number":796,"context_line":"            else:"},{"line_number":797,"context_line":"                # Check if the disk has 4 partitions. The MBR based disk"},{"line_number":798,"context_line":"                # cannot have more than 4 partitions."}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_f7404aa5","line":795,"range":{"start_line":794,"start_character":0,"end_line":795,"end_character":58},"in_reply_to":"5f7c97a3_413ce591","updated":"2018-06-28 06:44:29.000000000","message":"As for the unit test, no problem, will try to have a look later today on how to do this\nAs for the copy, I understand but I am wondering why it should be done as it was not done previously \nThanks a lot for the reviews anyways","commit_id":"fe7c5551e6b68a92eeaf0684b610a8ac21049ac6"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5d5ef11c0c9ab9b8db7fad0107b0148fbe442330","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        else:"},{"line_number":792,"context_line":"            cur_parts \u003d set(part[\u0027number\u0027] for part in list_partitions(device))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"            if _is_disk_gpt_partitioned(device, node_uuid):"},{"line_number":795,"context_line":"                fix_gpt_partition(node_uuid, device, True)"},{"line_number":796,"context_line":"            else:"},{"line_number":797,"context_line":"                # Check if the disk has 4 partitions. The MBR based disk"},{"line_number":798,"context_line":"                # cannot have more than 4 partitions."}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_3f5f6b10","line":795,"range":{"start_line":794,"start_character":0,"end_line":795,"end_character":58},"in_reply_to":"5f7c97a3_f7404aa5","updated":"2018-06-28 16:14:18.000000000","message":"Awesome!\n\nAs for the copy, there is the possibility and the not common but not unheard of behavior of burning the configuration drive partition into the image being written. If our logic expects the configuration drive to already be present and doesn\u0027t fix the partition... then the partitioning information could still be wrong for the server if it was done with GPT. I know it seems like I\u0027m thinking of a future edge case, but I think the NFV folks do that presently with bios boot mode... which wouldn\u0027t have raised this error yet.","commit_id":"fe7c5551e6b68a92eeaf0684b610a8ac21049ac6"}]}