)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cfa8a9325eb0b3aa46db245a44527feac6ea597e","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add --reimage-boot-volume parameter to the rebuild command to"},{"line_number":10,"context_line":"allow rebuilding of volume backed instances."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I4a6e30b2cf12f32202a2d9ef1ced347e1dd139f3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"48889df6_25097ce1","line":11,"updated":"2022-08-31 19:32:23.000000000","message":"this should depend on https://review.opendev.org/c/openstack/python-novaclient/+/827163","commit_id":"b52bf62382b0a218bfd2f1bb328a6de2f4afdb2a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ec9b6d034526b05307e1ecd5502adaa7ab8f4746","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add --reimage-boot-volume parameter to the rebuild command to"},{"line_number":10,"context_line":"allow rebuilding of volume backed instances."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I4a6e30b2cf12f32202a2d9ef1ced347e1dd139f3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fde88ec_e040263b","line":11,"in_reply_to":"48889df6_25097ce1","updated":"2022-08-31 20:12:26.000000000","message":"Done","commit_id":"b52bf62382b0a218bfd2f1bb328a6de2f4afdb2a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3e546bf614a02374c698fced0307069fac0e6995","unresolved":true,"context_lines":[{"line_number":10,"context_line":"allow rebuilding of volume backed instances."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Depends-On: https://review.opendev.org/c/openstack/python-novaclient/+/827163"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I4a6e30b2cf12f32202a2d9ef1ced347e1dd139f3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"a7be5ddb_2032c0bf","line":13,"updated":"2022-09-01 10:13:24.000000000","message":"+1","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1e27a353c757065316aadc86c6776d9c4f8ce7ed","unresolved":false,"context_lines":[{"line_number":10,"context_line":"allow rebuilding of volume backed instances."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Depends-On: https://review.opendev.org/c/openstack/python-novaclient/+/827163"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I4a6e30b2cf12f32202a2d9ef1ced347e1dd139f3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"75747621_b62a073d","line":13,"in_reply_to":"a7be5ddb_2032c0bf","updated":"2022-09-08 16:29:15.000000000","message":"Ack","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cfa8a9325eb0b3aa46db245a44527feac6ea597e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"08a84f46_5c6127b2","updated":"2022-08-31 19:32:23.000000000","message":"some minor copy paste issue but overall this looks ok\n","commit_id":"b52bf62382b0a218bfd2f1bb328a6de2f4afdb2a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ec9b6d034526b05307e1ecd5502adaa7ab8f4746","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"44c1e02a_02e058fa","updated":"2022-08-31 20:12:26.000000000","message":"Thanks Sean","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3e546bf614a02374c698fced0307069fac0e6995","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0a08dc4a_32e25568","updated":"2022-09-01 10:13:24.000000000","message":"yep this looks good to me now thanks","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1e27a353c757065316aadc86c6776d9c4f8ce7ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c82238c6_8e08b2b1","updated":"2022-09-08 16:29:15.000000000","message":"Thanks Stephen for the review.","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9284e33390d9f40a722bdf4bfdb904fe9d8b5a14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"39dcf057_d1f974f6","updated":"2022-09-09 10:03:22.000000000","message":"Much better, thanks 🙏 Still need to address the option though. The tests are also a bit wonky.\n\nWhat are the chances of getting a functional test here? I\u0027d be okay with it being a follow-up.","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7074f531673745c0109ec281de9407fc6dbc4c59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a7892cb7_352bc0a0","updated":"2022-09-08 18:59:36.000000000","message":"recheck\n\nunrelated network tests failure\n\nopenstackclient.tests.functional.network.v2.test_network_ndp_proxy.L3NDPProxyTests\t","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bcac2217fc2c6391243b30e2bbbe17759207d050","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"34473436_be01a196","updated":"2022-09-13 16:04:33.000000000","message":"Thanks Stephen","commit_id":"71496a71db2f01a8f61210107392399f236cf22b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"32e98bd8d7580127358583e3b73ca20961ab835c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"277076b9_addc1834","updated":"2022-09-14 10:39:51.000000000","message":"Thanks Stephen","commit_id":"4024bdb3933dd79eec4bcf99c13f3dbf17add40b"}],"openstackclient/compute/v2/server.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"824310907c47a3b1de3ecf1ee99b6fa354ed3d47","unresolved":true,"context_lines":[{"line_number":3233,"context_line":"                )"},{"line_number":3234,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3235,"context_line":""},{"line_number":3236,"context_line":"            kwargs[\u0027reimage_boot_volume\u0027] \u003d parsed_args.reimage_boot_volume"},{"line_number":3237,"context_line":""},{"line_number":3238,"context_line":"        try:"},{"line_number":3239,"context_line":"            server \u003d server.rebuild(image, parsed_args.password, **kwargs)"}],"source_content_type":"text/x-python","patch_set":4,"id":"823f7c5d_22dcd642","line":3236,"updated":"2022-09-06 10:48:33.000000000","message":"so this is not required.\n\nwe actully want an else\n\n\n        if parsed_args.reimage_boot_volume:\n            if compute_client.api_version \u003c api_versions.APIVersion(\u00272.93\u0027):\n                msg \u003d _(\n                    \u0027--os-compute-api-version 2.93 or greater is required to \u0027\n                    \u0027support the --reimage-boot-volume option\u0027\n                )\n                raise exceptions.CommandError(msg)\n        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027)\n           ... check if instance is bfv\n                msg \u003d _(\n                    \u0027--os-compute-api-version 2.93 or greater and \u0027\n                    \u0027--reimage-boot-volume option are required to rebuild a bfv guest\u0027\n                )\n                raise exceptions.CommandError(msg)","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8b19c5a6fbd693535e2d410cc6d5c0aa248f8ab3","unresolved":false,"context_lines":[{"line_number":3233,"context_line":"                )"},{"line_number":3234,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3235,"context_line":""},{"line_number":3236,"context_line":"            kwargs[\u0027reimage_boot_volume\u0027] \u003d parsed_args.reimage_boot_volume"},{"line_number":3237,"context_line":""},{"line_number":3238,"context_line":"        try:"},{"line_number":3239,"context_line":"            server \u003d server.rebuild(image, parsed_args.password, **kwargs)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1c3c8517_f4e85457","line":3236,"in_reply_to":"823f7c5d_22dcd642","updated":"2022-09-06 15:51:18.000000000","message":"Done","commit_id":"09d7fdc8d25e3600af27443b8b1371a7ea9e3574"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fc21543c5e1c6c5a8f4f94525eb0eb2651a9ebcc","unresolved":true,"context_lines":[{"line_number":3102,"context_line":"            help\u003d_(\u0027Reimage a volume backed instance. Note that this will \u0027"},{"line_number":3103,"context_line":"                   \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3104,"context_line":"                   \u0027provided image.\u0027),"},{"line_number":3105,"context_line":"        )"},{"line_number":3106,"context_line":"        return parser"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"    def take_action(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":5,"id":"2f62e957_ff77aa99","line":3105,"updated":"2022-09-08 15:08:54.000000000","message":"Do we actually need this? Is there a practical reason that users would *not* want to do this? If we do want to do this, could we use something a little more descriptive? How about\n\n  --reimage-volume\n  --no-reimage-volume (default)\n\nAlso, please place this above the \u0027--hostname\u0027 option and add the following string to the help text:\n\n  \u0027(supported by --os-compute-api-version 2.93 or above)\u0027\n\nIf you could wrap the help text like we\u0027ve done elsewhere that would be super too 👍\n\n  help\u003d_(\n      \u0027Reimage a volume-backed instance. ... \u0027\n      ...\n  ),\n\nAll assuming we actually need this, going back to my first point.","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8d14541729394d9bf1a560b3d0ae19c6bb9decf9","unresolved":true,"context_lines":[{"line_number":3102,"context_line":"            help\u003d_(\u0027Reimage a volume backed instance. Note that this will \u0027"},{"line_number":3103,"context_line":"                   \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3104,"context_line":"                   \u0027provided image.\u0027),"},{"line_number":3105,"context_line":"        )"},{"line_number":3106,"context_line":"        return parser"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"    def take_action(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":5,"id":"633ef32b_4a62cbfe","line":3105,"in_reply_to":"2f62e957_ff77aa99","updated":"2022-09-08 15:51:27.000000000","message":"This was decided as part of the spec[1]. This check was originally on the nova API side but it was decided that the microversion should be enough.\nSince it\u0027s a destructive feature (wipes out cinder volume data), and it didn\u0027t exist before it was decided to keep a check at least on the client side.\nThe original name was reimage-boot-volume and the nova team agreed to --confirm-reimage so can\u0027t change it at this point.\nWill work on the other comments.\n\n[1] https://specs.openstack.org/openstack/nova-specs/specs/zed/approved/volume-backed-server-rebuild.html#other-end-user-impact","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98093434f6349a57256a5a44747577c9410deb92","unresolved":true,"context_lines":[{"line_number":3102,"context_line":"            help\u003d_(\u0027Reimage a volume backed instance. Note that this will \u0027"},{"line_number":3103,"context_line":"                   \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3104,"context_line":"                   \u0027provided image.\u0027),"},{"line_number":3105,"context_line":"        )"},{"line_number":3106,"context_line":"        return parser"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"    def take_action(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":5,"id":"841d6d5b_f7b35e6b","line":3105,"in_reply_to":"633ef32b_4a62cbfe","updated":"2022-09-08 16:29:06.000000000","message":"\u003e This was decided as part of the spec[1]. This check was originally on the nova API side but it was decided that the microversion should be enough.\n\u003e Since it\u0027s a destructive feature (wipes out cinder volume data), and it didn\u0027t exist before it was decided to keep a check at least on the client side.\n\u003e The original name was reimage-boot-volume and the nova team agreed to --confirm-reimage so can\u0027t change it at this point.\n\nnova specs don\u0027t overrule OSC consistency guidelines 😉 Also, there\u0027s no reason we can\u0027t go back and update the spec. In any case, boolean options should always come in pairs [1]. More importantly, \u0027--confirm-reimage\u0027 isn\u0027t a great choice because it\u0027s not descriptive. What are you reimaging? I\u0027m not wedded to \u0027--reimage-volume\u0027 and we could make it longer if we wanted, with something like \u0027--allow-reimage-volume\u0027 / \u0027--disallow-reimage-volume\u0027 (though I think that\u0027s unnecessary, personally). In any case, let\u0027s come up with something more helpful.\n\n[1] https://docs.openstack.org/python-openstackclient/latest/contributor/command-options.html#boolean-options\n\n\u003e Will work on the other comments.\n\u003e \n\u003e [1] https://specs.openstack.org/openstack/nova-specs/specs/zed/approved/volume-backed-server-rebuild.html#other-end-user-impact","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1e27a353c757065316aadc86c6776d9c4f8ce7ed","unresolved":false,"context_lines":[{"line_number":3102,"context_line":"            help\u003d_(\u0027Reimage a volume backed instance. Note that this will \u0027"},{"line_number":3103,"context_line":"                   \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3104,"context_line":"                   \u0027provided image.\u0027),"},{"line_number":3105,"context_line":"        )"},{"line_number":3106,"context_line":"        return parser"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"    def take_action(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":5,"id":"75148bb0_3af76bf4","line":3105,"in_reply_to":"633ef32b_4a62cbfe","updated":"2022-09-08 16:29:15.000000000","message":"Done","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fc21543c5e1c6c5a8f4f94525eb0eb2651a9ebcc","unresolved":true,"context_lines":[{"line_number":3130,"context_line":"            if not image_dict:"},{"line_number":3131,"context_line":"                image_dict \u003d {}"},{"line_number":3132,"context_line":"            image_id \u003d image_dict.get(\u0027id\u0027)"},{"line_number":3133,"context_line":"            image \u003d image_client.get_image(image_id)"},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"        kwargs \u003d {}"},{"line_number":3136,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"415fd62e_7e2a126e","line":3133,"updated":"2022-09-08 15:08:54.000000000","message":"Should we actually be making this call if \u0027id\u0027 is None? Surely this will crash and burn?\n\n  \u003e\u003e\u003e image_dict \u003d {}\n  \u003e\u003e\u003e image_id \u003d image_dict.get(\u0027id\u0027)\n  \u003e\u003e\u003e image_id is None\n  True","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8d14541729394d9bf1a560b3d0ae19c6bb9decf9","unresolved":true,"context_lines":[{"line_number":3130,"context_line":"            if not image_dict:"},{"line_number":3131,"context_line":"                image_dict \u003d {}"},{"line_number":3132,"context_line":"            image_id \u003d image_dict.get(\u0027id\u0027)"},{"line_number":3133,"context_line":"            image \u003d image_client.get_image(image_id)"},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"        kwargs \u003d {}"},{"line_number":3136,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"8d498393_9036a948","line":3133,"in_reply_to":"415fd62e_7e2a126e","updated":"2022-09-08 15:51:27.000000000","message":"Yeah true, will update this.","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1e27a353c757065316aadc86c6776d9c4f8ce7ed","unresolved":false,"context_lines":[{"line_number":3130,"context_line":"            if not image_dict:"},{"line_number":3131,"context_line":"                image_dict \u003d {}"},{"line_number":3132,"context_line":"            image_id \u003d image_dict.get(\u0027id\u0027)"},{"line_number":3133,"context_line":"            image \u003d image_client.get_image(image_id)"},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"        kwargs \u003d {}"},{"line_number":3136,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5efd2ffb_d80d941e","line":3133,"in_reply_to":"8d498393_9036a948","updated":"2022-09-08 16:29:15.000000000","message":"Done\nI\u0027ve made some changes here i.e. for volume backed instance rebuild, we should supply an image since it\u0027s not trivial to use an existing one.","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fc21543c5e1c6c5a8f4f94525eb0eb2651a9ebcc","unresolved":true,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"71404796_dbf55e03","line":3241,"updated":"2022-09-08 15:08:54.000000000","message":"This check isn\u0027t necessary and confuses matters. What about if a user does not pass the \u0027--confirm-resize\u0027 flag and is using e.g. API microversion 2.88? Both blocks will be skipped and we\u0027ll attempt the resize. I\u0027m thinking you should just replace this with an \u0027else\u0027?","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c89f330957ecfde369ac879315bf8e01daf7613","unresolved":true,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bb99fdb1_13522a82","line":3241,"in_reply_to":"3f91673e_88e3f30e","updated":"2022-09-08 16:43:18.000000000","message":"Example:\n\n  if parsed_args.reimage_volume:\n      ...\n  else:\n      # force user to explicitly request reimaging of volume-backed server\n      if server.image is None:\n          msg \u003d _(\n              \u0027--os-compute-api-version 2.93 or greater and \u0027\n              \u0027--reimage-volume are required to rebuild a volume-backed \u0027\n              \u0027server.\u0027\n          )\n          raise exceptions.CommandError(msg)","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98093434f6349a57256a5a44747577c9410deb92","unresolved":true,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f91673e_88e3f30e","line":3241,"in_reply_to":"65c2259d_4a67318f","updated":"2022-09-08 16:29:06.000000000","message":"\u003e We don\u0027t allow reimaging on the API side if MV \u003c 2.93[1] so we just check here for the --confirm-reimage flag.\n\u003e 1) If --confirm-reimage is passed then microversion should be \u003e\u003d 2.93\n\u003e 2) If --confirm-reimage is not passed and microversion is greater than or equal to 2.93, then the instance should be image backed (and not volume backed)\n\nAh, so you\u0027re relying on a server-side check. Do we have to? Could we not do it here to avoid making that invalid request in the first place. It would actually simplify this, I think?\n\n  if user specified argument\n    then check if the api version is new enough\n      if yes, proceed\n      if not, fail\n  otherwise\n    if the user is attempting to resize a volume-backed image, fail \n\n\u003e An \"else\" clause would also be good here but it is generic and has a lot of cases among which I can\u0027t think of a bad one right now but might cause issues later. Eg: if --confirm-reimage option is not passed, everything will go into else.\n\u003e This \"elif\" clause is specific and here we\u0027re just dealing with two cases,\n\u003e \n\u003e 1) image backed instance\n\u003e 2) volume backed instance\n\u003e \n\u003e which to me is simple and straightforward. what do you think?\n\nAll that will change if you change to an \u0027else\u0027 here is that we will also check if \u0027image\u0027 is None for older API versions and prevent the operation if so. I don\u0027t see why we shouldn\u0027t do that client side, even if the server would eventually catch the issues for us.\n\n\u003e \n\u003e [1] https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/compute/servers.py#L1208-L1209","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1e27a353c757065316aadc86c6776d9c4f8ce7ed","unresolved":true,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bdab7c52_ac284f71","line":3241,"in_reply_to":"65c2259d_4a67318f","updated":"2022-09-08 16:29:15.000000000","message":"Just remembered the case, we allowed volume backed server rebuild having same old and new image (before mv 2.93) so the else would cause failure in that case which used to work before.","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8d14541729394d9bf1a560b3d0ae19c6bb9decf9","unresolved":true,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"65c2259d_4a67318f","line":3241,"in_reply_to":"71404796_dbf55e03","updated":"2022-09-08 15:51:27.000000000","message":"We don\u0027t allow reimaging on the API side if MV \u003c 2.93[1] so we just check here for the --confirm-reimage flag.\n1) If --confirm-reimage is passed then microversion should be \u003e\u003d 2.93\n2) If --confirm-reimage is not passed and microversion is greater than or equal to 2.93, then the instance should be image backed (and not volume backed)\n\nAn \"else\" clause would also be good here but it is generic and has a lot of cases among which I can\u0027t think of a bad one right now but might cause issues later. Eg: if --confirm-reimage option is not passed, everything will go into else.\nThis \"elif\" clause is specific and here we\u0027re just dealing with two cases,\n\n1) image backed instance\n2) volume backed instance\n\nwhich to me is simple and straightforward. what do you think?\n\n[1] https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/compute/servers.py#L1208-L1209","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b10d13e0761550ac0079937e2fa3dcedd0eb8114","unresolved":false,"context_lines":[{"line_number":3238,"context_line":"                    \u0027support the --confirm-reimage option.\u0027"},{"line_number":3239,"context_line":"                )"},{"line_number":3240,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3241,"context_line":"        elif compute_client.api_version \u003e\u003d api_versions.APIVersion(\u00272.93\u0027):"},{"line_number":3242,"context_line":"            # If the value of image attribute is None, it is BFV server."},{"line_number":3243,"context_line":"            check_image \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3244,"context_line":"            if not check_image:"}],"source_content_type":"text/x-python","patch_set":5,"id":"b39866ed_0240c1c5","line":3241,"in_reply_to":"bb99fdb1_13522a82","updated":"2022-09-08 18:36:16.000000000","message":"Done\nUpdated based on our discussion on IRC.","commit_id":"e0408748cf7738815c6e260bea2ce541baa2843e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c89f330957ecfde369ac879315bf8e01daf7613","unresolved":true,"context_lines":[{"line_number":3098,"context_line":"                \u0027Rebuild a volume backed instance. Note that this will \u0027"},{"line_number":3099,"context_line":"                \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3100,"context_line":"                \u0027provided image. By default, this will be False and \u0027"},{"line_number":3101,"context_line":"                \u0027would not rebuild a volume backed instance.\u0027"},{"line_number":3102,"context_line":"                \u0027(supported by --os-compute-api-version 2.93 or above)\u0027"},{"line_number":3103,"context_line":"            ),"},{"line_number":3104,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":6,"id":"f5fcb6b2_cc3dec36","line":3101,"range":{"start_line":3101,"start_character":60,"end_line":3101,"end_character":61},"updated":"2022-09-08 16:43:18.000000000","message":"missing trailing space","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b10d13e0761550ac0079937e2fa3dcedd0eb8114","unresolved":false,"context_lines":[{"line_number":3098,"context_line":"                \u0027Rebuild a volume backed instance. Note that this will \u0027"},{"line_number":3099,"context_line":"                \u0027wipe the root volume data and overwrite it with the \u0027"},{"line_number":3100,"context_line":"                \u0027provided image. By default, this will be False and \u0027"},{"line_number":3101,"context_line":"                \u0027would not rebuild a volume backed instance.\u0027"},{"line_number":3102,"context_line":"                \u0027(supported by --os-compute-api-version 2.93 or above)\u0027"},{"line_number":3103,"context_line":"            ),"},{"line_number":3104,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":6,"id":"f2be9cb8_1f9c82cc","line":3101,"range":{"start_line":3101,"start_character":60,"end_line":3101,"end_character":61},"in_reply_to":"f5fcb6b2_cc3dec36","updated":"2022-09-08 18:36:16.000000000","message":"Done","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c89f330957ecfde369ac879315bf8e01daf7613","unresolved":true,"context_lines":[{"line_number":3132,"context_line":"        else:"},{"line_number":3133,"context_line":"            # When the \u0027image\u0027 attribute in server dict is None"},{"line_number":3134,"context_line":"            # (volume backed server) then we don\u0027t default to empty"},{"line_number":3135,"context_line":"            # dict but get the None value instead"},{"line_number":3136,"context_line":"            image_dict \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3137,"context_line":"            if image_dict:"},{"line_number":3138,"context_line":"                image_id \u003d image_dict.get(\u0027id\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"8737d245_5a6f272f","line":3135,"updated":"2022-09-08 16:43:18.000000000","message":"nit:\n\n  if parsed_args.image:\n      ...\n  else:\n      if server.image is None:\n          msg \u003d _(\n              \u0027The --image option is required when rebuilding a \u0027\n              \u0027volume-backed server\u0027\n          )\n      image \u003d image_client.get_image(server.image[\u0027id\u0027])\n\nThere\u0027s no need to use \u0027to_dict()\u0027 here. Also, we can simply check if \u0027image\u0027 is None rather than unpacking to a dict like this.","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b10d13e0761550ac0079937e2fa3dcedd0eb8114","unresolved":false,"context_lines":[{"line_number":3132,"context_line":"        else:"},{"line_number":3133,"context_line":"            # When the \u0027image\u0027 attribute in server dict is None"},{"line_number":3134,"context_line":"            # (volume backed server) then we don\u0027t default to empty"},{"line_number":3135,"context_line":"            # dict but get the None value instead"},{"line_number":3136,"context_line":"            image_dict \u003d server.to_dict().get(\u0027image\u0027, {})"},{"line_number":3137,"context_line":"            if image_dict:"},{"line_number":3138,"context_line":"                image_id \u003d image_dict.get(\u0027id\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b767fc7d_abf4af07","line":3135,"in_reply_to":"8737d245_5a6f272f","updated":"2022-09-08 18:36:16.000000000","message":"Done","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c89f330957ecfde369ac879315bf8e01daf7613","unresolved":true,"context_lines":[{"line_number":3257,"context_line":"                msg \u003d _("},{"line_number":3258,"context_line":"                    \u0027--os-compute-api-version 2.93 or greater and \u0027"},{"line_number":3259,"context_line":"                    \u0027--confirm-reimage option are required to rebuild a \u0027"},{"line_number":3260,"context_line":"                    \u0027BFV guest.\u0027"},{"line_number":3261,"context_line":"                )"},{"line_number":3262,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3263,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ff27a0dd_a6775763","line":3260,"range":{"start_line":3260,"start_character":21,"end_line":3260,"end_character":31},"updated":"2022-09-08 16:43:18.000000000","message":"nit:\n\n  volume-backed server.\n\n(BFV is internal lingo that we should try to avoid exposing)","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b10d13e0761550ac0079937e2fa3dcedd0eb8114","unresolved":false,"context_lines":[{"line_number":3257,"context_line":"                msg \u003d _("},{"line_number":3258,"context_line":"                    \u0027--os-compute-api-version 2.93 or greater and \u0027"},{"line_number":3259,"context_line":"                    \u0027--confirm-reimage option are required to rebuild a \u0027"},{"line_number":3260,"context_line":"                    \u0027BFV guest.\u0027"},{"line_number":3261,"context_line":"                )"},{"line_number":3262,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3263,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1758ab26_1e36f072","line":3260,"range":{"start_line":3260,"start_character":21,"end_line":3260,"end_character":31},"in_reply_to":"ff27a0dd_a6775763","updated":"2022-09-08 18:36:16.000000000","message":"Done","commit_id":"0933ed5be197d315f93028e3e1efdf679bae9e5c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9284e33390d9f40a722bdf4bfdb904fe9d8b5a14","unresolved":true,"context_lines":[{"line_number":3092,"context_line":"            ),"},{"line_number":3093,"context_line":"        )"},{"line_number":3094,"context_line":"        parser.add_argument("},{"line_number":3095,"context_line":"            \u0027--confirm-reimage\u0027,"},{"line_number":3096,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":3097,"context_line":"            help\u003d_("},{"line_number":3098,"context_line":"                \u0027Rebuild a volume backed instance. Note that this will \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"4439ee3e_931b825f","line":3095,"updated":"2022-09-09 10:03:22.000000000","message":"This still needs to be updated and needs an equivalent negative option which will be the default:\n\n  --reimage-volume\n  --no-reimage-volume\n\nWhen adding boolean options, you can use a common dest and then store_true/store_false actions to avoid having to check. For example:\n\n  parsed.add_argument(\n      \u0027--reimage-volume\u0027,\n      actions\u003d\u0027store_true\u0027,\n      dest\u003d\u0027reimage_volume\u0027,\n      default\u003dNone,\n      help\u003d_(...),\n  )\n  parsed.add_argument(\n      \u0027--no-reimage-volume\u0027,\n      actions\u003d\u0027store_false\u0027,\n      dest\u003d\u0027reimage_volume\u0027,\n      default\u003dNone,\n      help\u003d_(...),\n  )","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bcac2217fc2c6391243b30e2bbbe17759207d050","unresolved":false,"context_lines":[{"line_number":3092,"context_line":"            ),"},{"line_number":3093,"context_line":"        )"},{"line_number":3094,"context_line":"        parser.add_argument("},{"line_number":3095,"context_line":"            \u0027--confirm-reimage\u0027,"},{"line_number":3096,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":3097,"context_line":"            help\u003d_("},{"line_number":3098,"context_line":"                \u0027Rebuild a volume backed instance. Note that this will \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"371d0ff9_a8740f6b","line":3095,"in_reply_to":"4439ee3e_931b825f","updated":"2022-09-13 16:04:33.000000000","message":"Sean Mooney proposed the name of this config parameter[1] and I\u0027ve tried to reach him few times on IRC but he hasn\u0027t responded so I will update the patch with the recommendation.\n\n[1] https://review.opendev.org/c/openstack/nova-specs/+/840155/4..5/specs/zed/approved/volume-backed-server-rebuild.rst#b142","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ac71f20b1cd9fc590579d070596b6994121b18af","unresolved":true,"context_lines":[{"line_number":3153,"context_line":"                    \u0027instance rebuild with --image parameter.\u0027"},{"line_number":3154,"context_line":"                )"},{"line_number":3155,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3156,"context_line":"            image \u003d image_client.get_image(server.image[\u0027id\u0027])"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"        kwargs \u003d {}"},{"line_number":3159,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9ffc4a54_59ef4eaa","line":3156,"updated":"2022-09-14 10:25:28.000000000","message":"I split this into a separate change so that we can backport it.","commit_id":"71496a71db2f01a8f61210107392399f236cf22b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"32e98bd8d7580127358583e3b73ca20961ab835c","unresolved":false,"context_lines":[{"line_number":3153,"context_line":"                    \u0027instance rebuild with --image parameter.\u0027"},{"line_number":3154,"context_line":"                )"},{"line_number":3155,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":3156,"context_line":"            image \u003d image_client.get_image(server.image[\u0027id\u0027])"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"        kwargs \u003d {}"},{"line_number":3159,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9d2c4a35_014097c1","line":3156,"in_reply_to":"9ffc4a54_59ef4eaa","updated":"2022-09-14 10:39:51.000000000","message":"Ack","commit_id":"71496a71db2f01a8f61210107392399f236cf22b"}],"openstackclient/tests/unit/compute/v2/test_server.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cfa8a9325eb0b3aa46db245a44527feac6ea597e","unresolved":true,"context_lines":[{"line_number":6287,"context_line":"        self.server.rebuild.assert_called_with("},{"line_number":6288,"context_line":"            self.image, None, reimage_boot_volume\u003dTrue)"},{"line_number":6289,"context_line":""},{"line_number":6290,"context_line":"    def test_rebuild_with_hostname_pre_v293(self):"},{"line_number":6291,"context_line":"        self.app.client_manager.compute.api_version \u003d \\"},{"line_number":6292,"context_line":"            api_versions.APIVersion(\u00272.92\u0027)"},{"line_number":6293,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"8bcc4593_7e5304a8","line":6290,"range":{"start_line":6290,"start_character":26,"end_line":6290,"end_character":34},"updated":"2022-08-31 19:32:23.000000000","message":"hostname?\n\nthis should be test_rebuild_with_reimage_boot_volume_pre_v293","commit_id":"b52bf62382b0a218bfd2f1bb328a6de2f4afdb2a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ec9b6d034526b05307e1ecd5502adaa7ab8f4746","unresolved":false,"context_lines":[{"line_number":6287,"context_line":"        self.server.rebuild.assert_called_with("},{"line_number":6288,"context_line":"            self.image, None, reimage_boot_volume\u003dTrue)"},{"line_number":6289,"context_line":""},{"line_number":6290,"context_line":"    def test_rebuild_with_hostname_pre_v293(self):"},{"line_number":6291,"context_line":"        self.app.client_manager.compute.api_version \u003d \\"},{"line_number":6292,"context_line":"            api_versions.APIVersion(\u00272.92\u0027)"},{"line_number":6293,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fc4bb791_7530ed08","line":6290,"range":{"start_line":6290,"start_character":26,"end_line":6290,"end_character":34},"in_reply_to":"8bcc4593_7e5304a8","updated":"2022-08-31 20:12:26.000000000","message":"Oops...\nDone","commit_id":"b52bf62382b0a218bfd2f1bb328a6de2f4afdb2a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9284e33390d9f40a722bdf4bfdb904fe9d8b5a14","unresolved":false,"context_lines":[{"line_number":6270,"context_line":"class TestServerRebuildVolumeBacked(TestServer):"},{"line_number":6271,"context_line":""},{"line_number":6272,"context_line":"    def setUp(self):"},{"line_number":6273,"context_line":"        super(TestServerRebuildVolumeBacked, self).setUp()"},{"line_number":6274,"context_line":""},{"line_number":6275,"context_line":"        self.new_image \u003d image_fakes.create_one_image()"},{"line_number":6276,"context_line":"        self.find_image_mock.return_value \u003d self.new_image"}],"source_content_type":"text/x-python","patch_set":7,"id":"32eb2095_4762f24a","line":6273,"range":{"start_line":6273,"start_character":14,"end_line":6273,"end_character":49},"updated":"2022-09-09 10:03:22.000000000","message":"nit: not needed in Python 3","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bcac2217fc2c6391243b30e2bbbe17759207d050","unresolved":false,"context_lines":[{"line_number":6270,"context_line":"class TestServerRebuildVolumeBacked(TestServer):"},{"line_number":6271,"context_line":""},{"line_number":6272,"context_line":"    def setUp(self):"},{"line_number":6273,"context_line":"        super(TestServerRebuildVolumeBacked, self).setUp()"},{"line_number":6274,"context_line":""},{"line_number":6275,"context_line":"        self.new_image \u003d image_fakes.create_one_image()"},{"line_number":6276,"context_line":"        self.find_image_mock.return_value \u003d self.new_image"}],"source_content_type":"text/x-python","patch_set":7,"id":"b095b6d2_01f56a82","line":6273,"range":{"start_line":6273,"start_character":14,"end_line":6273,"end_character":49},"in_reply_to":"32eb2095_4762f24a","updated":"2022-09-13 16:04:33.000000000","message":"Done","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9284e33390d9f40a722bdf4bfdb904fe9d8b5a14","unresolved":true,"context_lines":[{"line_number":6276,"context_line":"        self.find_image_mock.return_value \u003d self.new_image"},{"line_number":6277,"context_line":""},{"line_number":6278,"context_line":"        attrs \u003d {"},{"line_number":6279,"context_line":"            \u0027image\u0027: None,"},{"line_number":6280,"context_line":"            \u0027networks\u0027: {},"},{"line_number":6281,"context_line":"            \u0027adminPass\u0027: \u0027passw0rd\u0027,"},{"line_number":6282,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f45909a_514b72b9","line":6279,"range":{"start_line":6279,"start_character":21,"end_line":6279,"end_character":25},"updated":"2022-09-09 10:03:22.000000000","message":"nit: In the real world, this would actually be an empty string.","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bcac2217fc2c6391243b30e2bbbe17759207d050","unresolved":false,"context_lines":[{"line_number":6276,"context_line":"        self.find_image_mock.return_value \u003d self.new_image"},{"line_number":6277,"context_line":""},{"line_number":6278,"context_line":"        attrs \u003d {"},{"line_number":6279,"context_line":"            \u0027image\u0027: None,"},{"line_number":6280,"context_line":"            \u0027networks\u0027: {},"},{"line_number":6281,"context_line":"            \u0027adminPass\u0027: \u0027passw0rd\u0027,"},{"line_number":6282,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":7,"id":"443cd344_7ce34a08","line":6279,"range":{"start_line":6279,"start_character":21,"end_line":6279,"end_character":25},"in_reply_to":"5f45909a_514b72b9","updated":"2022-09-13 16:04:33.000000000","message":"Done","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9284e33390d9f40a722bdf4bfdb904fe9d8b5a14","unresolved":true,"context_lines":[{"line_number":6336,"context_line":"            self.cmd.take_action,"},{"line_number":6337,"context_line":"            parsed_args)"},{"line_number":6338,"context_line":""},{"line_number":6339,"context_line":"    def test_rebuild_with_confirm_reimage_pre_v293(self):"},{"line_number":6340,"context_line":"        self.app.client_manager.compute.api_version \u003d \\"},{"line_number":6341,"context_line":"            api_versions.APIVersion(\u00272.92\u0027)"},{"line_number":6342,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7e94f4e9_fa35e1a1","line":6339,"updated":"2022-09-09 10:03:22.000000000","message":"This isn\u0027t failing because of the reason you think. It\u0027s failing because you didn\u0027t specify an \u0027--image\u0027 option, meaning you fail the earlier check. You need at least one other test here to test that case (rebuilding an image-backed volume without providing an image) and then you need to add the \u0027--image\u0027 option here. It might be good to add a check of the exception string to each of these also.\n\n  exc \u003d self.assertRaises(...)\n  self.assertIn(\u0027message here\u0027, str(exc))","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bcac2217fc2c6391243b30e2bbbe17759207d050","unresolved":false,"context_lines":[{"line_number":6336,"context_line":"            self.cmd.take_action,"},{"line_number":6337,"context_line":"            parsed_args)"},{"line_number":6338,"context_line":""},{"line_number":6339,"context_line":"    def test_rebuild_with_confirm_reimage_pre_v293(self):"},{"line_number":6340,"context_line":"        self.app.client_manager.compute.api_version \u003d \\"},{"line_number":6341,"context_line":"            api_versions.APIVersion(\u00272.92\u0027)"},{"line_number":6342,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"b83be1ee_70b0e358","line":6339,"in_reply_to":"7e94f4e9_fa35e1a1","updated":"2022-09-13 16:04:33.000000000","message":"Done","commit_id":"1950292e447f3eaea13f7a39cf79a073ddd1854b"}]}