)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8e9ce9a4a84ac5d4102b6cafc567a07e44aa543","unresolved":false,"context_lines":[{"line_number":23,"context_line":"ResourceTracker._init_compute_node code will fail to create the"},{"line_number":24,"context_line":"ComputeNode record again because of the duplicate uuid."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Rather than try to made the ComputeNode.destroy conditionally"},{"line_number":27,"context_line":"do a hard-delete of the DB record if the hypervisor_type is"},{"line_number":28,"context_line":"ironic, which might be a mess for backports, just simply revert"},{"line_number":29,"context_line":"the original change that caused the regression since it was only"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_982230ba","line":26,"range":{"start_line":26,"start_character":19,"end_line":26,"end_character":23},"updated":"2019-08-09 18:22:01.000000000","message":"make","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bd7f7c483d4ec561cdf9a40a0a34df5f84bc05d2","unresolved":false,"context_lines":[{"line_number":30,"context_line":"for serviceability/debugging purposes (link the ironic node uuid"},{"line_number":31,"context_line":"to the compute node uuid to the resource provider id). Losing that"},{"line_number":32,"context_line":"linkage sucks, but if we\u0027re going to do it we need to deal with"},{"line_number":33,"context_line":"this duplicate uuid issue in the compute_nodes table first."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Change-Id: Iafba419fe86446ffe636721f523fb619f8f787b3"},{"line_number":36,"context_line":"Closes-Bug: #1839560"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_78541422","line":33,"updated":"2019-08-09 18:16:55.000000000","message":"Another alternative which would avoid messing with hard-deleting the record, is if on ComputeNode.create, we handle DuplicateEntry and if we hit that, we look for a (soft) deleted compute node with the same uuid and if found, we just update that thing and set deleted\u003d0, so it\u0027s not really created, but it\u0027s kind of the same idea.","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"174e370fd8ed6ef82186513b85213661a6021295","unresolved":false,"context_lines":[{"line_number":30,"context_line":"for serviceability/debugging purposes (link the ironic node uuid"},{"line_number":31,"context_line":"to the compute node uuid to the resource provider id). Losing that"},{"line_number":32,"context_line":"linkage sucks, but if we\u0027re going to do it we need to deal with"},{"line_number":33,"context_line":"this duplicate uuid issue in the compute_nodes table first."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Change-Id: Iafba419fe86446ffe636721f523fb619f8f787b3"},{"line_number":36,"context_line":"Closes-Bug: #1839560"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_23fdef3a","line":33,"in_reply_to":"7faddb67_69c5163d","updated":"2019-08-12 18:41:58.000000000","message":"Done","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"37ab87893f62c8e154feb72673b31aad9f9c1dc8","unresolved":false,"context_lines":[{"line_number":30,"context_line":"for serviceability/debugging purposes (link the ironic node uuid"},{"line_number":31,"context_line":"to the compute node uuid to the resource provider id). Losing that"},{"line_number":32,"context_line":"linkage sucks, but if we\u0027re going to do it we need to deal with"},{"line_number":33,"context_line":"this duplicate uuid issue in the compute_nodes table first."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Change-Id: Iafba419fe86446ffe636721f523fb619f8f787b3"},{"line_number":36,"context_line":"Closes-Bug: #1839560"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_69c5163d","line":33,"in_reply_to":"7faddb67_73a755a9","updated":"2019-08-12 09:51:28.000000000","message":"\u003e \u003e Yeah, I was going to suggest this. Technically we could encounter\n \u003e \u003e this somehow with another hypervisor driver if something went\n \u003e \u003e really wrong, we are recovering from an old host or something\n \u003e like\n \u003e \u003e that. So just handling this case and un-delete-ing the record\n \u003e seems\n \u003e \u003e like a better fit. I definitely don\u0027t want to make something\n \u003e \u003e ironic-specific.\n \u003e \u003e\n \u003e \u003e The constraint change is also fine, but since it\u0027s already in\n \u003e place\n \u003e \u003e I think I\u0027d rather do the DuplicateEntry way to avoid having to\n \u003e \u003e deal with the impact of changing the schema.\n \u003e \n \u003e Would you rather I change the fix to be that then, instead of this\n \u003e revert? If so, I\u0027ll probably invest some time into writing a\n \u003e functional regression test so we\u0027re not just relying on unit tests.\n\n+1 to handling the duplicate entry and just reverting the \"deleted\" flag if soft-deleted entry is found. That way we can still have the linkage which would make the operator life and debugging much easier.","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8e9ce9a4a84ac5d4102b6cafc567a07e44aa543","unresolved":false,"context_lines":[{"line_number":30,"context_line":"for serviceability/debugging purposes (link the ironic node uuid"},{"line_number":31,"context_line":"to the compute node uuid to the resource provider id). Losing that"},{"line_number":32,"context_line":"linkage sucks, but if we\u0027re going to do it we need to deal with"},{"line_number":33,"context_line":"this duplicate uuid issue in the compute_nodes table first."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Change-Id: Iafba419fe86446ffe636721f523fb619f8f787b3"},{"line_number":36,"context_line":"Closes-Bug: #1839560"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_b30dcd36","line":33,"in_reply_to":"7faddb67_78541422","updated":"2019-08-09 18:22:01.000000000","message":"Yeah, I was going to suggest this. Technically we could encounter this somehow with another hypervisor driver if something went really wrong, we are recovering from an old host or something like that. So just handling this case and un-delete-ing the record seems like a better fit. I definitely don\u0027t want to make something ironic-specific.\n\nThe constraint change is also fine, but since it\u0027s already in place I think I\u0027d rather do the DuplicateEntry way to avoid having to deal with the impact of changing the schema.","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bdfefd2576311e86be19190c058c6b2cd6f7a9e1","unresolved":false,"context_lines":[{"line_number":30,"context_line":"for serviceability/debugging purposes (link the ironic node uuid"},{"line_number":31,"context_line":"to the compute node uuid to the resource provider id). Losing that"},{"line_number":32,"context_line":"linkage sucks, but if we\u0027re going to do it we need to deal with"},{"line_number":33,"context_line":"this duplicate uuid issue in the compute_nodes table first."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Change-Id: Iafba419fe86446ffe636721f523fb619f8f787b3"},{"line_number":36,"context_line":"Closes-Bug: #1839560"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_73a755a9","line":33,"in_reply_to":"7faddb67_b30dcd36","updated":"2019-08-09 18:52:53.000000000","message":"\u003e Yeah, I was going to suggest this. Technically we could encounter\n \u003e this somehow with another hypervisor driver if something went\n \u003e really wrong, we are recovering from an old host or something like\n \u003e that. So just handling this case and un-delete-ing the record seems\n \u003e like a better fit. I definitely don\u0027t want to make something\n \u003e ironic-specific.\n \u003e \n \u003e The constraint change is also fine, but since it\u0027s already in place\n \u003e I think I\u0027d rather do the DuplicateEntry way to avoid having to\n \u003e deal with the impact of changing the schema.\n\nWould you rather I change the fix to be that then, instead of this revert? If so, I\u0027ll probably invest some time into writing a functional regression test so we\u0027re not just relying on unit tests.","commit_id":"ccd1c8a53a1afda57bd4821b0ac92eab8b598a68"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c785daca3a4521daa263573cb7ec40114ebfdc9c","unresolved":false,"context_lines":[{"line_number":736,"context_line":"        and updated, None otherwise"},{"line_number":737,"context_line":"    \"\"\""},{"line_number":738,"context_line":"    cn \u003d model_query("},{"line_number":739,"context_line":"        context, models.ComputeNode).filter_by(uuid\u003dvalues[\u0027uuid\u0027]).first()"},{"line_number":740,"context_line":"    if cn:"},{"line_number":741,"context_line":"        # Update with the provided values but un-soft-delete."},{"line_number":742,"context_line":"        update_values \u003d copy.deepcopy(values)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_3b898f25","line":739,"range":{"start_line":739,"start_character":17,"end_line":739,"end_character":35},"updated":"2019-08-13 09:48:49.000000000","message":"nit: you seem to only need an ID, so you can make it more efficient by only requesting it from the database.","commit_id":"8b007266f438ec0a5a797d05731cce6f2b155f4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5b03e1cb9897240ea2e1c1d798a882b09f00dc6","unresolved":false,"context_lines":[{"line_number":736,"context_line":"        and updated, None otherwise"},{"line_number":737,"context_line":"    \"\"\""},{"line_number":738,"context_line":"    cn \u003d model_query("},{"line_number":739,"context_line":"        context, models.ComputeNode).filter_by(uuid\u003dvalues[\u0027uuid\u0027]).first()"},{"line_number":740,"context_line":"    if cn:"},{"line_number":741,"context_line":"        # Update with the provided values but un-soft-delete."},{"line_number":742,"context_line":"        update_values \u003d copy.deepcopy(values)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_5e7f8eaa","line":739,"range":{"start_line":739,"start_character":17,"end_line":739,"end_character":35},"in_reply_to":"7faddb67_3b898f25","updated":"2019-08-13 19:42:30.000000000","message":"True, but I\u0027m not sure I can do that with model_query, I\u0027d have to execute my own select() ORM, which is fine, just not something I thought about since model_query is used extensively.","commit_id":"8b007266f438ec0a5a797d05731cce6f2b155f4c"}]}