)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b112127bb784ec6052231cb32cc6544f580b46f3","unresolved":true,"context_lines":[{"line_number":13,"context_line":"- functional tests"},{"line_number":14,"context_line":"- unit tests"},{"line_number":15,"context_line":"- docs: releasenote"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"0793a450_b47e5553","line":16,"updated":"2024-10-08 20:24:25.000000000","message":"I think you forgot `Closes-Bug:` here?","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dad25db41e455a8d949d106c7edfc7bcab8c570d","unresolved":false,"context_lines":[{"line_number":13,"context_line":"- functional tests"},{"line_number":14,"context_line":"- unit tests"},{"line_number":15,"context_line":"- docs: releasenote"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"496f4af8_c8ccf5ec","line":16,"in_reply_to":"0793a450_b47e5553","updated":"2024-10-15 16:50:44.000000000","message":"Done","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"bf2c846a15209e73dd5441a261720e67f2ddbfb4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d55accfb_354626fd","updated":"2024-09-19 19:27:14.000000000","message":"recheck nova-ovs-hybrid-plug  post fail no logs","commit_id":"4c5eeab55fa216d90bc5fe7fb0075b83b5fa74db"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b112127bb784ec6052231cb32cc6544f580b46f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b5de931d_f9fe793f","updated":"2024-10-08 20:24:25.000000000","message":"I might be worth trying the \"in-place\" list edit that was discussed in the previous proposed patch while you\u0027re respinning.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15d871dc08ce502b500f5d77c4a9807dd90e3555","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1a51800b_60e468dc","updated":"2024-10-08 17:53:01.000000000","message":"i think this is mostly fine but it would be better to break up teh unit test into its componet tests.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5f326752532c5f4b7d27e4c63ba6cd438a3203ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"36b53473_3b4e8e3c","updated":"2024-09-30 16:36:47.000000000","message":"i wont have time to fully review today but my previous feed back has been adressed\n\nill try an loop back to review fully during the week.\nping me if i dont","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78dbccba6807643bf53e7d037c25c0fb8996ed0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"788782ee_9fbc9dca","updated":"2024-10-08 17:54:11.000000000","message":"once the test clean up is done i think im fine with mergeing this at this point.\ni dont really have much other feedback so if melainie is also happy with it then we can proceed.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c6a68bf3c491ea322321a04b926575f37e411373","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ab128f49_992283b6","updated":"2024-09-22 02:29:54.000000000","message":"recheck SSH timeout\nbecause of kernel paic\n[    7.579023] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode\u003d0x00001000 ]---","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d1336b65ebdae80e81c5eb66de85f415febf7171","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ff00b457_42333244","updated":"2024-09-23 03:55:29.000000000","message":"recheck unrealted conductor unit test failed in openstack-tox-py311 during DB transaction\n\n```\n...site-packages/oslo_db/sqlalchemy/enginefacade.py\", line 707, in _writer\nTypeError: Can\u0027t upgrade a READER transaction to a WRITER mid-transaction\n```","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a63a497e66cf7bea58e895527c87d7297a3bf0eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8f9e3e01_85148277","updated":"2024-09-23 06:37:30.000000000","message":"recheck unrelated\nPOST_FAIL in nova-live-migration job for test test_live_block_migration","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ea9b4fd1a513eb8966ca41fc659475078d3c5095","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"eeede18a_0fdaadd9","in_reply_to":"b5de931d_f9fe793f","updated":"2024-10-08 20:25:56.000000000","message":"\u003e I might be worth trying the \"in-place\" list edit that was discussed in the previous proposed patch while you\u0027re respinning.\n\n*It might be worth trying ...","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6afe7fb0a422ac904e55ae62d158f300b507e577","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"5f62f717_9e07a1c3","updated":"2024-10-18 01:37:33.000000000","message":"This LGTM. I\u0027ll upgrade to +2 when @smooney@redhat.com reviews the test changes they requested","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"24ed7b86bfedffd8ae394b3df2f40bace161aba4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"149766ba_47298416","updated":"2024-10-16 03:30:56.000000000","message":"recheck\nopenstacksdk-functional-devstack fails with bug \nhttps://bugs.launchpad.net/nova/+bug/2083812","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2e2c91ef60044978fa5dd1df6aac5ba499b05388","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"036f3b61_eb319f13","updated":"2024-11-12 11:57:55.000000000","message":"recheck unrelated fail on job openstacksdk-functional-devstack\n~ while creating key-pair for new server","commit_id":"2bbbfb5f3c33790b8485bb7c20ccf4e106773df7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8c1fd9f120c77b2c942d0866b065111ae2623c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"019ad8cf_a4108dc1","updated":"2024-12-09 17:40:09.000000000","message":"i want to say +2 becasue im actully +1.5 but hte use of mocks in the unit tests makes me a little uncomfortabel\n\ncan you quickly see if you can locally replace them with the correct objects.","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3266d9994a238a926e36c586587537fb98d04ad0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"eb5c8ab2_8656e002","updated":"2024-12-12 03:17:48.000000000","message":"This looks OK to me, thanks for the changes","commit_id":"15dccaeed3cd0cc5a4283ec66d7347becabb64df"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c1c094ee71d1078ee552a9bfb0cd58c47a6209c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"2ff628d5_1cfe1d37","updated":"2024-12-10 13:18:04.000000000","message":"recheck \nnova-ceph-multistore job failed with unrealted issue\n`urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u0027200.225.47.58\u0027, port\u003d443): Read timed out. (read timeout\u003d90)`","commit_id":"15dccaeed3cd0cc5a4283ec66d7347becabb64df"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57342d7a0820f1ecdd540a34b16b7f71dc58908a","unresolved":true,"context_lines":[{"line_number":5102,"context_line":"        if not (instance.old_flavor and instance.new_flavor):"},{"line_number":5103,"context_line":"            return bdms"},{"line_number":5104,"context_line":""},{"line_number":5105,"context_line":"        if confirm:"},{"line_number":5106,"context_line":"            old_swap \u003d instance.old_flavor.swap"},{"line_number":5107,"context_line":"            new_swap \u003d instance.new_flavor.swap"},{"line_number":5108,"context_line":"        else:"},{"line_number":5109,"context_line":"            # revert flavor on _finish_revert_resize"},{"line_number":5110,"context_line":"            old_swap \u003d instance.new_flavor.swap"},{"line_number":5111,"context_line":"            new_swap \u003d instance.old_flavor.swap"},{"line_number":5112,"context_line":""},{"line_number":5113,"context_line":"        if old_swap \u003d\u003d new_swap:"},{"line_number":5114,"context_line":"            # no swap change to update"},{"line_number":5115,"context_line":"            return bdms"}],"source_content_type":"text/x-python","patch_set":3,"id":"be273127_4c6ada50","line":5112,"range":{"start_line":5105,"start_character":7,"end_line":5112,"end_character":1},"updated":"2024-09-20 11:58:00.000000000","message":"old_swap \u003d instance.old_flavor.swap\n        new_swap \u003d instance.new_flavor.swap\n        if not confirm:\n            # revert flavor on _finish_revert_resize\n            old_swap \u003d instance.new_flavor.swap\n            new_swap \u003d instance.old_flavor.swap","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"41d11617f82dd16b22433c352070a8bbd378beca","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"        if not (instance.old_flavor and instance.new_flavor):"},{"line_number":5103,"context_line":"            return bdms"},{"line_number":5104,"context_line":""},{"line_number":5105,"context_line":"        if confirm:"},{"line_number":5106,"context_line":"            old_swap \u003d instance.old_flavor.swap"},{"line_number":5107,"context_line":"            new_swap \u003d instance.new_flavor.swap"},{"line_number":5108,"context_line":"        else:"},{"line_number":5109,"context_line":"            # revert flavor on _finish_revert_resize"},{"line_number":5110,"context_line":"            old_swap \u003d instance.new_flavor.swap"},{"line_number":5111,"context_line":"            new_swap \u003d instance.old_flavor.swap"},{"line_number":5112,"context_line":""},{"line_number":5113,"context_line":"        if old_swap \u003d\u003d new_swap:"},{"line_number":5114,"context_line":"            # no swap change to update"},{"line_number":5115,"context_line":"            return bdms"}],"source_content_type":"text/x-python","patch_set":3,"id":"3426298a_f7609ec4","line":5112,"range":{"start_line":5105,"start_character":7,"end_line":5112,"end_character":1},"in_reply_to":"be273127_4c6ada50","updated":"2024-09-20 17:29:13.000000000","message":"Done","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57342d7a0820f1ecdd540a34b16b7f71dc58908a","unresolved":true,"context_lines":[{"line_number":5112,"context_line":""},{"line_number":5113,"context_line":"        if old_swap \u003d\u003d new_swap:"},{"line_number":5114,"context_line":"            # no swap change to update"},{"line_number":5115,"context_line":"            return bdms"},{"line_number":5116,"context_line":""},{"line_number":5117,"context_line":"        # add swap"},{"line_number":5118,"context_line":"        if old_swap \u003d\u003d 0 and new_swap:"}],"source_content_type":"text/x-python","patch_set":3,"id":"c47d5a42_19e0fabe","line":5115,"updated":"2024-09-20 11:58:00.000000000","message":"put this before the continue check and just use instance.old_flavor.swap ...","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"41d11617f82dd16b22433c352070a8bbd378beca","unresolved":false,"context_lines":[{"line_number":5112,"context_line":""},{"line_number":5113,"context_line":"        if old_swap \u003d\u003d new_swap:"},{"line_number":5114,"context_line":"            # no swap change to update"},{"line_number":5115,"context_line":"            return bdms"},{"line_number":5116,"context_line":""},{"line_number":5117,"context_line":"        # add swap"},{"line_number":5118,"context_line":"        if old_swap \u003d\u003d 0 and new_swap:"}],"source_content_type":"text/x-python","patch_set":3,"id":"60ef18cf_9b94a5ac","line":5115,"in_reply_to":"c47d5a42_19e0fabe","updated":"2024-09-20 17:29:13.000000000","message":"Done","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57342d7a0820f1ecdd540a34b16b7f71dc58908a","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            new_swap_bdm \u003d block_device.create_blank_bdm(new_swap, \u0027swap\u0027)"},{"line_number":5123,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5124,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5125,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":3,"id":"85f234a2_7aff04ab","line":5125,"updated":"2024-09-20 11:58:00.000000000","message":"return instance.get_bdms()","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"41d11617f82dd16b22433c352070a8bbd378beca","unresolved":false,"context_lines":[{"line_number":5122,"context_line":"            new_swap_bdm \u003d block_device.create_blank_bdm(new_swap, \u0027swap\u0027)"},{"line_number":5123,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5124,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5125,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":3,"id":"10f8ee7b_4dab07c5","line":5125,"in_reply_to":"85f234a2_7aff04ab","updated":"2024-09-20 17:29:13.000000000","message":"Done","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57342d7a0820f1ecdd540a34b16b7f71dc58908a","unresolved":true,"context_lines":[{"line_number":5130,"context_line":"                if new_swap \u003e 0:"},{"line_number":5131,"context_line":"                    LOG.info(\u0027Adding swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5132,"context_line":"                    bdm.volume_size \u003d new_swap"},{"line_number":5133,"context_line":"                    bdm.save()"},{"line_number":5134,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5135,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5136,"context_line":"                    bdm.destroy()"}],"source_content_type":"text/x-python","patch_set":3,"id":"8ef2d225_44dcb47e","line":5133,"updated":"2024-09-20 11:58:00.000000000","message":"add breake here","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"41d11617f82dd16b22433c352070a8bbd378beca","unresolved":false,"context_lines":[{"line_number":5130,"context_line":"                if new_swap \u003e 0:"},{"line_number":5131,"context_line":"                    LOG.info(\u0027Adding swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5132,"context_line":"                    bdm.volume_size \u003d new_swap"},{"line_number":5133,"context_line":"                    bdm.save()"},{"line_number":5134,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5135,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5136,"context_line":"                    bdm.destroy()"}],"source_content_type":"text/x-python","patch_set":3,"id":"e240d4de_be0bbd61","line":5133,"in_reply_to":"8ef2d225_44dcb47e","updated":"2024-09-20 17:29:13.000000000","message":"Done","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57342d7a0820f1ecdd540a34b16b7f71dc58908a","unresolved":true,"context_lines":[{"line_number":5133,"context_line":"                    bdm.save()"},{"line_number":5134,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5135,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5136,"context_line":"                    bdm.destroy()"},{"line_number":5137,"context_line":""},{"line_number":5138,"context_line":"        return instance.get_bdms()"},{"line_number":5139,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fcf72cbd_0e18c766","line":5136,"updated":"2024-09-20 11:58:00.000000000","message":"add breake here","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"41d11617f82dd16b22433c352070a8bbd378beca","unresolved":false,"context_lines":[{"line_number":5133,"context_line":"                    bdm.save()"},{"line_number":5134,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5135,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5136,"context_line":"                    bdm.destroy()"},{"line_number":5137,"context_line":""},{"line_number":5138,"context_line":"        return instance.get_bdms()"},{"line_number":5139,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"4e156df9_7cad2141","line":5136,"in_reply_to":"fcf72cbd_0e18c766","updated":"2024-09-20 17:29:13.000000000","message":"Done","commit_id":"1631eedbf13e5bf8e1c27e520e8a459717c26886"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b112127bb784ec6052231cb32cc6544f580b46f3","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1df3e38c_9acd745f","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"updated":"2024-10-08 20:24:25.000000000","message":"IIRC this was what sent the last swap BDM bug fix patch attempt off track. Sylvain I think pointed out that this will do another database query to get the BDMs, when we already know what the BDM list should be at this point.\n\nI think the idea was that instead of this, you could `return bdms + [bdm_obj]` for example to return the original list plus the new BDM without having to do another database query round trip.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cf4649c4854f8328280e3da60e4a2a64dc62b25d","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"a1b2830a_2ee10045","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"00e47f78_664c0e96","updated":"2024-10-09 00:07:06.000000000","message":"ya so in v3 that was my reaction too and i ask amit to make it very clear what branches we were taking because i was concerned that we appared to be hitting the db multipel times as well.\n\nhttps://review.opendev.org/c/openstack/nova/+/929858/3/nova/compute/manager.py\n\nin the current flow we have at least 1 db query and at most 3 \n\nwe always have 1 form  `bdms \u003d instance.get_bdms()` on line 5102\n\nthen if we modify the the bdms we have exactly 2 more the create/update/delete of the swap bdm follow by the get fo all the instnace bdms.\n\nwe can optimise out that final \"instance.get_bdms()\"\nby mutating the bdms list in place and returning that as you have noted below.\nprovided the modification in the db succeded i don\u0027t see why we cant optimise here.\n\ni missed the conversation about why the original approach was abandoned and came into the review understanding that the new approach intended to simplify the approach. The performance optimisation is not complex so I have no issues with including it.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"11974505937abc12f988331b6143924f75d8d57b","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"59564a99_e2be6d10","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"1b5ba5df_b0bd30a4","updated":"2024-10-09 08:20:49.000000000","message":"i think no only because when we are resizing we should not be able to attch/detach a volume or hard reboot so the bdm cleanup code should not run.\n\nso i dont think we will have interleaved request due to the vm task state.\nthe api would hopefully return a 409 or 400\n\nwith that said im not sure there are bugs that might alloww the bdms to be modifed.\nnot that i know of but its a good question.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"21de2d1b6651910d247781fedd1e19725e2c5c35","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ab54fbe3_371c9426","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"1df3e38c_9acd745f","updated":"2024-10-08 23:16:27.000000000","message":"I asked similar question in the past about modifying the bdms in place and then saving the updated list\nbut i tought this second attemept we wanted to ensure we were geting the fixed state form the db instead of taht to simplfy the code.\n\nnova unlike Neutron does not have plugins that modify hwo the objects are stored.\nneutron often does optimisation like this where they return the object they submited to the db instead of reading it back.\n\nthat can result in what is returned to the user being\ndiffernt then what in the the db becasue if the server does not have a plugin loaded the filed related to that plugin wont be saved. (nutron pluging can add db tables that only exist when the plugin is used...)\n\nthat is a bug in there api design and we do not have plugpoints in nova that allow this behvieor to diverge so its much safer for us to do that.\nbut i tought the instance.get_bdms was intentional here to avoid mutating the bdms in memory?\n\nif that was nto why we took a diffent path then yes i agree we shoudl optimise it.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"8c1e5a709a6f79a4a595102986ef97521f254a79","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"dfb962e6_92a040fb","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"59564a99_e2be6d10","updated":"2024-10-14 11:19:50.000000000","message":"as of now,\n1 - we are doing `at-least 1` DB query in this function, which we were doing in caller earlier. i.e `get-fresh` bdms - when no update is performed.\n\n2 - we will do `at-most 3` DB queries, in case of any BDM update.\n`    a) get-fresh + add + return-fresh`\n`    b) get-fresh + update + return-fresh`\n\n\n--------\nI think we should do the extra call of `return-fresh bdms`, it simplifies this little bit.\n\n\nplease let me know if it\u0027s must, and I\u0027ll update in next respin as Mel suggested on update state as follows.\nafter adding new bdm:\n---\u003e bdms.append(bdm)\n\nafter deleting new bdm:\n---\u003e bdms.objects.remove(bdm)\n\nand return same bdms object","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e4ea39b31243801c962ad1f6ac6fa60fb593d69d","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1b5ba5df_b0bd30a4","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"a1b2830a_2ee10045","updated":"2024-10-09 00:23:11.000000000","message":"You are right. It would be 3 round trips vs 2 round trips.\n\nIf a database update fails I think it will raise an exception, in my experience.\n\nI wonder if we should be concerned about BDM records being updated underneath us and if so, would the optimization be bad?","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b4fc97a6b0d3b7f968a9a91539fa2a5ac380cac9","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"00e47f78_664c0e96","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"ab54fbe3_371c9426","updated":"2024-10-08 23:33:53.000000000","message":"TBH I don\u0027t remember and/or it could have been discussed and I missed it.\n\nIf there is a reason not to update the list and return it, that is OK, I just didn\u0027t know about it.\n\nThis just feels like an area where someone in the future will do a patch like \"reduce database calls from 5 to 1\" or something like that 🙂","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1512eaa1cda992640e9b3d4413a6806f18a4ff9","unresolved":false,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"2779b612_5fe94911","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"d33f8e4e_121386db","updated":"2024-10-23 06:43:20.000000000","message":"Done","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dad25db41e455a8d949d106c7edfc7bcab8c570d","unresolved":true,"context_lines":[{"line_number":5122,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5123,"context_line":"                context, instance_uuid\u003dinstance.uuid, **new_swap_bdm)"},{"line_number":5124,"context_line":"            bdm_obj.update_or_create()"},{"line_number":5125,"context_line":"            return instance.get_bdms()"},{"line_number":5126,"context_line":""},{"line_number":5127,"context_line":"        # update swap"},{"line_number":5128,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"d33f8e4e_121386db","line":5125,"range":{"start_line":5125,"start_character":19,"end_line":5125,"end_character":38},"in_reply_to":"dfb962e6_92a040fb","updated":"2024-10-15 16:50:44.000000000","message":"\u003e after adding new bdm:\n\u003e ---\u003e bdms.append(bdm)\n\u003e \nwe do not need to add, it we are updating the same bdm","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b112127bb784ec6052231cb32cc6544f580b46f3","unresolved":true,"context_lines":[{"line_number":5134,"context_line":"                    break"},{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    break"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"        return instance.get_bdms()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7bbc23cb_9c63a067","line":5137,"updated":"2024-10-08 20:24:25.000000000","message":"Similar here, you could add `bdms.remove(bdm)` I think,","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dad25db41e455a8d949d106c7edfc7bcab8c570d","unresolved":false,"context_lines":[{"line_number":5134,"context_line":"                    break"},{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    break"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"        return instance.get_bdms()"}],"source_content_type":"text/x-python","patch_set":5,"id":"5935a7ef_819165a2","line":5137,"in_reply_to":"7bbc23cb_9c63a067","updated":"2024-10-15 16:50:44.000000000","message":"Acknowledged","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b112127bb784ec6052231cb32cc6544f580b46f3","unresolved":true,"context_lines":[{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    break"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"        return instance.get_bdms()"},{"line_number":5141,"context_line":""},{"line_number":5142,"context_line":"    @wrap_exception()"},{"line_number":5143,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":5,"id":"5bc28a45_16b600eb","line":5140,"range":{"start_line":5140,"start_character":16,"end_line":5140,"end_character":34},"updated":"2024-10-08 20:24:25.000000000","message":"And then `return bdms` here. Which would avoid another database query round trip.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dad25db41e455a8d949d106c7edfc7bcab8c570d","unresolved":false,"context_lines":[{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    break"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"        return instance.get_bdms()"},{"line_number":5141,"context_line":""},{"line_number":5142,"context_line":"    @wrap_exception()"},{"line_number":5143,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":5,"id":"026312d5_9fb39b5c","line":5140,"range":{"start_line":5140,"start_character":16,"end_line":5140,"end_character":34},"in_reply_to":"5bc28a45_16b600eb","updated":"2024-10-15 16:50:44.000000000","message":"Acknowledged","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f8054c21d8a72dd94755fc14bfdf9954b5ef315e","unresolved":true,"context_lines":[{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    bdms.objects.remove(bdm)"},{"line_number":5139,"context_line":"                    break"},{"line_number":5140,"context_line":"        return bdms"},{"line_number":5141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3bf8d11f_31259a54","line":5138,"updated":"2024-10-17 01:40:43.000000000","message":"Is there a reason you didn\u0027t just do `bdms.remove(bdm)` here? With oslo.versionedobjects that are lists, the `objects` attribute is mostly intended to be used internally by `BlockDeviceMappingList` (I say mostly because I have seen times when it was necessary). And in general it\u0027s better to call the outermost public method rather than accessing an attribute within it, if possible.","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"75ac3005ae9e2a2e77825c56ebbf747e1a0fe289","unresolved":true,"context_lines":[{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    bdms.objects.remove(bdm)"},{"line_number":5139,"context_line":"                    break"},{"line_number":5140,"context_line":"        return bdms"},{"line_number":5141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"b8d1cfa0_c316a7b1","line":5138,"in_reply_to":"3bf8d11f_31259a54","updated":"2024-10-17 03:54:52.000000000","message":"I tested it, there was not remove method with bdms, so have to do bdms.objects.remove.\n\nIn the last patch too, I had done the same.","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1512eaa1cda992640e9b3d4413a6806f18a4ff9","unresolved":false,"context_lines":[{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    bdms.objects.remove(bdm)"},{"line_number":5139,"context_line":"                    break"},{"line_number":5140,"context_line":"        return bdms"},{"line_number":5141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"aebe42aa_ef2ef4bd","line":5138,"in_reply_to":"aca64a90_b8e1c89f","updated":"2024-10-23 06:43:20.000000000","message":"Done","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b49797f26bcbfd91aa1063b1dd3880ba3ba1ede2","unresolved":true,"context_lines":[{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    bdms.objects.remove(bdm)"},{"line_number":5139,"context_line":"                    break"},{"line_number":5140,"context_line":"        return bdms"},{"line_number":5141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"e7cf999a_8887acad","line":5138,"in_reply_to":"b8d1cfa0_c316a7b1","updated":"2024-10-17 16:00:06.000000000","message":"Hm, it worked when I test a toy example locally and then just now I run your functional tests and they pass using `bdms.remove(bdm)`. Are you saying there you saw an error or how did you test?","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6afe7fb0a422ac904e55ae62d158f300b507e577","unresolved":true,"context_lines":[{"line_number":5135,"context_line":"                elif new_swap \u003d\u003d 0:"},{"line_number":5136,"context_line":"                    LOG.info(\u0027Deleting swap BDM.\u0027, instance\u003dinstance)"},{"line_number":5137,"context_line":"                    bdm.destroy()"},{"line_number":5138,"context_line":"                    bdms.objects.remove(bdm)"},{"line_number":5139,"context_line":"                    break"},{"line_number":5140,"context_line":"        return bdms"},{"line_number":5141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"aca64a90_b8e1c89f","line":5138,"in_reply_to":"e7cf999a_8887acad","updated":"2024-10-18 01:37:33.000000000","message":"Gah, I\u0027m wrong, I had been using a regular Python list when I tested, not a `BlockDeviceMappingList` object and didn\u0027t even realize 😣 What Amit did here is the right thing.","commit_id":"6743df82fde16f7b49d810ad7d5b72032073e625"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8c1fd9f120c77b2c942d0866b065111ae2623c6","unresolved":true,"context_lines":[{"line_number":5097,"context_line":""},{"line_number":5098,"context_line":"    def _update_bdm_for_swap_to_finish_resize("},{"line_number":5099,"context_line":"            self, context, instance, confirm\u003dTrue):"},{"line_number":5100,"context_line":"        \"\"\"This updates bdm.swap with new swap info"},{"line_number":5101,"context_line":"        \"\"\""},{"line_number":5102,"context_line":""},{"line_number":5103,"context_line":"        bdms \u003d instance.get_bdms()"},{"line_number":5104,"context_line":"        if not (instance.old_flavor and instance.new_flavor):"}],"source_content_type":"text/x-python","patch_set":11,"id":"b29bbfd9_ac0a3f9b","line":5101,"range":{"start_line":5100,"start_character":0,"end_line":5101,"end_character":11},"updated":"2024-12-09 17:40:09.000000000","message":"nit:\n\n```suggestion\n        \"\"\"This updates bdm.swap with new swap info\"\"\"\n```","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0cd65d3432cf3a0796a38f17f0b07049d1a845f5","unresolved":false,"context_lines":[{"line_number":5097,"context_line":""},{"line_number":5098,"context_line":"    def _update_bdm_for_swap_to_finish_resize("},{"line_number":5099,"context_line":"            self, context, instance, confirm\u003dTrue):"},{"line_number":5100,"context_line":"        \"\"\"This updates bdm.swap with new swap info"},{"line_number":5101,"context_line":"        \"\"\""},{"line_number":5102,"context_line":""},{"line_number":5103,"context_line":"        bdms \u003d instance.get_bdms()"},{"line_number":5104,"context_line":"        if not (instance.old_flavor and instance.new_flavor):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5dd42834_13ed1169","line":5101,"range":{"start_line":5100,"start_character":0,"end_line":5101,"end_character":11},"in_reply_to":"b29bbfd9_ac0a3f9b","updated":"2024-12-10 09:40:27.000000000","message":"Done","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15d871dc08ce502b500f5d77c4a9807dd90e3555","unresolved":true,"context_lines":[{"line_number":3349,"context_line":""},{"line_number":3350,"context_line":"    def test_swap_expand_2048_to_2048_confirm(self):"},{"line_number":3351,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3352,"context_line":""},{"line_number":3353,"context_line":"    def _server_created_with_host(self):"},{"line_number":3354,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3355,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":5,"id":"7730d541_05e68aaa","line":3352,"updated":"2024-10-08 17:53:01.000000000","message":"nit: the ordering of these makes it a little hard to see that all the required permutions are thre.\n\nthey are but it would have been better to group them by size.\n\ni.e instead of \n```\n    def test_swap_expand_0_to_1024_confirm(self):\n        self._test_swap_resize(0, 1024)\n    def test_swap_expand_1024_to_2048_confirm(self):\n        self._test_swap_resize(1024, 2048)\n    def test_swap_expand_0_to_1024_revert(self):\n        self._test_swap_resize(0, 1024, confirm\u003dFalse)\n```\n\n```\n    def test_swap_expand_0_to_1024_confirm(self):\n        self._test_swap_resize(0, 1024)\n    def test_swap_expand_0_to_1024_revert(self):\n        self._test_swap_resize(0, 1024, confirm\u003dFalse)\n    def test_swap_expand_1024_to_2048_confirm(self):\n        self._test_swap_resize(1024, 2048)\n```\n\nbasically sort them aphabetically.\n\nyou could have also done this with ddt but its fine as is.\n\nit just makes it simiper to reivew if they are in order","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"8c1e5a709a6f79a4a595102986ef97521f254a79","unresolved":true,"context_lines":[{"line_number":3349,"context_line":""},{"line_number":3350,"context_line":"    def test_swap_expand_2048_to_2048_confirm(self):"},{"line_number":3351,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3352,"context_line":""},{"line_number":3353,"context_line":"    def _server_created_with_host(self):"},{"line_number":3354,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3355,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":5,"id":"9f43fd04_2082a25a","line":3352,"in_reply_to":"7730d541_05e68aaa","updated":"2024-10-14 11:19:50.000000000","message":"updated order.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1512eaa1cda992640e9b3d4413a6806f18a4ff9","unresolved":false,"context_lines":[{"line_number":3349,"context_line":""},{"line_number":3350,"context_line":"    def test_swap_expand_2048_to_2048_confirm(self):"},{"line_number":3351,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3352,"context_line":""},{"line_number":3353,"context_line":"    def _server_created_with_host(self):"},{"line_number":3354,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3355,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":5,"id":"9d0150b0_df1ac9b9","line":3352,"in_reply_to":"9f43fd04_2082a25a","updated":"2024-10-23 06:43:20.000000000","message":"Done","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8c1fd9f120c77b2c942d0866b065111ae2623c6","unresolved":true,"context_lines":[{"line_number":3323,"context_line":"    def test_swap_expand_0_to_0_confirm(self):"},{"line_number":3324,"context_line":"        self._test_swap_resize(0, 0)"},{"line_number":3325,"context_line":""},{"line_number":3326,"context_line":"    def test_swap_expand_0_to_1024_revert(self):"},{"line_number":3327,"context_line":"        self._test_swap_resize(0, 1024, confirm\u003dFalse)"},{"line_number":3328,"context_line":""},{"line_number":3329,"context_line":"    def test_swap_expand_0_to_1024_confirm(self):"},{"line_number":3330,"context_line":"        self._test_swap_resize(0, 1024)"},{"line_number":3331,"context_line":""},{"line_number":3332,"context_line":"    def test_swap_expand_1024_to_2048_confirm(self):"},{"line_number":3333,"context_line":"        self._test_swap_resize(1024, 2048)"},{"line_number":3334,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a4fd09f_b516cd3c","line":3331,"range":{"start_line":3326,"start_character":0,"end_line":3331,"end_character":1},"updated":"2024-12-09 17:40:09.000000000","message":"nit: it would be nice to be consitent but no need to respin for this\n```suggestion\n    def test_swap_expand_0_to_1024_confirm(self):\n        self._test_swap_resize(0, 1024)\n\n    def test_swap_expand_0_to_1024_revert(self):\n        self._test_swap_resize(0, 1024, confirm\u003dFalse)\n\n```\n\nall the reset are confirm then revert","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0cd65d3432cf3a0796a38f17f0b07049d1a845f5","unresolved":false,"context_lines":[{"line_number":3323,"context_line":"    def test_swap_expand_0_to_0_confirm(self):"},{"line_number":3324,"context_line":"        self._test_swap_resize(0, 0)"},{"line_number":3325,"context_line":""},{"line_number":3326,"context_line":"    def test_swap_expand_0_to_1024_revert(self):"},{"line_number":3327,"context_line":"        self._test_swap_resize(0, 1024, confirm\u003dFalse)"},{"line_number":3328,"context_line":""},{"line_number":3329,"context_line":"    def test_swap_expand_0_to_1024_confirm(self):"},{"line_number":3330,"context_line":"        self._test_swap_resize(0, 1024)"},{"line_number":3331,"context_line":""},{"line_number":3332,"context_line":"    def test_swap_expand_1024_to_2048_confirm(self):"},{"line_number":3333,"context_line":"        self._test_swap_resize(1024, 2048)"},{"line_number":3334,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"aecfee9b_9aea8fd2","line":3331,"range":{"start_line":3326,"start_character":0,"end_line":3331,"end_character":1},"in_reply_to":"9a4fd09f_b516cd3c","updated":"2024-12-10 09:40:27.000000000","message":"Done","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15d871dc08ce502b500f5d77c4a9807dd90e3555","unresolved":true,"context_lines":[{"line_number":13222,"context_line":"        self.assertEqual(bdms[0].guest_format, \u0027swap\u0027)"},{"line_number":13223,"context_line":"        self.assertEqual(bdms[0].volume_size, new_swap)"},{"line_number":13224,"context_line":""},{"line_number":13225,"context_line":"        # expand swap (10 --\u003e 15)"},{"line_number":13226,"context_line":"        new_swap \u003d 15"},{"line_number":13227,"context_line":""},{"line_number":13228,"context_line":"        instance.old_flavor.swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":5,"id":"419cc0c0_32bf6354","line":13225,"updated":"2024-10-08 17:53:01.000000000","message":"each section with a comend below shoudl be a separate test.\n\nplease split this up","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dad25db41e455a8d949d106c7edfc7bcab8c570d","unresolved":true,"context_lines":[{"line_number":13222,"context_line":"        self.assertEqual(bdms[0].guest_format, \u0027swap\u0027)"},{"line_number":13223,"context_line":"        self.assertEqual(bdms[0].volume_size, new_swap)"},{"line_number":13224,"context_line":""},{"line_number":13225,"context_line":"        # expand swap (10 --\u003e 15)"},{"line_number":13226,"context_line":"        new_swap \u003d 15"},{"line_number":13227,"context_line":""},{"line_number":13228,"context_line":"        instance.old_flavor.swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":5,"id":"dd13fc35_ac8869db","line":13225,"in_reply_to":"10e364d8_f93fd16a","updated":"2024-10-15 16:50:44.000000000","message":"this becomes functional tests, which is already tested in test_servers\nhence removed unit tests from here.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"8c1e5a709a6f79a4a595102986ef97521f254a79","unresolved":true,"context_lines":[{"line_number":13222,"context_line":"        self.assertEqual(bdms[0].guest_format, \u0027swap\u0027)"},{"line_number":13223,"context_line":"        self.assertEqual(bdms[0].volume_size, new_swap)"},{"line_number":13224,"context_line":""},{"line_number":13225,"context_line":"        # expand swap (10 --\u003e 15)"},{"line_number":13226,"context_line":"        new_swap \u003d 15"},{"line_number":13227,"context_line":""},{"line_number":13228,"context_line":"        instance.old_flavor.swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":5,"id":"10e364d8_f93fd16a","line":13225,"in_reply_to":"419cc0c0_32bf6354","updated":"2024-10-14 11:19:50.000000000","message":"this was done, to confirm add-expand-shrink as a single test/story\nalso I could not figure out the way to save BDM in DB and retrieve it here in unit test.\n\nbut writing all separately makes these operation independent, hence better testing.\nI\u0027ll fix this.","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2e2c91ef60044978fa5dd1df6aac5ba499b05388","unresolved":false,"context_lines":[{"line_number":13222,"context_line":"        self.assertEqual(bdms[0].guest_format, \u0027swap\u0027)"},{"line_number":13223,"context_line":"        self.assertEqual(bdms[0].volume_size, new_swap)"},{"line_number":13224,"context_line":""},{"line_number":13225,"context_line":"        # expand swap (10 --\u003e 15)"},{"line_number":13226,"context_line":"        new_swap \u003d 15"},{"line_number":13227,"context_line":""},{"line_number":13228,"context_line":"        instance.old_flavor.swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":5,"id":"cee2ebec_1d39ce38","line":13225,"in_reply_to":"dd13fc35_ac8869db","updated":"2024-11-12 11:57:55.000000000","message":"Done","commit_id":"8982adb9bb1bba76120c9300cee9933324024c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6c1218aeef2301dacc42d5c3f4531b6e1bb7b467","unresolved":true,"context_lines":[{"line_number":13467,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13468,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13469,"context_line":"            volume_size\u003d1024)"},{"line_number":13470,"context_line":"        self.instance.get_bdms.return_value \u003d [existing_swap_bdm]"},{"line_number":13471,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13472,"context_line":"            self.context, self.instance)"},{"line_number":13473,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"0f0f3dd8_73b7ee85","line":13470,"range":{"start_line":13470,"start_character":46,"end_line":13470,"end_character":65},"updated":"2024-11-08 12:07:15.000000000","message":"this is what mel is refering too it shoudl be a BlockDeviceMappingList not a list()\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/block_device.py#L359","commit_id":"35d46f7f0ff2c82d656de2796836241cb709461d"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2e2c91ef60044978fa5dd1df6aac5ba499b05388","unresolved":false,"context_lines":[{"line_number":13467,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13468,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13469,"context_line":"            volume_size\u003d1024)"},{"line_number":13470,"context_line":"        self.instance.get_bdms.return_value \u003d [existing_swap_bdm]"},{"line_number":13471,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13472,"context_line":"            self.context, self.instance)"},{"line_number":13473,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7e8a8848_c4c7cefd","line":13470,"range":{"start_line":13470,"start_character":46,"end_line":13470,"end_character":65},"in_reply_to":"0f0f3dd8_73b7ee85","updated":"2024-11-12 11:57:55.000000000","message":"Acknowledged","commit_id":"35d46f7f0ff2c82d656de2796836241cb709461d"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e1512eaa1cda992640e9b3d4413a6806f18a4ff9","unresolved":true,"context_lines":[{"line_number":13470,"context_line":"        self.instance.get_bdms.return_value \u003d [existing_swap_bdm]"},{"line_number":13471,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13472,"context_line":"            self.context, self.instance)"},{"line_number":13473,"context_line":""},{"line_number":13474,"context_line":"        mock_bdm_obj.assert_not_called()"},{"line_number":13475,"context_line":"        existing_swap_bdm.destroy.assert_called_once()"},{"line_number":13476,"context_line":"        self.instance.get_bdms.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":9,"id":"356dcd68_716271d7","line":13473,"updated":"2024-10-23 06:43:20.000000000","message":"so right now this test is failing at `bdms.objects.remove(bdm)`\nthis is must to mock, I tried mocking it but it did not worked.\n\nthis is of type `CoercedList`\n\n----\ntype(bdms.objects)\n`\u003cclass \u0027oslo_versionedobjects.fields.CoercedList\u0027\u003e`\n\n----\n\n```\ndir(bdms.objects)\n[\u0027__add__\u0027, \u0027__class__\u0027, \u0027__class_getitem__\u0027, \u0027__contains__\u0027, \u0027__delattr__\u0027, \u0027__delitem__\u0027, \u0027__dict__\u0027, \u0027__dir__\u0027, \u0027__doc__\u0027, \u0027__eq__\u0027, \u0027__format__\u0027, \u0027__ge__\u0027, \u0027__getattribute__\u0027, \u0027__getitem__\u0027, \u0027__gt__\u0027, \u0027__hash__\u0027, \u0027__iadd__\u0027, \u0027__imul__\u0027, \u0027__init__\u0027, \u0027__init_subclass__\u0027, \u0027__iter__\u0027, \u0027__le__\u0027, \u0027__len__\u0027, \u0027__lt__\u0027, \u0027__module__\u0027, \u0027__mul__\u0027, \u0027__ne__\u0027, \u0027__new__\u0027, \u0027__reduce__\u0027, \u0027__reduce_ex__\u0027, \u0027__repr__\u0027, \u0027__reversed__\u0027, \u0027__rmul__\u0027, \u0027__setattr__\u0027, \u0027__setitem__\u0027, \u0027__setslice__\u0027, \u0027__sizeof__\u0027, \u0027__slotnames__\u0027, \u0027__str__\u0027, \u0027__subclasshook__\u0027, \u0027__weakref__\u0027, \u0027_coerce_item\u0027, \u0027_element_type\u0027, \u0027_field\u0027, \u0027_obj\u0027, \n\n\u0027append\u0027, \u0027clear\u0027, \u0027copy\u0027, \u0027count\u0027, \u0027enable_coercing\u0027, \u0027extend\u0027, \u0027index\u0027, \u0027insert\u0027, \u0027pop\u0027, \u0027remove\u0027, \u0027reverse\u0027, \u0027sort\u0027]\n```\n----\n```\ndir(bdms)\n[\u0027OBJ_PROJECT_NAMESPACE\u0027, \u0027OBJ_SERIAL_NAMESPACE\u0027, \u0027VERSION\u0027, \u0027__abstractmethods__\u0027, \u0027__add__\u0027, \u0027__class__\u0027, \u0027__class_getitem__\u0027, \u0027__contains__\u0027, \u0027__deepcopy__\u0027, \u0027__delattr__\u0027, \u0027__dict__\u0027, \u0027__dir__\u0027, \u0027__doc__\u0027, \u0027__eq__\u0027, \u0027__format__\u0027, \u0027__ge__\u0027, \u0027__getattribute__\u0027, \u0027__getitem__\u0027, \u0027__gt__\u0027, \u0027__hash__\u0027, \u0027__init__\u0027, \u0027__init_subclass__\u0027, \u0027__iter__\u0027, \u0027__le__\u0027, \u0027__len__\u0027, \u0027__lt__\u0027, \u0027__module__\u0027, \u0027__ne__\u0027, \u0027__new__\u0027, \u0027__radd__\u0027, \u0027__reduce__\u0027, \u0027__reduce_ex__\u0027, \u0027__repr__\u0027, \u0027__reversed__\u0027, \u0027__setattr__\u0027, \u0027__sizeof__\u0027, \u0027__slots__\u0027, \u0027__str__\u0027, \u0027__subclasshook__\u0027, \u0027__weakref__\u0027, \u0027_abc_impl\u0027, \u0027_changed_fields\u0027, \u0027_context\u0027, \n\n\u0027_db_block_device_mapping_get_all_by_instance\u0027, \u0027_db_block_device_mapping_get_all_by_instance_uuids\u0027, \u0027_db_block_device_mapping_get_all_by_volume\u0027, \u0027_lazy_loads\u0027, \u0027_obj_from_primitive\u0027, \u0027_obj_make_obj_compatible\u0027, \u0027_obj_objects\u0027, \u0027_obj_primitive_field\u0027, \u0027_obj_primitive_key\u0027, \u0027_obj_relationship_for\u0027, \n\n\u0027bdms_by_instance_uuid\u0027, \u0027child_versions\u0027, \u0027count\u0027, \u0027fields\u0027, \u0027get_by_instance_uuid\u0027, \u0027get_by_instance_uuids\u0027, \u0027get_by_volume\u0027, \u0027index\u0027, \u0027indirection_api\u0027, \u0027instance_uuids\u0027, \u0027obj_alternate_context\u0027,\n\n \u0027obj_attr_is_set\u0027, \u0027obj_class_from_name\u0027, \u0027obj_clone\u0027, \u0027obj_context\u0027, \u0027obj_extra_fields\u0027, \u0027obj_fields\u0027, \u0027obj_from_primitive\u0027, \u0027obj_get_changes\u0027, \u0027obj_load_attr\u0027, \u0027obj_make_compatible\u0027, \u0027obj_make_compatible_from_manifest\u0027, \u0027obj_name\u0027, \u0027obj_relationships\u0027, \u0027obj_reset_changes\u0027, \u0027obj_set_defaults\u0027, \u0027obj_to_primitive\u0027, \u0027obj_what_changed\u0027, \n\n \n \u0027objects\u0027, \u0027root_bdm\u0027, \u0027save\u0027, \u0027should_migrate_data\u0027, \u0027sort\u0027, \u0027to_json_schema\u0027]\n```","commit_id":"35d46f7f0ff2c82d656de2796836241cb709461d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"35afd82750ded7d2e68019979d747a51362a7aa2","unresolved":true,"context_lines":[{"line_number":13470,"context_line":"        self.instance.get_bdms.return_value \u003d [existing_swap_bdm]"},{"line_number":13471,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13472,"context_line":"            self.context, self.instance)"},{"line_number":13473,"context_line":""},{"line_number":13474,"context_line":"        mock_bdm_obj.assert_not_called()"},{"line_number":13475,"context_line":"        existing_swap_bdm.destroy.assert_called_once()"},{"line_number":13476,"context_line":"        self.instance.get_bdms.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":9,"id":"868ba163_8f5e216d","line":13473,"in_reply_to":"356dcd68_716271d7","updated":"2024-10-26 00:53:08.000000000","message":"I think you may have made the same mistake as I did when I asked you about bdms.remove vs bdms.objects.remove on an earlier PS 🙂 \n\nOn L13470 you are returning a built-in list, not a `BlockDeviceMappingList`. And the built-in list will not have the `objects` attribute.\n\nHonestly, I think the way you\u0027re mocking all these is asking for trouble because of the differences between mocks and real objects. I assume you\u0027re doing it because you want to verify how many times `get_bdms` is called.\n\nI think what would probably work better for you is if you used real `objects.BlockDeviceMapping` and `objects.BlockDeviceMappingList` and then selectively mock the methods you want to check. Similar to what you\u0027re doing with `self.instance`, you are mocking only `get_bdms`. You can do the same thing with `objects.BlockDeviceMapping.save` and `objects.BlockDeviceMapping.destroy`.","commit_id":"35d46f7f0ff2c82d656de2796836241cb709461d"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2e2c91ef60044978fa5dd1df6aac5ba499b05388","unresolved":false,"context_lines":[{"line_number":13470,"context_line":"        self.instance.get_bdms.return_value \u003d [existing_swap_bdm]"},{"line_number":13471,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13472,"context_line":"            self.context, self.instance)"},{"line_number":13473,"context_line":""},{"line_number":13474,"context_line":"        mock_bdm_obj.assert_not_called()"},{"line_number":13475,"context_line":"        existing_swap_bdm.destroy.assert_called_once()"},{"line_number":13476,"context_line":"        self.instance.get_bdms.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":9,"id":"65be8f5c_e9b74b1a","line":13473,"in_reply_to":"868ba163_8f5e216d","updated":"2024-11-12 11:57:55.000000000","message":"Acknowledged","commit_id":"35d46f7f0ff2c82d656de2796836241cb709461d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8c1fd9f120c77b2c942d0866b065111ae2623c6","unresolved":true,"context_lines":[{"line_number":13374,"context_line":"        self.compute \u003d manager.ComputeManager()"},{"line_number":13375,"context_line":"        self.context \u003d context.RequestContext(fakes.FAKE_USER_ID,"},{"line_number":13376,"context_line":"                                              fakes.FAKE_PROJECT_ID)"},{"line_number":13377,"context_line":"        self.instance \u003d mock.Mock()"},{"line_number":13378,"context_line":""},{"line_number":13379,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13380,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13381,"context_line":"    def test_no_flavor_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13382,"context_line":"        self.instance.get_bdms.return_value \u003d []"},{"line_number":13383,"context_line":"        self.instance.old_flavor \u003d None"},{"line_number":13384,"context_line":"        self.instance.new_flavor \u003d None"},{"line_number":13385,"context_line":""},{"line_number":13386,"context_line":"        bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13387,"context_line":"            self.context, self.instance)"},{"line_number":13388,"context_line":""},{"line_number":13389,"context_line":"        self.assertEqual(bdms, [])"},{"line_number":13390,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13391,"context_line":""},{"line_number":13392,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13393,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13394,"context_line":"    def test_no_swap_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13395,"context_line":"        self.instance.old_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13396,"context_line":"        self.instance.new_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13397,"context_line":""},{"line_number":13398,"context_line":"        existing_swap_bdm \u003d mock.Mock("},{"line_number":13399,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13400,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13401,"context_line":"            volume_size\u003d1024)"},{"line_number":13402,"context_line":""},{"line_number":13403,"context_line":"        bdms \u003d [existing_swap_bdm]"},{"line_number":13404,"context_line":""},{"line_number":13405,"context_line":"        self.instance.get_bdms.return_value \u003d bdms"},{"line_number":13406,"context_line":""},{"line_number":13407,"context_line":"        new_bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13408,"context_line":"            self.context, self.instance)"},{"line_number":13409,"context_line":""},{"line_number":13410,"context_line":"        self.assertEqual(new_bdms, bdms)"},{"line_number":13411,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13412,"context_line":""},{"line_number":13413,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13414,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ea8379d0_398e1a05","line":13411,"range":{"start_line":13377,"start_character":3,"end_line":13411,"end_character":51},"updated":"2024-12-09 17:40:09.000000000","message":"this is overall fine but my meta comment is if you dont need to use a mock or a fake you shoudl prefer using a reals object\n\nwe shoudl avoid using incorrect types like\n\n  bdms \u003d [existing_swap_bdm]\n\nand instead use an actual bdm list object here","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c1c094ee71d1078ee552a9bfb0cd58c47a6209c7","unresolved":true,"context_lines":[{"line_number":13374,"context_line":"        self.compute \u003d manager.ComputeManager()"},{"line_number":13375,"context_line":"        self.context \u003d context.RequestContext(fakes.FAKE_USER_ID,"},{"line_number":13376,"context_line":"                                              fakes.FAKE_PROJECT_ID)"},{"line_number":13377,"context_line":"        self.instance \u003d mock.Mock()"},{"line_number":13378,"context_line":""},{"line_number":13379,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13380,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13381,"context_line":"    def test_no_flavor_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13382,"context_line":"        self.instance.get_bdms.return_value \u003d []"},{"line_number":13383,"context_line":"        self.instance.old_flavor \u003d None"},{"line_number":13384,"context_line":"        self.instance.new_flavor \u003d None"},{"line_number":13385,"context_line":""},{"line_number":13386,"context_line":"        bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13387,"context_line":"            self.context, self.instance)"},{"line_number":13388,"context_line":""},{"line_number":13389,"context_line":"        self.assertEqual(bdms, [])"},{"line_number":13390,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13391,"context_line":""},{"line_number":13392,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13393,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13394,"context_line":"    def test_no_swap_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13395,"context_line":"        self.instance.old_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13396,"context_line":"        self.instance.new_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13397,"context_line":""},{"line_number":13398,"context_line":"        existing_swap_bdm \u003d mock.Mock("},{"line_number":13399,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13400,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13401,"context_line":"            volume_size\u003d1024)"},{"line_number":13402,"context_line":""},{"line_number":13403,"context_line":"        bdms \u003d [existing_swap_bdm]"},{"line_number":13404,"context_line":""},{"line_number":13405,"context_line":"        self.instance.get_bdms.return_value \u003d bdms"},{"line_number":13406,"context_line":""},{"line_number":13407,"context_line":"        new_bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13408,"context_line":"            self.context, self.instance)"},{"line_number":13409,"context_line":""},{"line_number":13410,"context_line":"        self.assertEqual(new_bdms, bdms)"},{"line_number":13411,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13412,"context_line":""},{"line_number":13413,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13414,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"a8a37672_005f36bb","line":13411,"range":{"start_line":13377,"start_character":3,"end_line":13411,"end_character":51},"in_reply_to":"c1b155eb_44da451b","updated":"2024-12-10 13:18:04.000000000","message":"Updated bdm object, but I did not updated the mocked self.instance with real instance because it does not help much with unit test.\n\nhaving mocked instance simplify test also less code.","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0cd65d3432cf3a0796a38f17f0b07049d1a845f5","unresolved":true,"context_lines":[{"line_number":13374,"context_line":"        self.compute \u003d manager.ComputeManager()"},{"line_number":13375,"context_line":"        self.context \u003d context.RequestContext(fakes.FAKE_USER_ID,"},{"line_number":13376,"context_line":"                                              fakes.FAKE_PROJECT_ID)"},{"line_number":13377,"context_line":"        self.instance \u003d mock.Mock()"},{"line_number":13378,"context_line":""},{"line_number":13379,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13380,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13381,"context_line":"    def test_no_flavor_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13382,"context_line":"        self.instance.get_bdms.return_value \u003d []"},{"line_number":13383,"context_line":"        self.instance.old_flavor \u003d None"},{"line_number":13384,"context_line":"        self.instance.new_flavor \u003d None"},{"line_number":13385,"context_line":""},{"line_number":13386,"context_line":"        bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13387,"context_line":"            self.context, self.instance)"},{"line_number":13388,"context_line":""},{"line_number":13389,"context_line":"        self.assertEqual(bdms, [])"},{"line_number":13390,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13391,"context_line":""},{"line_number":13392,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13393,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"},{"line_number":13394,"context_line":"    def test_no_swap_change(self, mock_bdm_obj, mock_create_bdm):"},{"line_number":13395,"context_line":"        self.instance.old_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13396,"context_line":"        self.instance.new_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13397,"context_line":""},{"line_number":13398,"context_line":"        existing_swap_bdm \u003d mock.Mock("},{"line_number":13399,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13400,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13401,"context_line":"            volume_size\u003d1024)"},{"line_number":13402,"context_line":""},{"line_number":13403,"context_line":"        bdms \u003d [existing_swap_bdm]"},{"line_number":13404,"context_line":""},{"line_number":13405,"context_line":"        self.instance.get_bdms.return_value \u003d bdms"},{"line_number":13406,"context_line":""},{"line_number":13407,"context_line":"        new_bdms \u003d self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13408,"context_line":"            self.context, self.instance)"},{"line_number":13409,"context_line":""},{"line_number":13410,"context_line":"        self.assertEqual(new_bdms, bdms)"},{"line_number":13411,"context_line":"        self.instance.get_bdms.assert_called_once()"},{"line_number":13412,"context_line":""},{"line_number":13413,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13414,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"c1b155eb_44da451b","line":13411,"range":{"start_line":13377,"start_character":3,"end_line":13411,"end_character":51},"in_reply_to":"ea8379d0_398e1a05","updated":"2024-12-10 09:40:27.000000000","message":"Acknowledged","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8c1fd9f120c77b2c942d0866b065111ae2623c6","unresolved":true,"context_lines":[{"line_number":13466,"context_line":"        self.instance.old_flavor \u003d mock.Mock(swap\u003d1024)"},{"line_number":13467,"context_line":"        self.instance.new_flavor \u003d mock.Mock(swap\u003d0)"},{"line_number":13468,"context_line":""},{"line_number":13469,"context_line":"        existing_swap_bdm \u003d objects.BlockDeviceMapping("},{"line_number":13470,"context_line":"            guest_format\u003d\u0027swap\u0027,"},{"line_number":13471,"context_line":"            device_type\u003d\u0027disk\u0027,"},{"line_number":13472,"context_line":"            volume_size\u003d1024)"},{"line_number":13473,"context_line":""},{"line_number":13474,"context_line":"        self.instance.get_bdms.return_value \u003d objects.BlockDeviceMappingList("},{"line_number":13475,"context_line":"            objects\u003d[existing_swap_bdm])"},{"line_number":13476,"context_line":""},{"line_number":13477,"context_line":"        self.compute._update_bdm_for_swap_to_finish_resize("},{"line_number":13478,"context_line":"            self.context, self.instance)"},{"line_number":13479,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"e442a91f_debcd3b0","line":13476,"range":{"start_line":13469,"start_character":3,"end_line":13476,"end_character":1},"updated":"2024-12-09 17:40:09.000000000","message":"here you are usingthe real object so it woudl have been perferable if you did that in the other unit tests.","commit_id":"c1b1df6afab05d607e1bc1a621ba2b38524e7ecb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3266d9994a238a926e36c586587537fb98d04ad0","unresolved":true,"context_lines":[{"line_number":13374,"context_line":"        self.compute \u003d manager.ComputeManager()"},{"line_number":13375,"context_line":"        self.context \u003d context.RequestContext(fakes.FAKE_USER_ID,"},{"line_number":13376,"context_line":"                                              fakes.FAKE_PROJECT_ID)"},{"line_number":13377,"context_line":"        self.instance \u003d mock.Mock()"},{"line_number":13378,"context_line":""},{"line_number":13379,"context_line":"    @mock.patch(\u0027nova.block_device.create_blank_bdm\u0027)"},{"line_number":13380,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMapping\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"b615ceea_f726983f","line":13377,"updated":"2024-12-12 03:17:48.000000000","message":"I don\u0027t like this either but after testing a few things, I found that the oslo.versionedobjects don\u0027t seem to validate anything unless there is database access. So, you can set arbitrary attributes on them etc (like any regular python object) and there\u0027s no complaint. It wouldn\u0027t catch any issues like setting a non-existent field or making a typo for a field.\n\nAFAICT the object in this case wouldn\u0027t be doing anything different than a mock, so I can\u0027t really find a good argument for it.","commit_id":"15dccaeed3cd0cc5a4283ec66d7347becabb64df"}]}