)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_6556d2d0","line":15,"updated":"2020-02-18 11:54:01.000000000","message":"How can you call resize with the same flavor? \n\nstack@aio:~$ openstack server resize vm1 --flavor c1\nWhen resizing, instances must change flavor! (HTTP 400) (Request-ID: req-75e8a4de-1734-41bb-9419-91d7c88da0d0)","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aaa8ee171c3a4e9f38557025423a123a7b4e5bd1","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_76ea030e","line":15,"in_reply_to":"3fa7e38b_1d1092f4","updated":"2020-02-18 14:45:42.000000000","message":"I would have assumed Sundar meant \"cold migration\" here instead of \"resize with same flavor\". I\u0027m guessing that works because we keep the ARQs that we have, but won\u0027t request a new one and change what the instance has currently if the flavor is different.\n\nHowever, if resize doesn\u0027t work. I agree that we need to mark it as unsupported/blocked until it does.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4baa6b73eead3aba2b300d8324bff954f0ad1d89","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_ebb933c5","line":15,"in_reply_to":"3fa7e38b_6556d2d0","updated":"2020-02-18 12:04:31.000000000","message":"you cant i test this and it does not work\n\nmaybe sundar was confused with rebuild or cold migrate(which does resize with same flavor internally)?\n\nwhen i tested resize with the fake driver the operation completed but the arqs and allocation where not updated correctly. that said that was code form a week ago so it might be fixed.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_ed5cab1c","line":15,"in_reply_to":"3fa7e38b_76ea030e","updated":"2020-02-21 11:51:51.000000000","message":"Guys, not sure if you noticed \"but are no-ops\". It doesn\u0027t do anything. In this case, it gives a message and quits. As opposed to other ops which may crash or leak resources, unless explicitly forbidden.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ee772a8311094a62a590f30211396e6167e0a297","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_1d1092f4","line":15,"in_reply_to":"3fa7e38b_ab27bb1d","updated":"2020-02-18 12:41:06.000000000","message":"If resize / cold-migrate does not work with instances having ARQs then I think we should reject such operations instead of complete them and loosing the ARQ in the process.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a77300c5be3c6504799333213357000e86db71b9","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_ab27bb1d","line":15,"in_reply_to":"3fa7e38b_ebb933c5","updated":"2020-02-18 12:13:53.000000000","message":"these where the test result on the last version i tested\n\nhttp://paste.openstack.org/show/789306/\ni dont recall exactly which version that was (it was 3-5 revisions back i think) as i have had a hardware failure on that server so i have created a new set of test vms on my laptop since then.\n\nnot i marked hard reboot and suspped as works but i can fully validate those with the fake driver as i cant assert if the device would be correctly reatttached in those cases since the fake driver does not modify the xml.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"},{"line_number":19,"context_line":"forces a hard reboot in the libvirt driver."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Suspend gives a generic error unrelated to this patch:"},{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_e5f642d6","line":21,"range":{"start_line":21,"start_character":30,"end_line":21,"end_character":39},"updated":"2020-02-18 11:54:01.000000000","message":"pre-existing but not unrelated. I think we don\u0027t support suspend with PCI devices and accelerators are also PCI devices.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"},{"line_number":19,"context_line":"forces a hard reboot in the libvirt driver."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Suspend gives a generic error unrelated to this patch:"},{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_a2a007f4","line":21,"range":{"start_line":21,"start_character":30,"end_line":21,"end_character":39},"in_reply_to":"3fa7e38b_ab5edb75","updated":"2020-02-21 11:51:51.000000000","message":"Ok. Let\u0027s block the suspend operation for now.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a77300c5be3c6504799333213357000e86db71b9","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"},{"line_number":19,"context_line":"forces a hard reboot in the libvirt driver."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Suspend gives a generic error unrelated to this patch:"},{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_ab5edb75","line":21,"range":{"start_line":21,"start_character":30,"end_line":21,"end_character":39},"in_reply_to":"3fa7e38b_e5f642d6","updated":"2020-02-18 12:13:53.000000000","message":"we do.\nin the libvirt dirver when we do suspend we loop over all the pci devices and detach them then call libvirts managed save function to suspend to disk. on resume we then attach the pci device again after the vm resumes.\n\nso from a guest point of view the see the device hot plugged.\n\n\nfor pause and unpasue where we jsut stop the execution of the guest cpus we do not need to remove the devices.\n\nthe error below is expect because you have not updated the suspend code to handel the detaching and reattching of the cyborg devices so this is a bug.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_e50f22d2","line":25,"updated":"2020-02-18 11:54:01.000000000","message":"What will be the result of calling shelve on an instance with accelerators? Do we return a proper error? Do we leak allocations or bound ARQs when we error out?","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_e25b5ff5","line":25,"in_reply_to":"3fa7e38b_3ddeae3d","updated":"2020-02-21 11:51:51.000000000","message":"Yes, the block-ops patch (to be added later in this series) will address it.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ee772a8311094a62a590f30211396e6167e0a297","unresolved":false,"context_lines":[{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_3ddeae3d","line":25,"in_reply_to":"3fa7e38b_cb46b7a9","updated":"2020-02-18 12:41:06.000000000","message":"Then I think we should reject the shelve operation if the instance has an ARQ as silently loosing the ARQ is a really bad user experience","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a77300c5be3c6504799333213357000e86db71b9","unresolved":false,"context_lines":[{"line_number":22,"context_line":"   libvirtError: Requested operation is not valid:"},{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_cb46b7a9","line":25,"in_reply_to":"3fa7e38b_e50f22d2","updated":"2020-02-18 12:13:53.000000000","message":"currntly it shevles the instance nad does not free the placement allcoations and whne you unshelv it does not have an fpga anymore.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"},{"line_number":29,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_45d4f61e","line":26,"updated":"2020-02-18 11:54:01.000000000","message":"ditto, what will happen? Do we error out gracefully?","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a77300c5be3c6504799333213357000e86db71b9","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"},{"line_number":29,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_4bc8871f","line":26,"in_reply_to":"3fa7e38b_45d4f61e","updated":"2020-02-18 12:13:53.000000000","message":"libvirt should reject it but currently we do not prevnet it. so it will fail when we call migrate.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ee772a8311094a62a590f30211396e6167e0a297","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"},{"line_number":29,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_bdb37e0c","line":26,"in_reply_to":"3fa7e38b_4bc8871f","updated":"2020-02-18 12:41:06.000000000","message":"If it fails gracefully (not loosing anything in the process) then I\u0027m fine with this.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"},{"line_number":29,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_62676fbb","line":26,"in_reply_to":"3fa7e38b_9de6c2af","updated":"2020-02-21 11:51:51.000000000","message":"The block-ops patch (to be added later in this series) will block it.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9190345ca0fed2eba8b1a253e06ecacae45e7cf5","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   domain has assigned non-USB host devices"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Shelve is not supported yet."},{"line_number":26,"context_line":"Live migration is not intended to be supported with accelerators now."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: Icb95890d8f16cad1f7dc18487a48def2f7c9aec2"},{"line_number":29,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"3fa7e38b_9de6c2af","line":26,"in_reply_to":"3fa7e38b_bdb37e0c","updated":"2020-02-18 13:06:43.000000000","message":"basically when we call libvirt it will bitch saying you cannoth migrate if a domian conatins hostsdevs that are not of type usb. so we will catch the error and we should roll back the migration leaving the vm in the active state and the migration should be marked in error in the event log.\n\nthis is what happens if you try to live migrate with a pci device before we added a check in the conductor for this when we added sriov live migration support.\n\nso if you have a stien or older openstack and live migrate a vm with a sriov or pci device then it will fail gracefully this way. its a little wastful in that we have already configured networking an a bunch of things on the host in pre live migrate which we then undo in the revert but it should not casue data lose or result in the vm entering the error state.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":45,"id":"1fa4df85_36018607","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":25},"updated":"2020-03-19 12:28:08.000000000","message":"I guess you mean cold migrate","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4acb26725b90b66eacd831d8d04f96091b052713","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":45,"id":"df33271e_fee5ef0b","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":25},"in_reply_to":"1fa4df85_0b43f314","updated":"2020-03-23 11:15:39.000000000","message":"$ openstack server resize \u003cserver-id\u003e  seems like a client side no-op regardless of any nova feature. It only calls GETs on nova\n\nstack@aio:~$ openstack --debug  server resize 4ab3fca0-f155-4489-8221-b9f16cb3d92d 2\u003e\u00261| grep curl\nREQ: curl -g -i -X GET http://192.168.121.129/identity -H \"Accept: application/json\" -H \"User-Agent: openstacksdk/0.40.0 keystoneauth1/3.18.0 python-requests/2.22.0 CPython/3.6.9\"\nREQ: curl -g -i -X GET http://192.168.121.129/compute/v2.1/servers/4ab3fca0-f155-4489-8221-b9f16cb3d92d -H \"Accept: application/json\" -H \"User-Agent: python-novaclient\" -H \"X-Auth-Token: {SHA256}2f4f14c3ada49b326059a0406dfb8cb8c34c6df2a8452d1252d3e1e9b31f50ef\" -H \"X-OpenStack-Nova-API-Version: 2.1\"\n\n\nWhat operation do you want to trigger here? As there is no such operations as resize with same flavor.\n\nstack@aio:~$ openstack server resize 4ab3fca0-f155-4489-8221-b9f16cb3d92d --flavor c1\nWhen resizing, instances must change flavor! (HTTP 400) (Request-ID: req-f06df0aa-d544-4ebb-ada0-89c62d3c52bb)","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":45,"id":"1fa4df85_0b43f314","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":25},"in_reply_to":"1fa4df85_36018607","updated":"2020-03-22 01:28:06.000000000","message":"I mean that this operation works as a no-op:\n$ openstack server resize \u003cserver-id\u003e \n\nThe n-api logs show this output:\n\n Mar 19 19:33:28 otcfpga2 devstack@n-api.service[3519]: INFO nova.api.openstack.requestlog [None req-1a8bb6ca-a12b-4aa4-b260-dae2594b7cbb admin admin] 172.25.103.180 \"GET /compute/v2.1/servers/6536326f-42c2-4dd4-9078-c69a0d9e9bed\" status: 200 len: 1752 microversion: 2.1 time: 0.309959\n Mar 19 19:33:28 otcfpga2 devstack@n-api.service[3519]: [pid: 3520|app: 0|req: 1/1] 172.25.103.180 () {60 vars in 1249 bytes} [Thu Mar 19 19:33:28 2020] GET /compute/v2.1/servers/6536326f-42c2-4dd4-9078-c69a0d9e9bed \u003d\u003e generated 1752 bytes in 311 msecs (HTTP/1.1 200) 9 headers in 358 bytes (1 switches on core 0)","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"2dc82d3e519499e77400685cb88ed04be364784e","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":18,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":45,"id":"df33271e_67f02bcb","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":25},"in_reply_to":"df33271e_fee5ef0b","updated":"2020-03-23 17:23:10.000000000","message":"Gotcha. Will drop this.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4acb26725b90b66eacd831d8d04f96091b052713","unresolved":false,"context_lines":[{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor, i.e.,"},{"line_number":16,"context_line":"  $ openstack server resize \u003cserver_id\u003e"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":19,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":46,"id":"df33271e_decbcb6f","line":16,"updated":"2020-03-23 11:15:39.000000000","message":"This client call is not really triggering any nova server action.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"2dc82d3e519499e77400685cb88ed04be364784e","unresolved":false,"context_lines":[{"line_number":13,"context_line":"The following work but are no-ops:"},{"line_number":14,"context_line":"* Lock/unlock"},{"line_number":15,"context_line":"* Resize with same flavor, i.e.,"},{"line_number":16,"context_line":"  $ openstack server resize \u003cserver_id\u003e"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Hard reboots are taken up in a later patch in this series."},{"line_number":19,"context_line":"Soft reboots work for accelerators unless some unrelated failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":46,"id":"df33271e_a7bb53e3","line":16,"in_reply_to":"df33271e_decbcb6f","updated":"2020-03-23 17:23:10.000000000","message":"Ok, will drop.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"}],"nova/accelerator/cyborg.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"54df1407c4748f7a78c3acb5e5a9fdc70fc211a4","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        if not resp:"},{"line_number":227,"context_line":"            raise exception.AcceleratorRequestOpFailed("},{"line_number":228,"context_line":"                op\u003d\u0027delete\u0027,"},{"line_number":229,"context_line":"                param\u003d\u0027instance %s\u0027 % instance_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_1f918d52","line":229,"updated":"2019-08-06 17:12:58.000000000","message":"Needs a test.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"0c70123beba68d4872e2fc18175cfadd23431c14","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        if not resp:"},{"line_number":227,"context_line":"            raise exception.AcceleratorRequestOpFailed("},{"line_number":228,"context_line":"                op\u003d\u0027delete\u0027,"},{"line_number":229,"context_line":"                param\u003d\u0027instance %s\u0027 % instance_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_7f725a14","line":229,"in_reply_to":"7faddb67_1f918d52","updated":"2019-08-15 22:56:30.000000000","message":"As I said elsewhere, once we mock the Cyborg API call, there is not much going on here to test in terms of side effects or return values.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":216,"context_line":"           :returns: Nothing"},{"line_number":217,"context_line":"        \"\"\""},{"line_number":218,"context_line":"        url \u003d self.ARQ_URL"},{"line_number":219,"context_line":"        # Unbind and delete the ARQs"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_da355846","line":216,"updated":"2019-10-15 18:10:20.000000000","message":":raises: here would be nice","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":216,"context_line":"           :returns: Nothing"},{"line_number":217,"context_line":"        \"\"\""},{"line_number":218,"context_line":"        url \u003d self.ARQ_URL"},{"line_number":219,"context_line":"        # Unbind and delete the ARQs"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_a7101902","line":216,"in_reply_to":"3fa7e38b_da355846","updated":"2019-10-30 02:42:30.000000000","message":"Done","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    return cyclient.get_device_profile_groups(dp_name)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_7e2d63fa","line":76,"updated":"2020-01-06 19:50:04.000000000","message":"I was going to re-complain about this being a silent no-op by inspecting the instance flavor. I still don\u0027t like that, but I see you\u0027re using this in a similar way to the more compute-specific bdm and network info cleanup routines. I don\u0027t like that you\u0027ve spread the very nova- and time-specific notion of what it means for an instance to have ARQs into this basically-client code module.\n\nCan you come up with a slightly more descriptive name that makes it clear that this is going to delete ARQs for an instance, if it has a device profile, else be a no-op? Otherwise, I\u0027d like to see us keep the inspection of the instance\u0027s flavor in the compute-specific code instead of dragging it over here. If we were to move that to something other than an extra spec in the future, it would be nice if this cyborg module worked independently from how nova (currently) stores the device profile of an instance. Maybe just put something in compute/utils that does this \"nop-if-no-device-profile\" behavior to wrap?","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    return cyclient.get_device_profile_groups(dp_name)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_0be04ff9","line":76,"in_reply_to":"3fa7e38b_5f717744","updated":"2020-01-14 21:40:16.000000000","message":"Done","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"638ec915aa944cd82468ef32a04c4f32ce9ffc52","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    return cyclient.get_device_profile_groups(dp_name)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_5f717744","line":76,"in_reply_to":"3fa7e38b_6754f356","updated":"2020-01-07 15:48:12.000000000","message":"\u003e The check could be done in compute utils, but there is not much\n \u003e else in this function anyway. Would you be ok if I moved this\n \u003e entire function to compute utils?\n\nYep, that\u0027s what I was suggesting.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"e8a3f6421c06e1dc5120d60bf1f63f93cdb249dd","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    return cyclient.get_device_profile_groups(dp_name)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_6754f356","line":76,"in_reply_to":"3fa7e38b_7e2d63fa","updated":"2020-01-07 08:16:53.000000000","message":"Yup, this is meant to parallel the bdm/network approaches. \n\nI could change the name to \u0027delete_instance_arqs_if_needed\u0027. \n\nThis function is called in a bunch of places: replicating the if check in every call site is going to result in quite some redundancy.\n\nThe check could be done in compute utils, but there is not much else in this function anyway. Would you be ok if I moved this entire function to compute utils?","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"00a9aae55e74677c72c984696dbc22c5be3e2fc2","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_99586425","line":78,"updated":"2020-01-08 03:35:46.000000000","message":"I have a doubt, if the \u0027accel:device_profile\u0027 in the instance\u0027s flavor, but the instance already lose the accelerator device (the device is offline), How do we do this scenario?\n\n* If the active instance lost the accerator device, is it in a state of death? (It\u0027s in active status, but cannot do nothing)\n\n    NOTE: IMO, if the hardware of an instance in the running state is lost, it will be suspended.\n\n* If we hard reboot or soft reboot *this* instance, does it become ERROR?\n\nTherefore, when we delete ARQ, do we need to determine whether the device accelerator used by this instance still exists? Does it exist to raise an exception?","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def delete_arqs(context, instance):"},{"line_number":77,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":78,"context_line":"    dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_abdb3b1e","line":78,"in_reply_to":"3fa7e38b_99586425","updated":"2020-01-14 21:40:16.000000000","message":"Many of these questions are very broad in scope, and the answers may vary substantially across devices. Most of them seem to be outside the scope of Cyborg/OpenStack.\n\n \u003e I have a doubt, if the \u0027accel:device_profile\u0027 in the instance\u0027s\n \u003e flavor, but the instance already lose the accelerator device (the\n \u003e device is offline), How do we do this scenario?\n\nIf the accelerator device is not responding to PCI transactions, it may trigger a fatal machine check error in the system, causing a host crash. This is outside of Cyborg/OpenStack, or even qemu/libvirt.\n\n \u003e * If the active instance lost the accerator device, is it in a\n \u003e state of death? (It\u0027s in active status, but cannot do nothing)\n\nWhat does \u0027lost\u0027 mean? If the device responds to PCI transactions, but its computation is hanging/deadlocked, I suppose it is possible for the instance to be active while the accelerator is \u0027lost\u0027. But one would expect that the application or the device driver has a timeout for unresponsive devices. These are general questions outside the scope of Cyborg.\n\n \u003e NOTE: IMO, if the hardware of an instance in the running state is\n \u003e lost, it will be suspended.\n\nHave you actually seen such behavior? I have seen host crashes when the device fails PCI transactions, but haven\u0027t seen the VM getting suspended. Which entity will suspend the instance?\n\n \u003e * If we hard reboot or soft reboot *this* instance, does it become\n \u003e ERROR?\n\nDepends on what happens when the instance disconnects from the device after a reboot. Some devices may do an automatic reset, perhaps a PCI FLR. That can result in or trigger device errors if the device was already in a bad state.\n\n \u003e Therefore, when we delete ARQ, do we need to determine whether the\n \u003e device accelerator used by this instance still exists? Does it\n \u003e exist to raise an exception?\n\nFirst, it is not Nova\u0027s job to check the device state. Secondly, though the Cyborg agent scans the compute host periodically for discovery, when a device is assigned to a VM, its PCI function will not show up in the host scan. So, Cyborg would not know about its health, unless programming failed or something. In short, my answer to these questions is No.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"},{"line_number":82,"context_line":"    LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":83,"context_line":"             {\u0027instance\u0027: instance.uuid})"},{"line_number":84,"context_line":"    cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_9ee4dfe0","line":82,"updated":"2020-01-06 19:50:04.000000000","message":"This is too verbose for info level, IMHO.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"},{"line_number":82,"context_line":"    LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":83,"context_line":"             {\u0027instance\u0027: instance.uuid})"},{"line_number":84,"context_line":"    cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_2bdd0b3f","line":82,"in_reply_to":"3fa7e38b_3fb91bca","updated":"2020-01-14 21:40:16.000000000","message":"Done","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"638ec915aa944cd82468ef32a04c4f32ce9ffc52","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"},{"line_number":82,"context_line":"    LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":83,"context_line":"             {\u0027instance\u0027: instance.uuid})"},{"line_number":84,"context_line":"    cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_3fb91bca","line":82,"in_reply_to":"3fa7e38b_474f77bd","updated":"2020-01-07 15:48:12.000000000","message":"Well, the openstack logging guidelines are a good start:\n\nhttps://specs.openstack.org/openstack/openstack-specs/specs/log-guidelines.html#log-messages-at-info-and-above-should-be-a-unit-of-work\n\nIt says that info should be a \"unit of work\" and gives the relevant-to-you example of \"instance spawned\" or \"instance destroyed\". In the next section about debug level, it specifically also calls out debug as the proper level for \"starting and ending\" messages of smaller actions useful for debugging a flow.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"e8a3f6421c06e1dc5120d60bf1f63f93cdb249dd","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    if dp_name is None:"},{"line_number":80,"context_line":"        return"},{"line_number":81,"context_line":"    cyclient \u003d get_client(context)"},{"line_number":82,"context_line":"    LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":83,"context_line":"             {\u0027instance\u0027: instance.uuid})"},{"line_number":84,"context_line":"    cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_474f77bd","line":82,"in_reply_to":"3fa7e38b_9ee4dfe0","updated":"2020-01-07 08:16:53.000000000","message":"It is literally printing 7 words and a UUID, nothing else. What is your threshold for \u0027too verbose\u0027?","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":257,"context_line":"           :returns: Nothing"},{"line_number":258,"context_line":"           :raises: AcceleratorRequestOpFailed"},{"line_number":259,"context_line":"        \"\"\""},{"line_number":260,"context_line":"        # Unbind and delete the ARQs"},{"line_number":261,"context_line":"        params \u003d {\"instance\": instance_uuid}"},{"line_number":262,"context_line":"        resp, err_msg \u003d self._call_cyborg(self._client.delete,"},{"line_number":263,"context_line":"            self.ARQ_URL, params\u003dparams)"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_e5b962c5","line":260,"range":{"start_line":260,"start_character":1,"end_line":260,"end_character":36},"updated":"2020-02-18 11:54:01.000000000","message":"We only call delete below. Does this means unbind in implicit in cyborg?","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":257,"context_line":"           :returns: Nothing"},{"line_number":258,"context_line":"           :raises: AcceleratorRequestOpFailed"},{"line_number":259,"context_line":"        \"\"\""},{"line_number":260,"context_line":"        # Unbind and delete the ARQs"},{"line_number":261,"context_line":"        params \u003d {\"instance\": instance_uuid}"},{"line_number":262,"context_line":"        resp, err_msg \u003d self._call_cyborg(self._client.delete,"},{"line_number":263,"context_line":"            self.ARQ_URL, params\u003dparams)"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_82070bc9","line":260,"range":{"start_line":260,"start_character":1,"end_line":260,"end_character":36},"in_reply_to":"3fa7e38b_e5b962c5","updated":"2020-02-21 11:51:51.000000000","message":"Yes. Cyborg will unbind an ARQ before deleting if it notices that it is in bound state.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":276,"context_line":"        resp, err_msg \u003d self._call_cyborg(self._client.delete,"},{"line_number":277,"context_line":"            self.ARQ_URL, params\u003dparams)"},{"line_number":278,"context_line":"        if err_msg:"},{"line_number":279,"context_line":"            msg \u003d err_msg + _(\u0027instance %s\u0027) % instance_uuid"},{"line_number":280,"context_line":"            raise exception.AcceleratorRequestOpFailed("},{"line_number":281,"context_line":"                op\u003d_(\u0027delete\u0027), msg\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_8db43737","line":279,"range":{"start_line":279,"start_character":31,"end_line":279,"end_character":32},"updated":"2020-03-13 17:36:29.000000000","message":"Don\u0027t you need a space here?","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":276,"context_line":"        resp, err_msg \u003d self._call_cyborg(self._client.delete,"},{"line_number":277,"context_line":"            self.ARQ_URL, params\u003dparams)"},{"line_number":278,"context_line":"        if err_msg:"},{"line_number":279,"context_line":"            msg \u003d err_msg + _(\u0027instance %s\u0027) % instance_uuid"},{"line_number":280,"context_line":"            raise exception.AcceleratorRequestOpFailed("},{"line_number":281,"context_line":"                op\u003d_(\u0027delete\u0027), msg\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_9820ee4b","line":279,"range":{"start_line":279,"start_character":31,"end_line":279,"end_character":32},"in_reply_to":"1fa4df85_8db43737","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b846956bfc8b0dd3862fbb0dcb3b16e544d871be","unresolved":false,"context_lines":[{"line_number":213,"context_line":"             self.ARQ_URL, json\u003dpatch_list)"},{"line_number":214,"context_line":"        if err_msg:"},{"line_number":215,"context_line":"            msg \u003d _(\u0027 Binding failed for ARQ UUIDs: \u0027)"},{"line_number":216,"context_line":"            err_msg \u003d err_msg + msg + \u0027,\u0027.join(bindings.keys())"},{"line_number":217,"context_line":"            raise exception.AcceleratorRequestOpFailed("},{"line_number":218,"context_line":"                op\u003d_(\u0027bind\u0027), msg\u003derr_msg)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_8dcbb251","line":216,"updated":"2020-03-25 05:55:18.000000000","message":"If PATCH API failed, we should have a try to delete the created arqs mapping in Cyborg db.\n\nAs I said in https://review.opendev.org/#/c/631244/69/nova/accelerator/cyborg.py@214","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"e3f81a304c49cb9b9d3e2c20fc09bd6980457104","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        return arqs"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def delete_arqs_for_instance(self, instance_uuid):"},{"line_number":268,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":271,"context_line":"           :returns: Nothing"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_d1c0d053","line":268,"range":{"start_line":268,"start_character":37,"end_line":268,"end_character":52},"updated":"2020-03-24 01:08:30.000000000","message":"a question, where are you doing the unbinding? or it needn\u0027t?","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"5275da49ba5826d2238fbced1ed3f6eabedfceb9","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        return arqs"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def delete_arqs_for_instance(self, instance_uuid):"},{"line_number":268,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":271,"context_line":"           :returns: Nothing"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_6c8807c6","line":268,"range":{"start_line":268,"start_character":37,"end_line":268,"end_character":52},"in_reply_to":"df33271e_0c0e9b33","updated":"2020-03-25 03:23:45.000000000","message":"Yes indeed.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c09bd8dc8b06c49f0469a3d8e920f2a8fcfbd18","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        return arqs"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def delete_arqs_for_instance(self, instance_uuid):"},{"line_number":268,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":271,"context_line":"           :returns: Nothing"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_0c0e9b33","line":268,"range":{"start_line":268,"start_character":37,"end_line":268,"end_character":52},"in_reply_to":"df33271e_d1c0d053","updated":"2020-03-24 02:23:03.000000000","message":"i think cyboog is doing it automatically.\n\nthe current cyborg workflow does not require nova to unbind before the delete.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c40f2a86ccbc98f29b006a44e0918b7bc3539d42","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":271,"context_line":"           :returns: Nothing"},{"line_number":272,"context_line":"           :raises: AcceleratorRequestOpFailed"},{"line_number":273,"context_line":"        \"\"\""},{"line_number":274,"context_line":"        # Unbind and delete the ARQs"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_3d73d417","line":271,"range":{"start_line":271,"start_character":10,"end_line":271,"end_character":28},"updated":"2020-03-23 19:39:21.000000000","message":"nit: techncailaly if it returns nothing and you assgin it to a vailabel and look at the type it will be None.\n\nbut in general its better to not have a :returns: paramater if it does not return something.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"5275da49ba5826d2238fbced1ed3f6eabedfceb9","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        \"\"\"Delete ARQs for instance, after unbinding if needed."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"           :param instance_uuid: Instance UUID"},{"line_number":271,"context_line":"           :returns: Nothing"},{"line_number":272,"context_line":"           :raises: AcceleratorRequestOpFailed"},{"line_number":273,"context_line":"        \"\"\""},{"line_number":274,"context_line":"        # Unbind and delete the ARQs"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_4c8543ad","line":271,"range":{"start_line":271,"start_character":10,"end_line":271,"end_character":28},"in_reply_to":"df33271e_3d73d417","updated":"2020-03-25 03:23:45.000000000","message":"Done","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"}],"nova/compute/api.py":[{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[{"line_number":2340,"context_line":"            self._local_cleanup_bdm_volumes(bdms, instance, context)"},{"line_number":2341,"context_line":""},{"line_number":2342,"context_line":"            # cleanup accelerator requests (ARQs)"},{"line_number":2343,"context_line":"            self.delete_arqs(context, instance)"},{"line_number":2344,"context_line":""},{"line_number":2345,"context_line":"            # Cleanup allocations in Placement since we can\u0027t do it from the"},{"line_number":2346,"context_line":"            # compute service."}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_600fd62d","line":2343,"updated":"2019-10-15 18:10:20.000000000","message":"Tests should ensure this was called with correct values","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[{"line_number":2340,"context_line":"            self._local_cleanup_bdm_volumes(bdms, instance, context)"},{"line_number":2341,"context_line":""},{"line_number":2342,"context_line":"            # cleanup accelerator requests (ARQs)"},{"line_number":2343,"context_line":"            self.delete_arqs(context, instance)"},{"line_number":2344,"context_line":""},{"line_number":2345,"context_line":"            # Cleanup allocations in Placement since we can\u0027t do it from the"},{"line_number":2346,"context_line":"            # compute service."}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_349ad56e","line":2343,"in_reply_to":"3fa7e38b_600fd62d","updated":"2019-10-30 02:42:30.000000000","message":"Done","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":2452,"context_line":"            # cleanup volumes"},{"line_number":2453,"context_line":"            self._local_cleanup_bdm_volumes(bdms, instance, context)"},{"line_number":2454,"context_line":""},{"line_number":2455,"context_line":"            # cleanup accelerator requests (ARQs)"},{"line_number":2456,"context_line":"            compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2457,"context_line":""},{"line_number":2458,"context_line":"            # Cleanup allocations in Placement since we can\u0027t do it from the"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_0dd04750","line":2455,"updated":"2020-03-13 17:36:29.000000000","message":"Does this really add anything over what the method name says? I know that there\u0027s a similarly useless comment for volumes above, but I think this is just noise.\n\nComments should describe *why* you\u0027re doing something, not as much  *what*. The code should be readable enough to know the latter. The placement case below is a good example - from the code, it\u0027s clear we\u0027re deleting allocations, but the comment explains why we need to do it here/now.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":2452,"context_line":"            # cleanup volumes"},{"line_number":2453,"context_line":"            self._local_cleanup_bdm_volumes(bdms, instance, context)"},{"line_number":2454,"context_line":""},{"line_number":2455,"context_line":"            # cleanup accelerator requests (ARQs)"},{"line_number":2456,"context_line":"            compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2457,"context_line":""},{"line_number":2458,"context_line":"            # Cleanup allocations in Placement since we can\u0027t do it from the"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_259437e8","line":2455,"in_reply_to":"1fa4df85_0dd04750","updated":"2020-03-22 01:28:06.000000000","message":"This comment clarifies what ARQ means to somebody unfamiliar with that acronym, and points them to Cyborg. Looking at other comments even in this same file, that is more info than [1], which did get approved. Or [2] or the more recent [3].\n\nHad I not added this, maybe somebody would have commented that the previous and next steps are commented, and this is not. :)\n\n[1] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/api.py;hb\u003drefs/changes/35/673735/45#l1193 \n\n[2] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/api.py;hb\u003drefs/changes/35/673735/45#l327\n\n[3] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/api.py;hb\u003drefs/changes/35/673735/45#l735","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"54df1407c4748f7a78c3acb5e5a9fdc70fc211a4","unresolved":false,"context_lines":[{"line_number":2774,"context_line":"            six.reraise(exc_info[0], exc_info[1], exc_info[2])"},{"line_number":2775,"context_line":""},{"line_number":2776,"context_line":"    def _delete_arqs(self, context, instance_uuid):"},{"line_number":2777,"context_line":"        \"\"\"Delete Cyborg ARQs for the instance asynchronously.\"\"\""},{"line_number":2778,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":2779,"context_line":"        LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":2780,"context_line":"                 {\u0027instance\u0027: instance_uuid})"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_9fc01d52","line":2777,"range":{"start_line":2777,"start_character":47,"end_line":2777,"end_character":61},"updated":"2019-08-06 17:12:58.000000000","message":"This method is not (or doesn\u0027t appear to be) async. are you reflecting that the ARQ delete is async on the cyborg side?","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"0c70123beba68d4872e2fc18175cfadd23431c14","unresolved":false,"context_lines":[{"line_number":2774,"context_line":"            six.reraise(exc_info[0], exc_info[1], exc_info[2])"},{"line_number":2775,"context_line":""},{"line_number":2776,"context_line":"    def _delete_arqs(self, context, instance_uuid):"},{"line_number":2777,"context_line":"        \"\"\"Delete Cyborg ARQs for the instance asynchronously.\"\"\""},{"line_number":2778,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":2779,"context_line":"        LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":2780,"context_line":"                 {\u0027instance\u0027: instance_uuid})"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_9f707642","line":2777,"range":{"start_line":2777,"start_character":47,"end_line":2777,"end_character":61},"in_reply_to":"7faddb67_9fc01d52","updated":"2019-08-15 22:56:30.000000000","message":"Yes, Cyborg API for delete is expected to return without waiting for device cleanup, if any. If there is nothing to clean up, it may return right away.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"54df1407c4748f7a78c3acb5e5a9fdc70fc211a4","unresolved":false,"context_lines":[{"line_number":2778,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":2779,"context_line":"        LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":2780,"context_line":"                 {\u0027instance\u0027: instance_uuid})"},{"line_number":2781,"context_line":"        cyclient.delete_arqs_for_instance(instance_uuid)"},{"line_number":2782,"context_line":""},{"line_number":2783,"context_line":"    @hooks.add_hook(\"delete_instance\")"},{"line_number":2784,"context_line":"    def _delete_instance(self, context, instance, bdms):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_dfc1b54f","line":2781,"updated":"2019-08-06 17:12:58.000000000","message":"This method needs a test.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"0c70123beba68d4872e2fc18175cfadd23431c14","unresolved":false,"context_lines":[{"line_number":2778,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":2779,"context_line":"        LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":2780,"context_line":"                 {\u0027instance\u0027: instance_uuid})"},{"line_number":2781,"context_line":"        cyclient.delete_arqs_for_instance(instance_uuid)"},{"line_number":2782,"context_line":""},{"line_number":2783,"context_line":"    @hooks.add_hook(\"delete_instance\")"},{"line_number":2784,"context_line":"    def _delete_instance(self, context, instance, bdms):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_dfcd8e5d","line":2781,"in_reply_to":"7faddb67_dfc1b54f","updated":"2019-08-15 22:56:30.000000000","message":"The delete_arqs_for_instance() method doesn\u0027t change anything in Nova or return anything  back to the caller. So, once we mock the Cyborg API call, there is nothing much to test for.\n\nHowever, I\u0027ll add UT for _delete_arqs method as a whole.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"474a45eb1613074c9a99a3bdf67d92c6fd7fd4c4","unresolved":false,"context_lines":[{"line_number":2778,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":2779,"context_line":"        LOG.info(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":2780,"context_line":"                 {\u0027instance\u0027: instance_uuid})"},{"line_number":2781,"context_line":"        cyclient.delete_arqs_for_instance(instance_uuid)"},{"line_number":2782,"context_line":""},{"line_number":2783,"context_line":"    @hooks.add_hook(\"delete_instance\")"},{"line_number":2784,"context_line":"    def _delete_instance(self, context, instance, bdms):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_757cb9b5","line":2781,"in_reply_to":"7faddb67_dfcd8e5d","updated":"2019-08-16 03:22:57.000000000","message":"On 2nd thoughts, I will add UT to check that a failure return from Cyborg triggers the right exception.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"54df1407c4748f7a78c3acb5e5a9fdc70fc211a4","unresolved":false,"context_lines":[{"line_number":2800,"context_line":"                phase\u003dfields.NotificationPhase.START, bdms\u003dbdms)"},{"line_number":2801,"context_line":""},{"line_number":2802,"context_line":"        self._shutdown_instance(context, instance, bdms)"},{"line_number":2803,"context_line":"        self._delete_arqs(context, instance.uuid)"},{"line_number":2804,"context_line":""},{"line_number":2805,"context_line":"        # NOTE(vish): We have already deleted the instance, so we have"},{"line_number":2806,"context_line":"        #             to ignore problems cleaning up the volumes. It"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_ff959140","line":2803,"updated":"2019-08-06 17:12:58.000000000","message":"This needs a test.\n\nAre you sure you want to do this delete here? If the compute is down, we won\u0027t do the ARQ delete (obviously). That may be desirable since I expect cyborg won\u0027t be able to do its node-resident work either, but, just something to think about. Also, you probably want/need a local delete version of this in compute/api.py","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"0c70123beba68d4872e2fc18175cfadd23431c14","unresolved":false,"context_lines":[{"line_number":2800,"context_line":"                phase\u003dfields.NotificationPhase.START, bdms\u003dbdms)"},{"line_number":2801,"context_line":""},{"line_number":2802,"context_line":"        self._shutdown_instance(context, instance, bdms)"},{"line_number":2803,"context_line":"        self._delete_arqs(context, instance.uuid)"},{"line_number":2804,"context_line":""},{"line_number":2805,"context_line":"        # NOTE(vish): We have already deleted the instance, so we have"},{"line_number":2806,"context_line":"        #             to ignore problems cleaning up the volumes. It"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_df24ae0f","line":2803,"in_reply_to":"7faddb67_ff959140","updated":"2019-08-15 22:56:30.000000000","message":"Do you have suggestions for where else this can be done? \n\nA. Since create/bind was explicitly asked to be moved from the conductor to compute.manager, I supposed that the deletion should happen here too. \n\nB. As you noted, the compute down may go out of touch with the controller after the VM has spawned. So, if we delete ARQs form the conductor, Cyborg may mark the resources are free whereas the VM is still using those resources.\n\nAdded local delete support.","commit_id":"689c6b901a47acbb5db6c25081c5ad2b4c854497"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[{"line_number":2734,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2735,"context_line":"                finally:"},{"line_number":2736,"context_line":"                    if dp_name:"},{"line_number":2737,"context_line":"                        self.compute_api.delete_arqs(context, instance)"},{"line_number":2738,"context_line":""},{"line_number":2739,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2740,"context_line":"            requested_networks):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_c04eca6d","line":2737,"updated":"2019-10-15 18:10:20.000000000","message":"tests?","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[{"line_number":2734,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2735,"context_line":"                finally:"},{"line_number":2736,"context_line":"                    if dp_name:"},{"line_number":2737,"context_line":"                        self.compute_api.delete_arqs(context, instance)"},{"line_number":2738,"context_line":""},{"line_number":2739,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2740,"context_line":"            requested_networks):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_ffc71c44","line":2737,"in_reply_to":"3fa7e38b_c04eca6d","updated":"2019-10-30 02:42:30.000000000","message":"For other exceptions in _build_resources, which have been added in next patch set, UT has been added. But it is not easy to force \u0027yield resources\u0027 to generate an exception.","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[{"line_number":2988,"context_line":""},{"line_number":2989,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2990,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2991,"context_line":"        # Delete Cyborg ARQs if the instance has an device profile."},{"line_number":2992,"context_line":"        self.compute_api.delete_arqs(context, instance)"},{"line_number":2993,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2994,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_5a6f2827","line":2991,"range":{"start_line":2991,"start_character":50,"end_line":2991,"end_character":51},"updated":"2019-10-15 18:10:20.000000000","message":"-n","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[{"line_number":2988,"context_line":""},{"line_number":2989,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2990,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2991,"context_line":"        # Delete Cyborg ARQs if the instance has an device profile."},{"line_number":2992,"context_line":"        self.compute_api.delete_arqs(context, instance)"},{"line_number":2993,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2994,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_148f992a","line":2991,"range":{"start_line":2991,"start_character":50,"end_line":2991,"end_character":51},"in_reply_to":"3fa7e38b_5a6f2827","updated":"2019-10-30 02:42:30.000000000","message":"Done","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"42bc086a00f097d7d4b186f7b4219d5137034d1d","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"                msg \u003d _(\u0027Failed to allocate the network(s), not rescheduling.\u0027)"},{"line_number":2658,"context_line":"                raise exception.BuildAbortException("},{"line_number":2659,"context_line":"                    instance_uuid\u003dinstance.uuid, reason\u003dmsg)"},{"line_number":2660,"context_line":"            finally:"},{"line_number":2661,"context_line":"                # If other resources cannot be acquired, ARQs should get"},{"line_number":2662,"context_line":"                # deleted. But any exception in ARQ deletion should get"},{"line_number":2663,"context_line":"                # ignored so that the original resource exception gets"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_34a1a3f8","line":2660,"updated":"2019-12-03 15:05:36.000000000","message":"I think what you want is \"else\" instead of this flag usage.\n\n try:\n     ...code\n else:\n     # this only gets called  if no exception is raised\n finally:\n     # always gets called regardless","commit_id":"7217cfd7928d751ff5e98fa116fc97119e98e9b5"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"1fc7b1de25d5c54b39bbdcd311cf7aaf2a84ffd9","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"                msg \u003d _(\u0027Failed to allocate the network(s), not rescheduling.\u0027)"},{"line_number":2658,"context_line":"                raise exception.BuildAbortException("},{"line_number":2659,"context_line":"                    instance_uuid\u003dinstance.uuid, reason\u003dmsg)"},{"line_number":2660,"context_line":"            finally:"},{"line_number":2661,"context_line":"                # If other resources cannot be acquired, ARQs should get"},{"line_number":2662,"context_line":"                # deleted. But any exception in ARQ deletion should get"},{"line_number":2663,"context_line":"                # ignored so that the original resource exception gets"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_cb6182d6","line":2660,"in_reply_to":"3fa7e38b_34a1a3f8","updated":"2019-12-04 09:09:20.000000000","message":"This has become moot in next patchset.","commit_id":"7217cfd7928d751ff5e98fa116fc97119e98e9b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"42bc086a00f097d7d4b186f7b4219d5137034d1d","unresolved":false,"context_lines":[{"line_number":2725,"context_line":"                msg \u003d _(\u0027Failure prepping block device.\u0027)"},{"line_number":2726,"context_line":"                raise exception.BuildAbortException("},{"line_number":2727,"context_line":"                    instance_uuid\u003dinstance.uuid, reason\u003dmsg)"},{"line_number":2728,"context_line":"            finally:"},{"line_number":2729,"context_line":"                # See comments after previous \u0027finally\u0027 clause."},{"line_number":2730,"context_line":"                try:"},{"line_number":2731,"context_line":"                    if exception_flag and dp_name:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_f4aa2bd1","line":2728,"updated":"2019-12-03 15:05:36.000000000","message":"Same here","commit_id":"7217cfd7928d751ff5e98fa116fc97119e98e9b5"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"1fc7b1de25d5c54b39bbdcd311cf7aaf2a84ffd9","unresolved":false,"context_lines":[{"line_number":2725,"context_line":"                msg \u003d _(\u0027Failure prepping block device.\u0027)"},{"line_number":2726,"context_line":"                raise exception.BuildAbortException("},{"line_number":2727,"context_line":"                    instance_uuid\u003dinstance.uuid, reason\u003dmsg)"},{"line_number":2728,"context_line":"            finally:"},{"line_number":2729,"context_line":"                # See comments after previous \u0027finally\u0027 clause."},{"line_number":2730,"context_line":"                try:"},{"line_number":2731,"context_line":"                    if exception_flag and dp_name:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_6b608ed0","line":2728,"in_reply_to":"3fa7e38b_f4aa2bd1","updated":"2019-12-04 09:09:20.000000000","message":"Ditto.","commit_id":"7217cfd7928d751ff5e98fa116fc97119e98e9b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"965b48446e1961c09fcf4c191c06ef337830864f","unresolved":false,"context_lines":[{"line_number":3075,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":3076,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":3077,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3078,"context_line":"        cyborg.delete_arqs(context, instance)"},{"line_number":3079,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":3080,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":3081,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fa7e38b_68ebada3","line":3078,"updated":"2019-12-04 15:16:45.000000000","message":"Why can you call this always, but have to check for dp_name on L2775?","commit_id":"db84e0e29abdccf11368ac6b465a164c10e88c82"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"e8a3f6421c06e1dc5120d60bf1f63f93cdb249dd","unresolved":false,"context_lines":[{"line_number":3075,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":3076,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":3077,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3078,"context_line":"        cyborg.delete_arqs(context, instance)"},{"line_number":3079,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":3080,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":3081,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fa7e38b_b7af9529","line":3078,"in_reply_to":"3fa7e38b_68ebada3","updated":"2020-01-07 08:16:53.000000000","message":"Done","commit_id":"db84e0e29abdccf11368ac6b465a164c10e88c82"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":2719,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2720,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2721,"context_line":"                finally:"},{"line_number":2722,"context_line":"                    cyborg.delete_arqs(context, instance)"},{"line_number":2723,"context_line":""},{"line_number":2724,"context_line":"    def _get_bound_arq_resources(self, context, dp_name, instance):"},{"line_number":2725,"context_line":"        \"\"\"Get bound accelerator requests."}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_901a8e68","line":2722,"updated":"2020-01-06 19:50:04.000000000","message":"I don\u0027t think you\u0027re testing this code. You need to blow up inside the context in your test in order to hit it.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":2719,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2720,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2721,"context_line":"                finally:"},{"line_number":2722,"context_line":"                    cyborg.delete_arqs(context, instance)"},{"line_number":2723,"context_line":""},{"line_number":2724,"context_line":"    def _get_bound_arq_resources(self, context, dp_name, instance):"},{"line_number":2725,"context_line":"        \"\"\"Get bound accelerator requests."}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_57a25a41","line":2722,"in_reply_to":"3fa7e38b_901a8e68","updated":"2020-01-14 21:40:16.000000000","message":"Done","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"63b3f170e7bba9bdb6d71eb52f8f728de0f10421","unresolved":false,"context_lines":[{"line_number":2601,"context_line":"        except (Exception, eventlet.timeout.Timeout) as exc:"},{"line_number":2602,"context_line":"            LOG.exception(exc.format_message())"},{"line_number":2603,"context_line":"            self._build_resources_cleanup(instance, network_info)"},{"line_number":2604,"context_line":"            compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2605,"context_line":"            msg \u003d _(\u0027Failure getting accelerator requests.\u0027)"},{"line_number":2606,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":2607,"context_line":"                    reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_3befef1e","line":2604,"range":{"start_line":2604,"start_character":12,"end_line":2604,"end_character":66},"updated":"2020-03-06 05:00:06.000000000","message":"if there is an exception raised, so I guess it should be ok, then it turns into rescheduling in the up-layer.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f2fd755ca5c39f127feb6d8d924c8fb16e50f78b","unresolved":false,"context_lines":[{"line_number":2601,"context_line":"        except (Exception, eventlet.timeout.Timeout) as exc:"},{"line_number":2602,"context_line":"            LOG.exception(exc.format_message())"},{"line_number":2603,"context_line":"            self._build_resources_cleanup(instance, network_info)"},{"line_number":2604,"context_line":"            compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2605,"context_line":"            msg \u003d _(\u0027Failure getting accelerator requests.\u0027)"},{"line_number":2606,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":2607,"context_line":"                    reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_192b0483","line":2604,"range":{"start_line":2604,"start_character":12,"end_line":2604,"end_character":66},"in_reply_to":"1fa4df85_3befef1e","updated":"2020-03-08 23:24:47.000000000","message":"Yea, we delete the ARQs, which includes unbinding from this host, here before rescheduling.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"63b3f170e7bba9bdb6d71eb52f8f728de0f10421","unresolved":false,"context_lines":[{"line_number":2637,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2638,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2639,"context_line":"                finally:"},{"line_number":2640,"context_line":"                    compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2641,"context_line":""},{"line_number":2642,"context_line":"    def _get_bound_arq_resources(self, context, dp_name, instance):"},{"line_number":2643,"context_line":"        \"\"\"Get bound accelerator requests."}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_9bdf0368","line":2640,"range":{"start_line":2640,"start_character":18,"end_line":2640,"end_character":74},"updated":"2020-03-06 05:00:06.000000000","message":"ditto","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"63b3f170e7bba9bdb6d71eb52f8f728de0f10421","unresolved":false,"context_lines":[{"line_number":2934,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2935,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2936,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2937,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2938,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2939,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2940,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_7b8b2767","line":2937,"range":{"start_line":2937,"start_character":7,"end_line":2937,"end_character":62},"updated":"2020-03-06 05:00:06.000000000","message":"we should avoid to raise exception from this. Since we don\u0027t want to stop the deletion even if the arqs deleted failure I think.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"7b3a5ba9d5ce902b4be04f9d67a5e2a061fa829c","unresolved":false,"context_lines":[{"line_number":2934,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2935,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2936,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2937,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2938,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2939,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2940,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_79fc78b4","line":2937,"range":{"start_line":2937,"start_character":7,"end_line":2937,"end_character":62},"in_reply_to":"1fa4df85_392e0092","updated":"2020-03-08 23:38:57.000000000","message":"This is for boot up new instance, this is for normal instance deletion call.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"fd9bbaffe336172a2327bc0cfd933886905dc267","unresolved":false,"context_lines":[{"line_number":2934,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2935,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2936,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2937,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2938,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2939,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2940,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_8c24945f","line":2937,"range":{"start_line":2937,"start_character":7,"end_line":2937,"end_character":62},"in_reply_to":"1fa4df85_79fc78b4","updated":"2020-03-09 05:37:45.000000000","message":"Discussed with Alex. Suppressing AcceleratorOp exception from delete_arqs_if_needed() in next PS.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f2fd755ca5c39f127feb6d8d924c8fb16e50f78b","unresolved":false,"context_lines":[{"line_number":2934,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2935,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2936,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2937,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2938,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2939,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2940,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_392e0092","line":2937,"range":{"start_line":2937,"start_character":7,"end_line":2937,"end_character":62},"in_reply_to":"1fa4df85_7b8b2767","updated":"2020-03-08 23:24:47.000000000","message":"Any exception from here is caught in https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/manager.py;hb\u003drefs/changes/35/673735/41#l2601 .","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":2976,"context_line":""},{"line_number":2977,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2978,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2979,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2980,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2981,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2982,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_2de8a314","line":2979,"updated":"2020-03-13 17:36:29.000000000","message":"I don\u0027t think this is particularly necessary, especially given you don\u0027t need it in the other two places where we call it in this file. The name of the method (which is good) pretty much covers it.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":2976,"context_line":""},{"line_number":2977,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2978,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2979,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2980,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2981,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2982,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_857c0bb6","line":2979,"in_reply_to":"1fa4df85_2de8a314","updated":"2020-03-22 01:28:06.000000000","message":"This highlights that there is a call to Cyborg involved here. Re. the other two calls, one of them [1] has a reference to accelerator requests right after, so the context should be clear. The other one can benefit from an explicit reference.\n\n[1] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/manager.py;hb\u003drefs/changes/35/673735/45#l2603\n\n[2] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/manager.py;hb\u003drefs/changes/35/673735/45#l2639","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":2977,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2978,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2979,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2980,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2981,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2982,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2983,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_8dfe77ac","line":2980,"updated":"2020-03-13 17:36:29.000000000","message":"I\u0027m not seeing where you\u0027re testing that this is working.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":2977,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2978,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2979,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":2980,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":2981,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2982,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2983,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_e537a8fd","line":2980,"in_reply_to":"1fa4df85_8dfe77ac","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/compute/utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":1554,"context_line":"def delete_arqs_if_needed(context, instance):"},{"line_number":1555,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":1556,"context_line":"    try:"},{"line_number":1557,"context_line":"        dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":1558,"context_line":"        if dp_name is None:"},{"line_number":1559,"context_line":"            return"},{"line_number":1560,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":1561,"context_line":"        LOG.debug(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":1562,"context_line":"                  {\u0027instance\u0027: instance.uuid})"},{"line_number":1563,"context_line":"        cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":1564,"context_line":"    except exception.AcceleratorRequestOpFailed as e:"},{"line_number":1565,"context_line":"        LOG.exception(\u0027Failed to delete accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_0df7e7f7","line":1562,"range":{"start_line":1557,"start_character":0,"end_line":1562,"end_character":46},"updated":"2020-03-13 17:36:29.000000000","message":"All of this can be done outside the try block.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":1554,"context_line":"def delete_arqs_if_needed(context, instance):"},{"line_number":1555,"context_line":"    \"\"\"Delete Cyborg ARQs for the instance.\"\"\""},{"line_number":1556,"context_line":"    try:"},{"line_number":1557,"context_line":"        dp_name \u003d instance.flavor.extra_specs.get(\u0027accel:device_profile\u0027)"},{"line_number":1558,"context_line":"        if dp_name is None:"},{"line_number":1559,"context_line":"            return"},{"line_number":1560,"context_line":"        cyclient \u003d cyborg.get_client(context)"},{"line_number":1561,"context_line":"        LOG.debug(\u0027Calling Cyborg to delete ARQs for instance %(instance)s\u0027,"},{"line_number":1562,"context_line":"                  {\u0027instance\u0027: instance.uuid})"},{"line_number":1563,"context_line":"        cyclient.delete_arqs_for_instance(instance.uuid)"},{"line_number":1564,"context_line":"    except exception.AcceleratorRequestOpFailed as e:"},{"line_number":1565,"context_line":"        LOG.exception(\u0027Failed to delete accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_813fc923","line":1562,"range":{"start_line":1557,"start_character":0,"end_line":1562,"end_character":46},"in_reply_to":"1fa4df85_0df7e7f7","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/conductor/manager.py":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"10a44850e8fae25018fc81ecf08889b0f205bc3f","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        # The BuildRequest needs to be stored until the instance is mapped to"},{"line_number":525,"context_line":"        # an instance table. At that point it will never be used again and"},{"line_number":526,"context_line":"        # should be deleted."},{"line_number":527,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":528,"context_line":"        build_request \u003d objects.BuildRequest.get_by_instance_uuid("},{"line_number":529,"context_line":"            context, instance.uuid)"},{"line_number":530,"context_line":"        # TODO(alaski): Sync API updates of the build_request to the"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_57b86cbf","line":527,"updated":"2020-02-13 07:19:04.000000000","message":"nit: you can put this code to Line524, above comments said for BuildRequest.","commit_id":"27e6d05541b47f51a135102193f09192c05a41c5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0b3d3960690ddc34e4e3f13b294c34f1a9b1917a","unresolved":false,"context_lines":[{"line_number":521,"context_line":"                                     host_list, self.network_api)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    def _destroy_build_request(self, context, instance):"},{"line_number":524,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":525,"context_line":"        # The BuildRequest needs to be stored until the instance is mapped to"},{"line_number":526,"context_line":"        # an instance table. At that point it will never be used again and"},{"line_number":527,"context_line":"        # should be deleted."}],"source_content_type":"text/x-python","patch_set":37,"id":"3fa7e38b_ba965ced","line":524,"updated":"2020-02-13 17:26:48.000000000","message":"This is not the right place to do this, unless you change the name of the method (but don\u0027t do that).\n\nWhat you want is to delete this at the same time as the build request, but you\u0027re basically adding an undocumented (and un-obvious) side-effect that has nothing to do with the BuildRequest record. Also, the BuildRequest is deleted normally as part of the boot process once the cell is known, so mixing this with that for the reschedule case is super confusing.","commit_id":"290f5382a3acf52937a7952ad21ed19fade5bc21"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"7048eb98290e8bda37c81b3e0d88cb6c8b26b02e","unresolved":false,"context_lines":[{"line_number":521,"context_line":"                                     host_list, self.network_api)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    def _destroy_build_request(self, context, instance):"},{"line_number":524,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":525,"context_line":"        # The BuildRequest needs to be stored until the instance is mapped to"},{"line_number":526,"context_line":"        # an instance table. At that point it will never be used again and"},{"line_number":527,"context_line":"        # should be deleted."}],"source_content_type":"text/x-python","patch_set":37,"id":"3fa7e38b_066f0893","line":524,"in_reply_to":"3fa7e38b_ba965ced","updated":"2020-02-14 21:48:57.000000000","message":"Done","commit_id":"290f5382a3acf52937a7952ad21ed19fade5bc21"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"0a93ab2685bb0e6b044cb113ca0dce811c22f50e","unresolved":false,"context_lines":[{"line_number":595,"context_line":"            legacy_request_spec)"},{"line_number":596,"context_line":"        self._cleanup_allocated_networks("},{"line_number":597,"context_line":"            context, instance, requested_networks)"},{"line_number":598,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"    # NOTE(danms): This is never cell-targeted because it is only used for"},{"line_number":601,"context_line":"    # n-cpu reschedules which go to the cell conductor and thus are always"}],"source_content_type":"text/x-python","patch_set":44,"id":"1fa4df85_1b266118","line":598,"updated":"2020-03-11 21:45:29.000000000","message":"See https://review.opendev.org/#/c/712231/ for a possible race condition that is addressed by this deletion.","commit_id":"d61f24cc5b0b631981b63d0a919d4d502eae62e7"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b63789a9caaf9afc79a5cb3641826ecdf2f7d441","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_91c64867","line":1632,"updated":"2020-03-24 01:09:53.000000000","message":"you need to cleanup arq here also","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d057898ec14ea30a8ae9dd0fcc992343efd09005","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_fcc93778","line":1632,"in_reply_to":"df33271e_2de08fcc","updated":"2020-03-26 12:44:57.000000000","message":"we might also want to consider adding a periodic task to cyborg that would clean up unbound arqs after like a week? month? year? ( add a conf option for the interval)\n\npart of me fells like we shoudl leave teh untill told to delete them but if we care about cleaning them up then having a house keeping task that deleted old arqs. that would require the cyborg data base to be exteded with a create_at filed as we don tcurrenlty track that for arqs i belive but that is the only  way to handel the api C1.\n\nthe periodic should also not be in nova it should be in cyborg. that said i dont think its needed right now but it would be good to consider in the future otherwise i think you will need to add a \"cyborg manage db clean-up-arqs\" command at some point to help operators clean this up.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3d0e07f3f1ba89f9dffa0666c2ddf643be7d1c30","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_7ee6d618","line":1632,"in_reply_to":"df33271e_2dec26c6","updated":"2020-03-25 09:00:29.000000000","message":"Good discussion. I think cleaning up unbound ARQs is needed but I\u0027m willing to compromise on doing that in a follow up patch.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"9d5cab05c45d8e4f8e31e4f6808d6581cbfe716c","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_c2eaa073","line":1632,"in_reply_to":"df33271e_7ee6d618","updated":"2020-03-25 14:18:52.000000000","message":"I\u0027ll add a new method _delete_arqs_by_uuid(), private to nova/accelerator/cyborg.py, and call it at:\n\n https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/accelerator/cyborg.py;hb\u003drefs/changes/35/673735/47#l215","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"5275da49ba5826d2238fbced1ed3f6eabedfceb9","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_d4c9cb78","line":1632,"in_reply_to":"df33271e_91c64867","updated":"2020-03-25 03:23:45.000000000","message":"If this exception path is hit, that means one of the following happened:\nA. Failed to invoke Cyborg\u0027s POST call to create ARQs in [1].\nB. Contacted Cyborg but creation of ARQs failed in Cyborg for some reason.\nC. ARQs got created but the next call to bind ARQs failed. This could\n   only be because one of these:\n   1. Failed to invoke Cyborg\u0027s PATCH call to bind ARQs in [1].\n   2. Contacted Cyborg but binding failed within Cyborg for some reason. \n      Note that the binding is async, so device config/programming errors will not cause this.\n      Today the binding failure should not happen, because Cyborg fails it for one of two validation errors:\n      a. The patch list for binding is bad [2]. Nova shouldn\u0027t hit this.\n      b. In the future, if Cyborg makes ARQ deletions async, the instance could be bound to\n         another set of ARQs during rescheduling due to a possible race. See [3]. But that\u0027s not the case today.\n\nFor cases A and B, no cleanup is needed. For C, technically, we should cleanup the newly created ARQs. However:\n\n . Note that C-1 is the main concern today, as C-2 should not happen today as things stand. \n . For the case C-1, there is no guarantee that the DELETE API call will succeed when the PATCH API call failed right before that.\n . Since the ARQs were not bound to an instance, delete_arqs_by_instance() [4] would not work. However, Cyborg has another form DELETE /accelerator_requests that deletes by ARQ UUIDs. \n . Though newly created (unbound) ARQs are relatively inexpensive resources (they only consume an entry in a db table), it is probably not a good idea to leave them around. \n . So, we should probably use the alternative API to delete ARQs by UUIDs. However, that call is not idempotent [5], so we have to take extra care not to call it twice.\n\nIt would be good to discuss this further.\n\n\n[1] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/accelerator/cyborg.py;hb\u003drefs/changes/35/673735/46#l89\n\n[2] https://opendev.org/openstack/cyborg/src/commit/aea02c1fc9ee55f64b84fb249d05939ca414b55a/cyborg/api/controllers/v2/arqs.py#L232 \n\n[3] https://review.opendev.org/712231\n\n[4] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/accelerator/cyborg.py;hb\u003drefs/changes/35/673735/46#l267 \n\n[5] https://review.opendev.org/714782","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"51d8657b02480bd69c920ccee4ad8e2e9334cedd","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_c7ed858c","line":1632,"in_reply_to":"df33271e_c2eaa073","updated":"2020-03-26 01:04:57.000000000","message":"Emm...sorry, after second think, I think it is fine. Since the instance is going to error status. The end user only can delete it. So when we delete it, the arq will be deleted. So we are fine at here.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"96c73598a789e9921c771ab8997d6a3d07ab92f7","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_c77624d9","line":1632,"in_reply_to":"df33271e_c7ed858c","updated":"2020-03-26 03:13:18.000000000","message":"We are in the for cycle [1], unless we can be sure that is the last scheduled, otherwise I think we should consider cleaning the arqs.\n\n[1]https://review.opendev.org/#/c/673735/46/nova/conductor/manager.py@1560","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"d050de760973fa2196740361668350fe6c34b317","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_2de08fcc","line":1632,"in_reply_to":"df33271e_d4c9cb78","updated":"2020-03-26 07:18:23.000000000","message":"Detail explanation to reviewers. \nThanks for your patient","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b846956bfc8b0dd3862fbb0dcb3b16e544d871be","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_2dec26c6","line":1632,"in_reply_to":"df33271e_d4c9cb78","updated":"2020-03-25 05:55:18.000000000","message":"This reply covered my comment in compute manager https://review.opendev.org/#/c/631244/69/nova/compute/manager.py@2238\n\nand in conductor manager https://review.opendev.org/#/c/631244/69/nova/conductor/manager.py@847\n\n\u003e Though newly created (unbound) ARQs are relatively inexpensive resources (they only consume an entry in a db table), it is probably not a good idea to leave them around. \n\nYes, sometimes those resources are scarce, and it\u0027s a good deal to clean up to provide rescheduling.\n\n\u003e . So, we should probably use the alternative API to delete ARQs by UUIDs. However, that call is not idempotent [5], so we have to take extra care not to call it twice.\n\nMaybe we should use DELETE /v2/accelerator_requests?arqs\u003duuid1,uuid2,... to cleanup the created arqs mapping in Cyborg db.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b7e843d5f1d22ea88bb6e12450f91e925d00ac51","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"            except Exception as exc:"},{"line_number":1630,"context_line":"                # If anything failed here we need to cleanup and bail out."},{"line_number":1631,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1632,"context_line":"                    self._cleanup_build_artifacts("},{"line_number":1633,"context_line":"                        context, exc, instances, build_requests, request_specs,"},{"line_number":1634,"context_line":"                        block_device_mapping, tags, cell_mapping_cache)"},{"line_number":1635,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_7379d5fa","line":1632,"in_reply_to":"df33271e_fcc93778","updated":"2020-03-27 07:53:20.000000000","message":"Agree with Brin: this is the instance creation path, so failures will cause rescheduling, not a movement to error state. So, I\u0027ll add the deletion of ARQs in a followup.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"}],"nova/tests/fixtures.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c40f2a86ccbc98f29b006a44e0918b7bc3539d42","unresolved":false,"context_lines":[{"line_number":2663,"context_line":"            \u0027nova.accelerator.cyborg._CyborgClient.\u0027"},{"line_number":2664,"context_line":"            \u0027get_arqs_for_instance\u0027,"},{"line_number":2665,"context_line":"            side_effect\u003dself.fake_get_arqs_for_instance)).mock"},{"line_number":2666,"context_line":"        self.mock_del_arqs \u003d self.useFixture(fixtures.MockPatch("},{"line_number":2667,"context_line":"            \u0027nova.accelerator.cyborg._CyborgClient.\u0027"},{"line_number":2668,"context_line":"            \u0027delete_arqs_for_instance\u0027,"},{"line_number":2669,"context_line":"            side_effect\u003dself.fake_delete_arqs_for_instance)).mock"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_bdeec485","line":2669,"range":{"start_line":2666,"start_character":6,"end_line":2669,"end_character":65},"updated":"2020-03-23 19:39:21.000000000","message":"by the way this is basically what self.stub_out does\n\ni thin kwe are ment to prefer this form but i see you are using both in this series depending on what is used in each file but i think we shoudl prefer this.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"965b48446e1961c09fcf4c191c06ef337830864f","unresolved":false,"context_lines":[{"line_number":7511,"context_line":"        self.assertEqual(\u0027ACTIVE\u0027, server[\u0027status\u0027])"},{"line_number":7512,"context_line":"        self._check_allocations_usage(server[\u0027id\u0027])"},{"line_number":7513,"context_line":""},{"line_number":7514,"context_line":"        # Delete server and check that ARQs got deleted"},{"line_number":7515,"context_line":"        self._delete_server(server)"},{"line_number":7516,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7517,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3fa7e38b_a836854e","line":7514,"updated":"2019-12-04 15:16:45.000000000","message":"You\u0027ve taken what used to be a very simple basic \"can I create a server\" test and turned it into a cyborg lifecycle test. I think you need to create your own test_create_and_delete_server_with_accel() test and stop abusing this one.","commit_id":"db84e0e29abdccf11368ac6b465a164c10e88c82"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b604d2ef91bfc88aa4f70ec2dc39730f1763a93b","unresolved":false,"context_lines":[{"line_number":7511,"context_line":"        self.assertEqual(\u0027ACTIVE\u0027, server[\u0027status\u0027])"},{"line_number":7512,"context_line":"        self._check_allocations_usage(server[\u0027id\u0027])"},{"line_number":7513,"context_line":""},{"line_number":7514,"context_line":"        # Delete server and check that ARQs got deleted"},{"line_number":7515,"context_line":"        self._delete_server(server)"},{"line_number":7516,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7517,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3fa7e38b_686e4d2d","line":7514,"in_reply_to":"3fa7e38b_a836854e","updated":"2019-12-04 15:19:00.000000000","message":"Sorry, ignore this. I had lost context that this was you own new test class from the limited scope gerrit shows me by default. The fact that this is not in its own test_accel.py file furthered the confusion.","commit_id":"db84e0e29abdccf11368ac6b465a164c10e88c82"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":7638,"context_line":"        # Delete server and check that ARQs got deleted"},{"line_number":7639,"context_line":"        self.api.delete_server(server[\u0027id\u0027])"},{"line_number":7640,"context_line":"        self._wait_until_deleted(server)"},{"line_number":7641,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7642,"context_line":""},{"line_number":7643,"context_line":"        # Check that resources are freed"},{"line_number":7644,"context_line":"        self._check_resources_are_freed()"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_aa2499b2","line":7641,"updated":"2020-02-18 11:54:01.000000000","message":"I think the below _check_resources_are_freed() covers the behavior well enough so that the mocking and asserting the mock calls are not needed.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":7638,"context_line":"        # Delete server and check that ARQs got deleted"},{"line_number":7639,"context_line":"        self.api.delete_server(server[\u0027id\u0027])"},{"line_number":7640,"context_line":"        self._wait_until_deleted(server)"},{"line_number":7641,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7642,"context_line":""},{"line_number":7643,"context_line":"        # Check that resources are freed"},{"line_number":7644,"context_line":"        self._check_resources_are_freed()"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_e2ceff8c","line":7641,"in_reply_to":"3fa7e38b_aa2499b2","updated":"2020-02-21 11:51:51.000000000","message":"\u003e I think the below _check_resources_are_freed() covers the behavior\n \u003e well enough so that the mocking and asserting the mock calls are\n \u003e not needed.\n\nThe _check_resources_are_freed() covers only Placement resources. We still want to check that del_arqs is called so that Cyborg resources are freed up.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3fa7e38b_25c45a61","line":7645,"updated":"2020-02-18 11:54:01.000000000","message":"We need to cover re-schedule cases:\n1) reschedule succeeds on the second host\n2) reschedule fails as we round out of alternative hosts","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3fa7e38b_82d5cb1c","line":7645,"in_reply_to":"3fa7e38b_25c45a61","updated":"2020-02-21 11:51:51.000000000","message":"Ok.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":7702,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7703,"context_line":""},{"line_number":7704,"context_line":"        # Check that resources are freed"},{"line_number":7705,"context_line":"        self._check_resources_are_freed()"},{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":""},{"line_number":7708,"context_line":"class AcceleratorServerReschedTest(AcceleratorServerBase):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_6d1a1b18","line":7705,"updated":"2020-03-13 17:36:29.000000000","message":"This tests the happy path to make sure if we delete an instance normally the ARQs go with it. Seems like we should also have a functional level test that causes resource claim to fail or something similar (i.e. in build_resources) and ensure that not only do the ARQs get deleted as expected, but that we can still delete the instance cleanly from error state. It\u0027s not uncommon for us to have undelete-able instances because our delete paths don\u0027t handle broken things well. It\u0027s super frustrating to not be able to delete stuff.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":7702,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7703,"context_line":""},{"line_number":7704,"context_line":"        # Check that resources are freed"},{"line_number":7705,"context_line":"        self._check_resources_are_freed()"},{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":""},{"line_number":7708,"context_line":"class AcceleratorServerReschedTest(AcceleratorServerBase):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_c85de147","line":7705,"in_reply_to":"1fa4df85_39b97714","updated":"2020-03-22 01:28:06.000000000","message":"I\u0027ll add this in https://review.opendev.org/699553, which adds support for stopping instances with accelerators.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":7702,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7703,"context_line":""},{"line_number":7704,"context_line":"        # Check that resources are freed"},{"line_number":7705,"context_line":"        self._check_resources_are_freed()"},{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":""},{"line_number":7708,"context_line":"class AcceleratorServerReschedTest(AcceleratorServerBase):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_39b97714","line":7705,"in_reply_to":"1fa4df85_6d1a1b18","updated":"2020-03-19 12:28:08.000000000","message":"Also it would be nice to see a local delete functional test:\n* create a server\n* stop (and force the compute down to avoid waiting)\n* delete the server\n* assert the cleanup\n* optionally start up the compute to see if this does not generate any error cases.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":7702,"context_line":"        self.cyborg.mock_del_arqs.assert_called_once_with(server[\u0027id\u0027])"},{"line_number":7703,"context_line":""},{"line_number":7704,"context_line":"        # Check that resources are freed"},{"line_number":7705,"context_line":"        self._check_resources_are_freed()"},{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":""},{"line_number":7708,"context_line":"class AcceleratorServerReschedTest(AcceleratorServerBase):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_ab989fbf","line":7705,"in_reply_to":"1fa4df85_6d1a1b18","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":7711,"context_line":"        self.NUM_HOSTS \u003d 2"},{"line_number":7712,"context_line":"        super(AcceleratorServerReschedTest, self).setUp()"},{"line_number":7713,"context_line":""},{"line_number":7714,"context_line":"    def _check_allocations_usage_resched(self, server):"},{"line_number":7715,"context_line":"        # Check allocations on host where instance is running"},{"line_number":7716,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7717,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_68e14d9b","line":7714,"updated":"2020-03-22 01:28:06.000000000","message":"Merged this with _check_allocations_usage().","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":7749,"context_line":"                         for arq in arqs}"},{"line_number":7750,"context_line":"        self.assertSetEqual(expected_arq_bind_info, arq_bind_info)"},{"line_number":7751,"context_line":""},{"line_number":7752,"context_line":"    def _check_no_allocations(self, server_uuid):"},{"line_number":7753,"context_line":"        allocs \u003d self._get_allocations_by_server_uuid(server_uuid)"},{"line_number":7754,"context_line":"        self.assertEqual(allocs, {})"},{"line_number":7755,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_f9d75f4a","line":7752,"range":{"start_line":7752,"start_character":8,"end_line":7752,"end_character":29},"updated":"2020-03-19 12:28:08.000000000","message":"I think this and _check_resources_are_freed function have the same goal so they can be merged.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":7749,"context_line":"                         for arq in arqs}"},{"line_number":7750,"context_line":"        self.assertSetEqual(expected_arq_bind_info, arq_bind_info)"},{"line_number":7751,"context_line":""},{"line_number":7752,"context_line":"    def _check_no_allocations(self, server_uuid):"},{"line_number":7753,"context_line":"        allocs \u003d self._get_allocations_by_server_uuid(server_uuid)"},{"line_number":7754,"context_line":"        self.assertEqual(allocs, {})"},{"line_number":7755,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_a8dbc5ea","line":7752,"range":{"start_line":7752,"start_character":8,"end_line":7752,"end_character":29},"in_reply_to":"1fa4df85_f9d75f4a","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":7756,"context_line":"        for i in range(self.NUM_HOSTS):"},{"line_number":7757,"context_line":"            usage \u003d self._get_provider_usages("},{"line_number":7758,"context_line":"                self.device_rp_uuids[i]).get(\u0027FPGA\u0027)"},{"line_number":7759,"context_line":"            self.assertEqual(usage, 0)"},{"line_number":7760,"context_line":""},{"line_number":7761,"context_line":"    def test_resched(self):"},{"line_number":7762,"context_line":"        orig_spawn \u003d fake.FakeDriver.spawn"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_997aeb74","line":7759,"updated":"2020-03-19 12:28:08.000000000","message":"Here you can check now that there the ARQs are also deleted.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":7756,"context_line":"        for i in range(self.NUM_HOSTS):"},{"line_number":7757,"context_line":"            usage \u003d self._get_provider_usages("},{"line_number":7758,"context_line":"                self.device_rp_uuids[i]).get(\u0027FPGA\u0027)"},{"line_number":7759,"context_line":"            self.assertEqual(usage, 0)"},{"line_number":7760,"context_line":""},{"line_number":7761,"context_line":"    def test_resched(self):"},{"line_number":7762,"context_line":"        orig_spawn \u003d fake.FakeDriver.spawn"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_a88985ca","line":7759,"in_reply_to":"1fa4df85_997aeb74","updated":"2020-03-22 01:28:06.000000000","message":"Instead I mocked this in the calling function. Reason: while I could arrange for fake_delete_arqs_for_instance() to delete self.cyborg.arq_list in the fixture, that is equivalent to the mocking, but requires more changes to the fixture.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c40f2a86ccbc98f29b006a44e0918b7bc3539d42","unresolved":false,"context_lines":[{"line_number":7929,"context_line":"            raise exception.BuildAbortException(reason\u003d\u0027\u0027,"},{"line_number":7930,"context_line":"                    instance_uuid\u003d\u0027fake\u0027)"},{"line_number":7931,"context_line":""},{"line_number":7932,"context_line":"        self.stub_out(\u0027nova.virt.fake.FakeDriver.spawn\u0027, throw_error)"},{"line_number":7933,"context_line":""},{"line_number":7934,"context_line":"        server \u003d self._get_server(expected_state\u003d\u0027ERROR\u0027)"},{"line_number":7935,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_7dc27ceb","line":7932,"range":{"start_line":7932,"start_character":8,"end_line":7932,"end_character":69},"updated":"2020-03-23 19:39:21.000000000","message":"so this woudl become \n self.useFixture(fixtures.MockPatch(\u0027nova.virt.fake.FakeDriver.spawn\u0027, throw_error)\n\nself.stub_out is shorter so its fine but just and fyi.\nhttps://github.com/openstack/nova/blob/0f81adfaa3f493b4397fcea9339ab14dfeb3ef45/nova/test.py#L341-L348\n\ni dont think its really important and certenly not important enough to respin this on its own. this can always be change in a follow up patch if we want in the future.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"9d5cab05c45d8e4f8e31e4f6808d6581cbfe716c","unresolved":false,"context_lines":[{"line_number":7929,"context_line":"            raise exception.BuildAbortException(reason\u003d\u0027\u0027,"},{"line_number":7930,"context_line":"                    instance_uuid\u003d\u0027fake\u0027)"},{"line_number":7931,"context_line":""},{"line_number":7932,"context_line":"        self.stub_out(\u0027nova.virt.fake.FakeDriver.spawn\u0027, throw_error)"},{"line_number":7933,"context_line":""},{"line_number":7934,"context_line":"        server \u003d self._get_server(expected_state\u003d\u0027ERROR\u0027)"},{"line_number":7935,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_8d4a9224","line":7932,"range":{"start_line":7932,"start_character":8,"end_line":7932,"end_character":69},"in_reply_to":"df33271e_7dc27ceb","updated":"2020-03-25 14:18:52.000000000","message":"Since stub_out is shorter and means the same, why do we need to change it?","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4acb26725b90b66eacd831d8d04f96091b052713","unresolved":false,"context_lines":[{"line_number":7940,"context_line":""},{"line_number":7941,"context_line":"        self.api.delete_server(server_uuid)"},{"line_number":7942,"context_line":"        self._wait_until_deleted(server)"},{"line_number":7943,"context_line":"        # Verify that there is one more call to delete ARQs"},{"line_number":7944,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7945,"context_line":"            [mock.call(server_uuid), mock.call(server_uuid)])"},{"line_number":7946,"context_line":""},{"line_number":7947,"context_line":"        # Verify that no allocations/usages remain after deletion"},{"line_number":7948,"context_line":"        self._check_no_allocs_usage(server_uuid)"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_beffe765","line":7945,"range":{"start_line":7943,"start_character":0,"end_line":7945,"end_character":61},"updated":"2020-03-23 11:15:39.000000000","message":"So this means that nova tries to delete the same ARQs in cyborg twice? Is this OK from cyborg perspective? Does the second delete will return 404 or 410? I think If cyborg returns 4xx in this case then nova will log an exception trace at https://review.opendev.org/#/c/673735/45..46/nova/compute/utils.py@1565 As this delete is actually successful (no ARQs exists any more) it should not pollute the log with exception backtraces.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1a597e9768d6c1c41d6bb987b9009e75acb1aecc","unresolved":false,"context_lines":[{"line_number":7940,"context_line":""},{"line_number":7941,"context_line":"        self.api.delete_server(server_uuid)"},{"line_number":7942,"context_line":"        self._wait_until_deleted(server)"},{"line_number":7943,"context_line":"        # Verify that there is one more call to delete ARQs"},{"line_number":7944,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7945,"context_line":"            [mock.call(server_uuid), mock.call(server_uuid)])"},{"line_number":7946,"context_line":""},{"line_number":7947,"context_line":"        # Verify that no allocations/usages remain after deletion"},{"line_number":7948,"context_line":"        self._check_no_allocs_usage(server_uuid)"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_3e5a8ef2","line":7945,"range":{"start_line":7943,"start_character":0,"end_line":7945,"end_character":61},"in_reply_to":"df33271e_91bc8806","updated":"2020-03-25 08:55:50.000000000","message":"Thanks. As cyborg accepts duplicate delete and returns 2xx this is OK.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"5275da49ba5826d2238fbced1ed3f6eabedfceb9","unresolved":false,"context_lines":[{"line_number":7940,"context_line":""},{"line_number":7941,"context_line":"        self.api.delete_server(server_uuid)"},{"line_number":7942,"context_line":"        self._wait_until_deleted(server)"},{"line_number":7943,"context_line":"        # Verify that there is one more call to delete ARQs"},{"line_number":7944,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7945,"context_line":"            [mock.call(server_uuid), mock.call(server_uuid)])"},{"line_number":7946,"context_line":""},{"line_number":7947,"context_line":"        # Verify that no allocations/usages remain after deletion"},{"line_number":7948,"context_line":"        self._check_no_allocs_usage(server_uuid)"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_91bc8806","line":7945,"range":{"start_line":7943,"start_character":0,"end_line":7945,"end_character":61},"in_reply_to":"df33271e_beffe765","updated":"2020-03-25 03:23:45.000000000","message":"Cyborg provides two ways to delete ARQs: one that takes a list of ARQ UUIDs, and another that takes the UUID of the instance whose ARQs need to be removed. The former would raise an error on duplicate removal, while the latter is idempotent (would not raise an error on duplicates). Nova uses the latter.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4acb26725b90b66eacd831d8d04f96091b052713","unresolved":false,"context_lines":[{"line_number":7996,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7997,"context_line":"        self._check_no_allocs_usage(server_uuid)"},{"line_number":7998,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7999,"context_line":"            [mock.call(server_uuid),"},{"line_number":8000,"context_line":"             mock.call(server_uuid),"},{"line_number":8001,"context_line":"             mock.call(server_uuid)])"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_5ef4db5b","line":8001,"range":{"start_line":7999,"start_character":0,"end_line":8001,"end_character":37},"updated":"2020-03-23 11:15:39.000000000","message":"Why we delete 3 times? We only have 2 computes as far as I see.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1a597e9768d6c1c41d6bb987b9009e75acb1aecc","unresolved":false,"context_lines":[{"line_number":7996,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7997,"context_line":"        self._check_no_allocs_usage(server_uuid)"},{"line_number":7998,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7999,"context_line":"            [mock.call(server_uuid),"},{"line_number":8000,"context_line":"             mock.call(server_uuid),"},{"line_number":8001,"context_line":"             mock.call(server_uuid)])"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_7e737681","line":8001,"range":{"start_line":7999,"start_character":0,"end_line":8001,"end_character":37},"in_reply_to":"df33271e_4c24230e","updated":"2020-03-25 08:55:50.000000000","message":"OK. Thanks.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c40f2a86ccbc98f29b006a44e0918b7bc3539d42","unresolved":false,"context_lines":[{"line_number":7996,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7997,"context_line":"        self._check_no_allocs_usage(server_uuid)"},{"line_number":7998,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7999,"context_line":"            [mock.call(server_uuid),"},{"line_number":8000,"context_line":"             mock.call(server_uuid),"},{"line_number":8001,"context_line":"             mock.call(server_uuid)])"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_7d355cf7","line":8001,"range":{"start_line":7999,"start_character":0,"end_line":8001,"end_character":37},"in_reply_to":"df33271e_5ef4db5b","updated":"2020-03-23 19:39:21.000000000","message":"i suspect there are 3 calls because that is the default we will try the first host, fail and delete the arqs un clean-up retry the second and clean-up, finally we get a no vaild host and likely delete the arqs one last time.\n\nthat is just a guess but i assume that is what is going on here.","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f70c92ea0e20dfeae85fe8025d409c49a1884b38","unresolved":false,"context_lines":[{"line_number":7996,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7997,"context_line":"        self._check_no_allocs_usage(server_uuid)"},{"line_number":7998,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7999,"context_line":"            [mock.call(server_uuid),"},{"line_number":8000,"context_line":"             mock.call(server_uuid),"},{"line_number":8001,"context_line":"             mock.call(server_uuid)])"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_9d913f9d","line":8001,"range":{"start_line":7999,"start_character":0,"end_line":8001,"end_character":37},"in_reply_to":"df33271e_7d355cf7","updated":"2020-03-23 19:51:24.000000000","message":"so yes two of the deletes are form delete_instance in the compute manager\nhttps://review.opendev.org/#/c/673735/46/nova/compute/manager.py@2981\n\nand the third one i think is from here\nhttps://review.opendev.org/#/c/673735/46/nova/conductor/manager.py@598","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"5275da49ba5826d2238fbced1ed3f6eabedfceb9","unresolved":false,"context_lines":[{"line_number":7996,"context_line":"        server_uuid \u003d server[\u0027id\u0027]"},{"line_number":7997,"context_line":"        self._check_no_allocs_usage(server_uuid)"},{"line_number":7998,"context_line":"        self.cyborg.mock_del_arqs.assert_has_calls("},{"line_number":7999,"context_line":"            [mock.call(server_uuid),"},{"line_number":8000,"context_line":"             mock.call(server_uuid),"},{"line_number":8001,"context_line":"             mock.call(server_uuid)])"}],"source_content_type":"text/x-python","patch_set":46,"id":"df33271e_4c24230e","line":8001,"range":{"start_line":7999,"start_character":0,"end_line":8001,"end_character":37},"in_reply_to":"df33271e_9d913f9d","updated":"2020-03-25 03:23:45.000000000","message":"I traced it to these three call sites:\n\n /opt/stack/nova/nova/compute/manager.py, line 2640, in _build_resources [1] \n /opt/stack/nova/nova/compute/manager.py, line 2640, in _build_resources  [1] \n /opt/stack/nova/nova/conductor/manager.py, line 598, in _cleanup_when_reschedule_fails [2]\n\nSo, I think Sean is close. Since spawn is mocked to raise exceptions, _build_resources would raise BuildAbortException and then clean up ARQs. This happens for both the instances, resulting in 2 cleanups, and eventually the call to  _cleanup_when_reschedule_fails() cleans it up once more.\n\nAs I said elsewhere, the extra call is fine because Cyborg\u0027s API to delete ARQs by instance UUID is idempotent.\n\n[1] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/compute/manager.py;hb\u003drefs/changes/35/673735/46#l2640\n[2] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/conductor/manager.py;hb\u003drefs/changes/35/673735/46#l598","commit_id":"f68d0b66c012327907840fab04db3dbcebc343e4"}],"nova/tests/unit/accelerator/test_cyborg.py":[{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3fa7e38b_5a008802","line":243,"updated":"2019-10-15 18:10:20.000000000","message":"Should include a test for the call to `self._client.delete`","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3fa7e38b_870ddde9","line":243,"in_reply_to":"3fa7e38b_5a008802","updated":"2019-10-30 02:42:30.000000000","message":"The self._client.delete is a method in keystoneauth1.adapter object. We can mock it to return a non-NULL value, but delete_arqs_for_instance() does not do anything if the return is not NULL [1].\n\n[1] https://review.opendev.org/#/c/673735/15/nova/accelerator/cyborg.py@222","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3fa7e38b_4505d667","line":345,"updated":"2020-02-18 11:54:01.000000000","message":"The happy case is missing when delete_arqs_for_instance calls forward to _call_cyborg with proper params (instance uuid filter)","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3fa7e38b_22e737da","line":345,"in_reply_to":"3fa7e38b_4505d667","updated":"2020-02-21 11:51:51.000000000","message":"Done","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        # If Cyborg returns invalid response, raise exception."},{"line_number":363,"context_line":"        mock_call_cyborg.return_value \u003d (None, \u0027Some error\u0027)"},{"line_number":364,"context_line":"        self.assertRaises(exception.AcceleratorRequestOpFailed,"},{"line_number":365,"context_line":"            self.client.delete_arqs_for_instance, instance_uuid\u003dNone)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_6d90db96","line":365,"updated":"2020-03-13 17:36:29.000000000","message":"Since you\u0027re doing surgery on the message (which I think is incorrect per my other comment), it would be good to assert the format of that message as something sane. Since assertRaises returns the exception, you can grab it here and assert the string matches what we expect.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        # If Cyborg returns invalid response, raise exception."},{"line_number":363,"context_line":"        mock_call_cyborg.return_value \u003d (None, \u0027Some error\u0027)"},{"line_number":364,"context_line":"        self.assertRaises(exception.AcceleratorRequestOpFailed,"},{"line_number":365,"context_line":"            self.client.delete_arqs_for_instance, instance_uuid\u003dNone)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_0859b933","line":365,"in_reply_to":"1fa4df85_6d90db96","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"4b76e62e549f0af6f51d5c0f967b9b5eefbe5b08","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1275,"context_line":"        self.compute_api.delete_arqs(self.context, inst)"},{"line_number":1276,"context_line":"        mock_del_arq.assert_not_called()"},{"line_number":1277,"context_line":""},{"line_number":1278,"context_line":"    @mock.patch.object(compute_utils, \u0027notify_about_instance_action\u0027)"},{"line_number":1279,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027destroy\u0027)"},{"line_number":1280,"context_line":"    @mock.patch.object(cinder.API, \u0027detach\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_fa1b747d","line":1277,"updated":"2019-10-15 18:10:20.000000000","message":"I think it should  have a negative  test for `dp_name is None`","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f45aeee4eef717a6d854395fa06192958b042e77","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1275,"context_line":"        self.compute_api.delete_arqs(self.context, inst)"},{"line_number":1276,"context_line":"        mock_del_arq.assert_not_called()"},{"line_number":1277,"context_line":""},{"line_number":1278,"context_line":"    @mock.patch.object(compute_utils, \u0027notify_about_instance_action\u0027)"},{"line_number":1279,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027destroy\u0027)"},{"line_number":1280,"context_line":"    @mock.patch.object(cinder.API, \u0027detach\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa7e38b_e76131aa","line":1277,"in_reply_to":"3fa7e38b_fa1b747d","updated":"2019-10-30 02:42:30.000000000","message":"That is the test above.","commit_id":"e491cc82a70a46036758df6b51bab0525ae605aa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":894,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":895,"context_line":"        inst.update(attrs)"},{"line_number":896,"context_line":"        inst._context \u003d self.context"},{"line_number":897,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":898,"context_line":"        deltas \u003d {\u0027instances\u0027: -1,"},{"line_number":899,"context_line":"                  \u0027cores\u0027: -inst.flavor.vcpus,"},{"line_number":900,"context_line":"                  \u0027ram\u0027: -inst.flavor.memory_mb}"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_79a04f91","line":897,"updated":"2020-03-19 12:28:08.000000000","message":"I think this is unnecessary. I removed it and the tests still passing. (The flavor already has extra spec)","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":894,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":895,"context_line":"        inst.update(attrs)"},{"line_number":896,"context_line":"        inst._context \u003d self.context"},{"line_number":897,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":898,"context_line":"        deltas \u003d {\u0027instances\u0027: -1,"},{"line_number":899,"context_line":"                  \u0027cores\u0027: -inst.flavor.vcpus,"},{"line_number":900,"context_line":"                  \u0027ram\u0027: -inst.flavor.memory_mb}"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_68606dd0","line":897,"in_reply_to":"1fa4df85_79a04f91","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":1274,"context_line":"        inst._context \u003d self.context"},{"line_number":1275,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1276,"context_line":"        mock_elevated.return_value \u003d self.context"},{"line_number":1277,"context_line":"        mock_detach.side_effect \u003d exception.VolumeNotFound(\u0027volume_id\u0027)"},{"line_number":1278,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_793ccf2e","line":1275,"updated":"2020-03-19 12:28:08.000000000","message":"ditto","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":1274,"context_line":"        inst._context \u003d self.context"},{"line_number":1275,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1276,"context_line":"        mock_elevated.return_value \u003d self.context"},{"line_number":1277,"context_line":"        mock_detach.side_effect \u003d exception.VolumeNotFound(\u0027volume_id\u0027)"},{"line_number":1278,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_885da916","line":1275,"in_reply_to":"1fa4df85_793ccf2e","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"            mock_bdm_destroy, mock_notify):"},{"line_number":1313,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":1314,"context_line":"        inst._context \u003d self.context"},{"line_number":1315,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1316,"context_line":"        mock_elevated.return_value \u003d self.context"},{"line_number":1317,"context_line":"        bdms \u003d []"},{"line_number":1318,"context_line":"        self.compute_api._local_delete(self.context, inst, bdms,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_19459bb4","line":1315,"updated":"2020-03-19 12:28:08.000000000","message":"ditto","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"            mock_bdm_destroy, mock_notify):"},{"line_number":1313,"context_line":"        inst \u003d self._create_instance_obj()"},{"line_number":1314,"context_line":"        inst._context \u003d self.context"},{"line_number":1315,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":1316,"context_line":"        mock_elevated.return_value \u003d self.context"},{"line_number":1317,"context_line":"        bdms \u003d []"},{"line_number":1318,"context_line":"        self.compute_api._local_delete(self.context, inst, bdms,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_289cb5be","line":1315,"in_reply_to":"1fa4df85_19459bb4","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":6954,"context_line":""},{"line_number":6955,"context_line":"        params \u003d {\u0027host\u0027: None, \u0027vm_state\u0027: vm_state}"},{"line_number":6956,"context_line":"        inst \u003d self._create_instance_obj(params\u003dparams)"},{"line_number":6957,"context_line":"        inst.flavor[\u0027extra_specs\u0027] \u003d {}"},{"line_number":6958,"context_line":"        mock_lookup.return_value \u003d None, inst"},{"line_number":6959,"context_line":"        connector \u003d conn_info[\u0027connector\u0027]"},{"line_number":6960,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_e8a1bdf8","line":6957,"updated":"2020-03-22 01:28:06.000000000","message":"Removed this too","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":6203,"context_line":""},{"line_number":6204,"context_line":"        try:"},{"line_number":6205,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6206,"context_line":"                pass"},{"line_number":6207,"context_line":"        except exception.BuildAbortException:"},{"line_number":6208,"context_line":"            pass"},{"line_number":6209,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_d049e665","line":6206,"updated":"2020-01-06 19:50:04.000000000","message":"Per the comment in the compute manager, you need a case where you raise an exception in here.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":6203,"context_line":""},{"line_number":6204,"context_line":"        try:"},{"line_number":6205,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6206,"context_line":"                pass"},{"line_number":6207,"context_line":"        except exception.BuildAbortException:"},{"line_number":6208,"context_line":"            pass"},{"line_number":6209,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_d7dbaa4b","line":6206,"in_reply_to":"3fa7e38b_d049e665","updated":"2020-01-14 21:40:16.000000000","message":"Done","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":6208,"context_line":"            pass"},{"line_number":6209,"context_line":""},{"line_number":6210,"context_line":"    @mock.patch.object(nova.accelerator.cyborg, \u0027delete_arqs\u0027)"},{"line_number":6211,"context_line":"    def test_delete_arqs(self, mock_del_arqs):"},{"line_number":6212,"context_line":"        # ARQs get deleted if _get_bound_arq_resources fails."},{"line_number":6213,"context_line":"        self.instance.flavor.extra_specs \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":6214,"context_line":"        self._test_delete_arqs_if_bind_fails()"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_8d3efbe0","line":6211,"range":{"start_line":6211,"start_character":8,"end_line":6211,"end_character":24},"updated":"2020-01-06 19:50:04.000000000","message":"This seems like a happy path test name, but it appears to be testing the case where build fails.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":6208,"context_line":"            pass"},{"line_number":6209,"context_line":""},{"line_number":6210,"context_line":"    @mock.patch.object(nova.accelerator.cyborg, \u0027delete_arqs\u0027)"},{"line_number":6211,"context_line":"    def test_delete_arqs(self, mock_del_arqs):"},{"line_number":6212,"context_line":"        # ARQs get deleted if _get_bound_arq_resources fails."},{"line_number":6213,"context_line":"        self.instance.flavor.extra_specs \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":6214,"context_line":"        self._test_delete_arqs_if_bind_fails()"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_77e0b6fa","line":6211,"range":{"start_line":6211,"start_character":8,"end_line":6211,"end_character":24},"in_reply_to":"3fa7e38b_8d3efbe0","updated":"2020-01-14 21:40:16.000000000","message":"Done","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70c73eefed394a85c9760bbbd1a9792fbc08db70","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"        # If dp_name is None, errors have no effect"},{"line_number":6220,"context_line":"        self.instance.flavor.extra_specs \u003d {}"},{"line_number":6221,"context_line":"        self._test_delete_arqs_if_bind_fails()"},{"line_number":6222,"context_line":"        mock_del_arqs.assert_not_called()"},{"line_number":6223,"context_line":""},{"line_number":6224,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"},{"line_number":6225,"context_line":"        self._test_build_and_run_instance()"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_2dfd8708","line":6222,"updated":"2020-01-06 19:50:04.000000000","message":"This should still be called even if we don\u0027t have a device profile, since that is checked _inside_ this method right? Perhaps you\u0027re not actually raising the error you think you are because you have no profile name in extra_specs for this test?\n\nTo catch this, you should be asserting that something else is called that is always going to be called, probably in the _test_delete_arqs_if_bind_fails() inner helper so that you\u0027re always making sure that things ran as expected, given everything is a \"pass\" in there.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a079d94bdffe901475d3f809bf4c71de2b788802","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"        # If dp_name is None, errors have no effect"},{"line_number":6220,"context_line":"        self.instance.flavor.extra_specs \u003d {}"},{"line_number":6221,"context_line":"        self._test_delete_arqs_if_bind_fails()"},{"line_number":6222,"context_line":"        mock_del_arqs.assert_not_called()"},{"line_number":6223,"context_line":""},{"line_number":6224,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"},{"line_number":6225,"context_line":"        self._test_build_and_run_instance()"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_37a55e49","line":6222,"in_reply_to":"3fa7e38b_2dfd8708","updated":"2020-01-14 21:40:16.000000000","message":"This test case calls _test_delete_arqs_if_bind_fails, which calls _build_resources. The _build_resources makes a check if there is a device profile involved [1].\n\n[1] https://review.opendev.org/#/c/673735/27/nova/compute/manager.py@2677","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3244bdb593ba7d0b2ec3ff54c7c1dd249877b3b3","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"        # If dp_name is None, errors have no effect"},{"line_number":6220,"context_line":"        self.instance.flavor.extra_specs \u003d {}"},{"line_number":6221,"context_line":"        self._test_delete_arqs_if_bind_fails()"},{"line_number":6222,"context_line":"        mock_del_arqs.assert_not_called()"},{"line_number":6223,"context_line":""},{"line_number":6224,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"},{"line_number":6225,"context_line":"        self._test_build_and_run_instance()"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fa7e38b_816ddf30","line":6222,"in_reply_to":"3fa7e38b_37a55e49","updated":"2020-01-15 15:01:22.000000000","message":"It\u0027s really hard to have to rebuild context on this over a week later. However, I think I was probably looking at L2722 for this and thinking that you were triggering that exception handler here. Looking now, I see it\u0027s testing the pre-fetch of those things and making sure it falls through if there\u0027s no dp_name.","commit_id":"4a0c0a1ffd364c12f936a2c8576f7fdcb11a1986"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"934df6b23bc9a290806736b5f91da7d17bb52eb1","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        # to test, then use call_tracker to verify call sequence"},{"line_number":225,"context_line":"        specd_compute._delete_instance \u003d orig_delete"},{"line_number":226,"context_line":"        specd_compute.host \u003d \u0027compute\u0027"},{"line_number":227,"context_line":"        specd_compute.compute_api \u003d compute_api.API()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        mock_inst \u003d mock.Mock()"},{"line_number":230,"context_line":"        mock_inst.uuid \u003d uuids.instance"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_a55e0a76","line":227,"updated":"2020-02-18 11:54:01.000000000","message":"seems unnecessary. I reverted this and the test passed.","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4e58f30dab29c7309704f2bcbfd90b397e0b87bb","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        # to test, then use call_tracker to verify call sequence"},{"line_number":225,"context_line":"        specd_compute._delete_instance \u003d orig_delete"},{"line_number":226,"context_line":"        specd_compute.host \u003d \u0027compute\u0027"},{"line_number":227,"context_line":"        specd_compute.compute_api \u003d compute_api.API()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        mock_inst \u003d mock.Mock()"},{"line_number":230,"context_line":"        mock_inst.uuid \u003d uuids.instance"}],"source_content_type":"text/x-python","patch_set":38,"id":"3fa7e38b_428d9334","line":227,"in_reply_to":"3fa7e38b_a55e0a76","updated":"2020-02-21 11:51:51.000000000","message":"Done","commit_id":"4d448687f4e580d4cc5e77d20c51d9a86848a5c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d2eee7d4b8259955930cfd0db2db8a780c8b0b7f","unresolved":false,"context_lines":[{"line_number":6287,"context_line":"            exception.AcceleratorRequestOpFailed(op\u003d\u0027get\u0027, msg\u003d\u0027\u0027))"},{"line_number":6288,"context_line":""},{"line_number":6289,"context_line":"        with self.compute._build_resources(*args):"},{"line_number":6290,"context_line":"            raise exception.NovaException"},{"line_number":6291,"context_line":""},{"line_number":6292,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6293,"context_line":"    def test_delete_arqs_on_build_res_exception(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":40,"id":"1fa4df85_0d932251","line":6290,"updated":"2020-03-02 16:45:25.000000000","message":"You should use TestingException unless there\u0027s some reason why it needs to be a novaexception. This helps to avoid confusion in your wrapper below about whether or not you\u0027re catching the test-induced failure, or something else.","commit_id":"15ced2efe995256695055fa29cac435cac0712bb"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a760d74011538adc9b62df0d139b5e2df2d30171","unresolved":false,"context_lines":[{"line_number":6287,"context_line":"            exception.AcceleratorRequestOpFailed(op\u003d\u0027get\u0027, msg\u003d\u0027\u0027))"},{"line_number":6288,"context_line":""},{"line_number":6289,"context_line":"        with self.compute._build_resources(*args):"},{"line_number":6290,"context_line":"            raise exception.NovaException"},{"line_number":6291,"context_line":""},{"line_number":6292,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6293,"context_line":"    def test_delete_arqs_on_build_res_exception(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":40,"id":"1fa4df85_cfa17aab","line":6290,"in_reply_to":"1fa4df85_0d932251","updated":"2020-03-03 04:12:48.000000000","message":"Done","commit_id":"15ced2efe995256695055fa29cac435cac0712bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d2eee7d4b8259955930cfd0db2db8a780c8b0b7f","unresolved":false,"context_lines":[{"line_number":6296,"context_line":"        self.instance.flavor.extra_specs \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":6297,"context_line":"        try:"},{"line_number":6298,"context_line":"            self._test_delete_arqs_if_build_res_exception()"},{"line_number":6299,"context_line":"        except exception.NovaException:  # expected by test case design"},{"line_number":6300,"context_line":"            pass"},{"line_number":6301,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6302,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"1fa4df85_ad844e0d","line":6299,"updated":"2020-03-02 16:45:25.000000000","message":"Use self.assertRaises() here.","commit_id":"15ced2efe995256695055fa29cac435cac0712bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af4897b25cdd986cb245c647aacbbc5068b85538","unresolved":false,"context_lines":[{"line_number":6296,"context_line":"        self.instance.flavor.extra_specs \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":6297,"context_line":"        try:"},{"line_number":6298,"context_line":"            self._test_delete_arqs_if_build_res_exception()"},{"line_number":6299,"context_line":"        except exception.NovaException:  # expected by test case design"},{"line_number":6300,"context_line":"            pass"},{"line_number":6301,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6302,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"1fa4df85_3505f314","line":6299,"in_reply_to":"1fa4df85_99f0abff","updated":"2020-03-03 15:46:02.000000000","message":"Testing the more specific exception is not the point. See my comments on the next patchset.","commit_id":"15ced2efe995256695055fa29cac435cac0712bb"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a760d74011538adc9b62df0d139b5e2df2d30171","unresolved":false,"context_lines":[{"line_number":6296,"context_line":"        self.instance.flavor.extra_specs \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":6297,"context_line":"        try:"},{"line_number":6298,"context_line":"            self._test_delete_arqs_if_build_res_exception()"},{"line_number":6299,"context_line":"        except exception.NovaException:  # expected by test case design"},{"line_number":6300,"context_line":"            pass"},{"line_number":6301,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6302,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"1fa4df85_99f0abff","line":6299,"in_reply_to":"1fa4df85_ad844e0d","updated":"2020-03-03 04:12:48.000000000","message":"Merged this with previous function and tested for a more specific exception.","commit_id":"15ced2efe995256695055fa29cac435cac0712bb"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"aa2106cfebe0b611bbcaed98d37b88f251a316b2","unresolved":false,"context_lines":[{"line_number":6245,"context_line":"        try:"},{"line_number":6246,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6247,"context_line":"                pass"},{"line_number":6248,"context_line":"        except exception.BuildAbortException:"},{"line_number":6249,"context_line":"            pass"},{"line_number":6250,"context_line":""},{"line_number":6251,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6252,"context_line":"    def test_delete_arqs_on_bind_failure(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_8d02a53c","line":6249,"range":{"start_line":6248,"start_character":0,"end_line":6249,"end_character":16},"updated":"2020-03-03 11:14:05.000000000","message":"This can be instead of bellow, that we can assert the exception is what we want to get:\n        except Exception as e:\n            self.assertIsInstance(e, exception.BuildAbortException)","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f2fd755ca5c39f127feb6d8d924c8fb16e50f78b","unresolved":false,"context_lines":[{"line_number":6245,"context_line":"        try:"},{"line_number":6246,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6247,"context_line":"                pass"},{"line_number":6248,"context_line":"        except exception.BuildAbortException:"},{"line_number":6249,"context_line":"            pass"},{"line_number":6250,"context_line":""},{"line_number":6251,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6252,"context_line":"    def test_delete_arqs_on_bind_failure(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_99dff467","line":6249,"range":{"start_line":6248,"start_character":0,"end_line":6249,"end_character":16},"in_reply_to":"1fa4df85_8d02a53c","updated":"2020-03-08 23:24:47.000000000","message":"Done","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"aa2106cfebe0b611bbcaed98d37b88f251a316b2","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        try:"},{"line_number":6294,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6295,"context_line":"                raise test.TestingException()"},{"line_number":6296,"context_line":"        except exception.BuildAbortException:"},{"line_number":6297,"context_line":"            pass"},{"line_number":6298,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6299,"context_line":""},{"line_number":6300,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_edf1d96c","line":6297,"range":{"start_line":6296,"start_character":8,"end_line":6297,"end_character":16},"updated":"2020-03-03 11:14:05.000000000","message":"ditto.\n\n        except Exception as e:\n            self.assertIsInstance(e, exception.BuildAbortException)","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b9d07e52e66c00610d95372a5dbd268faf966ff8","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        try:"},{"line_number":6294,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6295,"context_line":"                raise test.TestingException()"},{"line_number":6296,"context_line":"        except exception.BuildAbortException:"},{"line_number":6297,"context_line":"            pass"},{"line_number":6298,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6299,"context_line":""},{"line_number":6300,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_9a3de7c5","line":6297,"range":{"start_line":6296,"start_character":8,"end_line":6297,"end_character":16},"in_reply_to":"1fa4df85_950ce741","updated":"2020-03-04 03:13:12.000000000","message":"In https://review.opendev.org/#/c/631244/65/nova/compute/manager.py@2603,\nwhile we request arq failed, we will raise \u0027Failure getting accelerator requests.\u0027 message, @dansmith, how about use self.assertRaisesRegex() to match the raise message?","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6c5eab97412fde2f0cfc22b67ec312588aab83fc","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        try:"},{"line_number":6294,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6295,"context_line":"                raise test.TestingException()"},{"line_number":6296,"context_line":"        except exception.BuildAbortException:"},{"line_number":6297,"context_line":"            pass"},{"line_number":6298,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6299,"context_line":""},{"line_number":6300,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_d302653d","line":6297,"range":{"start_line":6296,"start_character":8,"end_line":6297,"end_character":16},"in_reply_to":"1fa4df85_9a3de7c5","updated":"2020-03-04 14:24:33.000000000","message":"Sure, that\u0027s fine too.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"f2fd755ca5c39f127feb6d8d924c8fb16e50f78b","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        try:"},{"line_number":6294,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6295,"context_line":"                raise test.TestingException()"},{"line_number":6296,"context_line":"        except exception.BuildAbortException:"},{"line_number":6297,"context_line":"            pass"},{"line_number":6298,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6299,"context_line":""},{"line_number":6300,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_d9f1ec02","line":6297,"range":{"start_line":6296,"start_character":8,"end_line":6297,"end_character":16},"in_reply_to":"1fa4df85_d302653d","updated":"2020-03-08 23:24:47.000000000","message":"Done","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af4897b25cdd986cb245c647aacbbc5068b85538","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        try:"},{"line_number":6294,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6295,"context_line":"                raise test.TestingException()"},{"line_number":6296,"context_line":"        except exception.BuildAbortException:"},{"line_number":6297,"context_line":"            pass"},{"line_number":6298,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6299,"context_line":""},{"line_number":6300,"context_line":"    def test_build_and_run_instance_called_with_proper_args(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"1fa4df85_950ce741","line":6297,"range":{"start_line":6296,"start_character":8,"end_line":6297,"end_character":16},"in_reply_to":"1fa4df85_edf1d96c","updated":"2020-03-03 15:46:02.000000000","message":"No, use assertRaises(). As I said before, if you\u0027re expecting that an exception is raised, re-raised, transformed, or whatever, you should use assertRaises(). Otherwise if your code *stops* raising that exception, you will fail.\n\nIn this case, you clearly want to make sure that if some unexpected exception is raised, it gets transformed into a BuildAbortException. That\u0027s good, so you should make sure that raising TestingException results in a BuildAbortException. Your code here will fail if the exception becomes different, but it will not fail if the context manager *stops* raising an exception. That\u0027s the difference, and that\u0027s the important detail.","commit_id":"697770e1a3bf0e30ff2851cdf8340f81339621ee"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                }"},{"line_number":106,"context_line":"            }"},{"line_number":107,"context_line":"        }"},{"line_number":108,"context_line":"        self.stub_out(\u0027nova.accelerator.cyborg._CyborgClient.\u0027"},{"line_number":109,"context_line":"                      \u0027delete_arqs_for_instance\u0027, lambda *a, **kw: None)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_get_power_state\u0027)"},{"line_number":112,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_sync_instance_power_state\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_f9f51f58","line":109,"range":{"start_line":108,"start_character":0,"end_line":109,"end_character":72},"updated":"2020-03-19 12:28:08.000000000","message":"I also removed this and the tests are still passing.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                }"},{"line_number":106,"context_line":"            }"},{"line_number":107,"context_line":"        }"},{"line_number":108,"context_line":"        self.stub_out(\u0027nova.accelerator.cyborg._CyborgClient.\u0027"},{"line_number":109,"context_line":"                      \u0027delete_arqs_for_instance\u0027, lambda *a, **kw: None)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_get_power_state\u0027)"},{"line_number":112,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_sync_instance_power_state\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_a8af45e1","line":109,"range":{"start_line":108,"start_character":0,"end_line":109,"end_character":72},"in_reply_to":"1fa4df85_f9f51f58","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":6005,"context_line":"                       \u0027_build_networks_for_instance\u0027)"},{"line_number":6006,"context_line":"    @mock.patch.object(virt_driver.ComputeDriver,"},{"line_number":6007,"context_line":"                       \u0027prepare_networks_before_block_device_mapping\u0027)"},{"line_number":6008,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6009,"context_line":"    def _test_accel_build_resources(self, mock_del_arqs,"},{"line_number":6010,"context_line":"            mock_prep_net, mock_build_net, mock_prep_spawn, mock_prep_bd,"},{"line_number":6011,"context_line":"            mock_bdnames, mock_save):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_7c3efdb8","line":6008,"updated":"2020-03-19 12:28:08.000000000","message":"ditto, I removed the mock but the test passes","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":6005,"context_line":"                       \u0027_build_networks_for_instance\u0027)"},{"line_number":6006,"context_line":"    @mock.patch.object(virt_driver.ComputeDriver,"},{"line_number":6007,"context_line":"                       \u0027prepare_networks_before_block_device_mapping\u0027)"},{"line_number":6008,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6009,"context_line":"    def _test_accel_build_resources(self, mock_del_arqs,"},{"line_number":6010,"context_line":"            mock_prep_net, mock_build_net, mock_prep_spawn, mock_prep_bd,"},{"line_number":6011,"context_line":"            mock_bdnames, mock_save):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_a898a5bf","line":6008,"in_reply_to":"1fa4df85_7c3efdb8","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e0a63d10e30aed27367b1e2e40d89e171ec103c","unresolved":false,"context_lines":[{"line_number":6246,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6247,"context_line":"                pass"},{"line_number":6248,"context_line":"        except Exception as e:"},{"line_number":6249,"context_line":"            self.assertIsInstance(e, exception.BuildAbortException)"},{"line_number":6250,"context_line":""},{"line_number":6251,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6252,"context_line":"    def test_delete_arqs_on_bind_failure(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_cd46af22","line":6249,"updated":"2020-03-13 17:36:29.000000000","message":"Again, this is not okay and should be assertRaises.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":6246,"context_line":"            with self.compute._build_resources(*args):"},{"line_number":6247,"context_line":"                pass"},{"line_number":6248,"context_line":"        except Exception as e:"},{"line_number":6249,"context_line":"            self.assertIsInstance(e, exception.BuildAbortException)"},{"line_number":6250,"context_line":""},{"line_number":6251,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6252,"context_line":"    def test_delete_arqs_on_bind_failure(self, mock_del_arqs):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_e8ef7d1a","line":6249,"in_reply_to":"1fa4df85_cd46af22","updated":"2020-03-22 01:28:06.000000000","message":"Again, there are precedents for this [1]. If this patch series is being held to a different set of coding guidelines than other Nova code, there should be at least some documentation of the new guidelines.\n\nAnyway, switched to using _test_delete_arqs_exception() instead of this.\n\n[1] https://review.opendev.org/gitweb?p\u003dopenstack/nova.git;f\u003dnova/tests/unit/compute/test_compute_mgr.py;hb\u003drefs/changes/35/673735/45#l7371","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":6256,"context_line":"        mock_del_arqs.assert_called_once_with(self.context, self.instance)"},{"line_number":6257,"context_line":""},{"line_number":6258,"context_line":"    @mock.patch.object(compute_utils, \u0027delete_arqs_if_needed\u0027)"},{"line_number":6259,"context_line":"    def test_delete_arqs_no_device_profile(self, mock_del_arqs):"},{"line_number":6260,"context_line":"        # If dp_name is None, errors have no effect"},{"line_number":6261,"context_line":"        self.instance.flavor.extra_specs \u003d {}"},{"line_number":6262,"context_line":"        self._test_delete_arqs_if_bind_fails()"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_68e38d35","line":6259,"updated":"2020-03-22 01:28:06.000000000","message":"Replaced this with test_delete_arqs_if_build_res_exception_no_dp() in next PS.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/tests/unit/compute/test_compute_utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"338f4981b40d366fafdb693a52cab603bd927dd6","unresolved":false,"context_lines":[{"line_number":1560,"context_line":"    def test_delete_with_device_profile(self, mock_del_arq):"},{"line_number":1561,"context_line":"        flavor \u003d objects.Flavor(**test_flavor.fake_flavor)"},{"line_number":1562,"context_line":"        flavor[\u0027extra_specs\u0027] \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":1563,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":1564,"context_line":"        instance.flavor \u003d flavor"},{"line_number":1565,"context_line":"        compute_utils.delete_arqs_if_needed(self.context, instance)"},{"line_number":1566,"context_line":"        mock_del_arq.assert_called_once_with(instance.uuid)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    @mock.patch.object(cyborgclient, \u0027delete_arqs_for_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_1cbca904","line":1565,"range":{"start_line":1563,"start_character":2,"end_line":1565,"end_character":1},"updated":"2020-03-19 12:28:08.000000000","message":"this can be written like:\n\n  instance \u003d fake_instance.fake_instance_obj(self.context, flavor\u003dflavor)","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b9ddbbf073014054f4c181f42699f96e14eace8e","unresolved":false,"context_lines":[{"line_number":1560,"context_line":"    def test_delete_with_device_profile(self, mock_del_arq):"},{"line_number":1561,"context_line":"        flavor \u003d objects.Flavor(**test_flavor.fake_flavor)"},{"line_number":1562,"context_line":"        flavor[\u0027extra_specs\u0027] \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":1563,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":1564,"context_line":"        instance.flavor \u003d flavor"},{"line_number":1565,"context_line":"        compute_utils.delete_arqs_if_needed(self.context, instance)"},{"line_number":1566,"context_line":"        mock_del_arq.assert_called_once_with(instance.uuid)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    @mock.patch.object(cyborgclient, \u0027delete_arqs_for_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1fa4df85_68bced05","line":1565,"range":{"start_line":1563,"start_character":2,"end_line":1565,"end_character":1},"in_reply_to":"1fa4df85_1cbca904","updated":"2020-03-22 01:28:06.000000000","message":"Done","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4acb26725b90b66eacd831d8d04f96091b052713","unresolved":false,"context_lines":[{"line_number":1560,"context_line":"    def test_delete_with_device_profile(self, mock_del_arq):"},{"line_number":1561,"context_line":"        flavor \u003d objects.Flavor(**test_flavor.fake_flavor)"},{"line_number":1562,"context_line":"        flavor[\u0027extra_specs\u0027] \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":1563,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":1564,"context_line":"        instance.flavor \u003d flavor"},{"line_number":1565,"context_line":"        compute_utils.delete_arqs_if_needed(self.context, instance)"},{"line_number":1566,"context_line":"        mock_del_arq.assert_called_once_with(instance.uuid)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    @mock.patch.object(cyborgclient, \u0027delete_arqs_for_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"df33271e_dead8b32","line":1565,"range":{"start_line":1563,"start_character":2,"end_line":1565,"end_character":1},"in_reply_to":"1fa4df85_68bced05","updated":"2020-03-23 11:15:39.000000000","message":"Thanks. It was my mistake not marking all the similar code in the below test cases. But still does can also be written like this.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"2dc82d3e519499e77400685cb88ed04be364784e","unresolved":false,"context_lines":[{"line_number":1560,"context_line":"    def test_delete_with_device_profile(self, mock_del_arq):"},{"line_number":1561,"context_line":"        flavor \u003d objects.Flavor(**test_flavor.fake_flavor)"},{"line_number":1562,"context_line":"        flavor[\u0027extra_specs\u0027] \u003d {\u0027accel:device_profile\u0027: \u0027mydp\u0027}"},{"line_number":1563,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":1564,"context_line":"        instance.flavor \u003d flavor"},{"line_number":1565,"context_line":"        compute_utils.delete_arqs_if_needed(self.context, instance)"},{"line_number":1566,"context_line":"        mock_del_arq.assert_called_once_with(instance.uuid)"},{"line_number":1567,"context_line":""},{"line_number":1568,"context_line":"    @mock.patch.object(cyborgclient, \u0027delete_arqs_for_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":45,"id":"df33271e_47bc0705","line":1565,"range":{"start_line":1563,"start_character":2,"end_line":1565,"end_character":1},"in_reply_to":"df33271e_dead8b32","updated":"2020-03-23 17:23:10.000000000","message":"Sorry, my bad. I\u0027ll fix the ones below in the next update.","commit_id":"c0f0a651e777490329a3eccff9867976ea51c28c"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"91fd83a50e5d0e1c2d60c230b437c9808fe87bcc","unresolved":false,"context_lines":[{"line_number":918,"context_line":"        for build_req in build_req_mocks:"},{"line_number":919,"context_line":"            build_req.destroy.assert_called_once_with()"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        mock_del_arqs.assert_has_calls("},{"line_number":922,"context_line":"            [mock.call(self.context, instance) for instance in instances])"},{"line_number":923,"context_line":""},{"line_number":924,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027, new\u003dmock.MagicMock())"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_1924db5e","line":921,"range":{"start_line":921,"start_character":8,"end_line":921,"end_character":39},"updated":"2020-02-22 14:50:51.000000000","message":"https://github.com/python/cpython/blob/master/Lib/unittest/mock.py#L2517\n\nThe __eq__ function will equal these 2 objects are same.\n\nActually, they are not same though values of their attributions are same, so it failed. \nThat\u0027s because you call a RPC cast conductor.build_instances.\n\nMaybe you can changed in this ways:\n1. assert it is called twice.\nself.assertEqual(mock_del_arqs.call_count, 2)\n2. compare the value of the instance. \nget the instance from \nmock_del_arqs.mock_calls or mock_del_arqs.call_args_list.\nand then dict the instance for compare. \nhttps://github.com/python/cpython/blob/master/Lib/unittest/mock.py#L583","commit_id":"27e6d05541b47f51a135102193f09192c05a41c5"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"40143ef432bd37bac9ec6181ab631a08d7035019","unresolved":false,"context_lines":[{"line_number":918,"context_line":"        for build_req in build_req_mocks:"},{"line_number":919,"context_line":"            build_req.destroy.assert_called_once_with()"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        mock_del_arqs.assert_has_calls("},{"line_number":922,"context_line":"            [mock.call(self.context, instance) for instance in instances])"},{"line_number":923,"context_line":""},{"line_number":924,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027, new\u003dmock.MagicMock())"}],"source_content_type":"text/x-python","patch_set":36,"id":"1fa4df85_861bec71","line":921,"range":{"start_line":921,"start_character":8,"end_line":921,"end_character":39},"in_reply_to":"1fa4df85_06085c26","updated":"2020-02-23 02:23:18.000000000","message":"or add a fixture for RPC call to fake it to local call.\nor add a new mock at where this Instance is copied.","commit_id":"27e6d05541b47f51a135102193f09192c05a41c5"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"dd911ce09cedee10f1bb7699e5b90c23b72a3728","unresolved":false,"context_lines":[{"line_number":918,"context_line":"        for build_req in build_req_mocks:"},{"line_number":919,"context_line":"            build_req.destroy.assert_called_once_with()"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        mock_del_arqs.assert_has_calls("},{"line_number":922,"context_line":"            [mock.call(self.context, instance) for instance in instances])"},{"line_number":923,"context_line":""},{"line_number":924,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027, new\u003dmock.MagicMock())"}],"source_content_type":"text/x-python","patch_set":36,"id":"1fa4df85_06085c26","line":921,"range":{"start_line":921,"start_character":8,"end_line":921,"end_character":39},"in_reply_to":"3fa7e38b_1924db5e","updated":"2020-02-23 02:12:34.000000000","message":"or we can add a equal method for objects.Instance","commit_id":"27e6d05541b47f51a135102193f09192c05a41c5"}]}