)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e0e8a684526b11c7d4f1d478d0afcbcd425382f6","unresolved":false,"context_lines":[{"line_number":10,"context_line":"a \u0027VolumeNotFound\u0027 exception will be raised. This is not currently"},{"line_number":11,"context_line":"handled. Correct this."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The behavior of the caller throwing the error is already validated in"},{"line_number":14,"context_line":"\u0027ComputeAPIUnitTestCase.test_get_volumes_for_bdms_errors\u0027 in"},{"line_number":15,"context_line":"\u0027nova/tests/unit/compute/test_api.py\u0027. There is therefore no additional"},{"line_number":16,"context_line":"unit test added here since it would simply be validating that one"},{"line_number":17,"context_line":"exception is transformed into another, which we already do for various"},{"line_number":18,"context_line":"other exceptions."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I6137dc1b6b51321fee1c080bf4b85197b19bf223"},{"line_number":21,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"83968829_0a05c5b7","line":18,"range":{"start_line":13,"start_character":0,"end_line":18,"end_character":17},"updated":"2021-06-02 11:24:33.000000000","message":"Fair given the size of the required test given the amount of stuff happening in the API *before* we call down into the compute API.","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e1c796dda4c84e542acef9635b67faeed99098f0","unresolved":false,"context_lines":[{"line_number":10,"context_line":"a \u0027VolumeNotFound\u0027 exception will be raised. This is not currently"},{"line_number":11,"context_line":"handled. Correct this."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The behavior of the caller throwing the error is already validated in"},{"line_number":14,"context_line":"\u0027ComputeAPIUnitTestCase.test_get_volumes_for_bdms_errors\u0027 in"},{"line_number":15,"context_line":"\u0027nova/tests/unit/compute/test_api.py\u0027. There is therefore no additional"},{"line_number":16,"context_line":"unit test added here since it would simply be validating that one"},{"line_number":17,"context_line":"exception is transformed into another, which we already do for various"},{"line_number":18,"context_line":"other exceptions."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I6137dc1b6b51321fee1c080bf4b85197b19bf223"},{"line_number":21,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"d6082698_98c5ca5a","line":18,"range":{"start_line":13,"start_character":0,"end_line":18,"end_character":17},"in_reply_to":"83968829_0a05c5b7","updated":"2021-06-02 19:52:30.000000000","message":"But API controller side handling is not tested in that test right? I mean if someone accidentally remove  \u0027exception.VolumeNotFound,\u0027 from API controller in a big change patch or so then it is easy to miss this. I think adding a simple API controller test (compute_api.create() raise VolumeNotFound and API return 400 to user) will be helpful here.","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c9f00efef14b457bbe0455374c0c141055db86a2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"a \u0027VolumeNotFound\u0027 exception will be raised. This is not currently"},{"line_number":11,"context_line":"handled. Correct this."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The behavior of the caller throwing the error is already validated in"},{"line_number":14,"context_line":"\u0027ComputeAPIUnitTestCase.test_get_volumes_for_bdms_errors\u0027 in"},{"line_number":15,"context_line":"\u0027nova/tests/unit/compute/test_api.py\u0027. There is therefore no additional"},{"line_number":16,"context_line":"unit test added here since it would simply be validating that one"},{"line_number":17,"context_line":"exception is transformed into another, which we already do for various"},{"line_number":18,"context_line":"other exceptions."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I6137dc1b6b51321fee1c080bf4b85197b19bf223"},{"line_number":21,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"80da203e_8c2c9b9c","line":18,"range":{"start_line":13,"start_character":0,"end_line":18,"end_character":17},"in_reply_to":"d6082698_98c5ca5a","updated":"2021-07-07 13:28:56.000000000","message":"There are 40 exceptions in that particular except block though and we currently have tests for 16 of these, pretty much all of which look like this:\n\n  @mock.patch.object(compute_api.API, \u0027create\u0027,\n                     side_effect\u003dexception.InvalidVolume(reason\u003d\u0027error\u0027))\n  def test_create_instance_with_invalid_volume_error(self, create_mock):\n      # Tests that InvalidVolume is translated to a 400 error.\n      self.assertRaises(webob.exc.HTTPBadRequest,\n                        self._test_create_extra, {})\n\ni.e. check that we translate exception X to HTTPBadRequest. I don\u0027t think these are helpful tests but if you insist 😅","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"}],"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5798849f767978ebce489cd2dd3f86ac7f4c87ba","unresolved":true,"context_lines":[{"line_number":743,"context_line":"                exception.NetworkNotFound,"},{"line_number":744,"context_line":"                exception.InvalidBDM,"},{"line_number":745,"context_line":"                exception.InvalidBDMSnapshot,"},{"line_number":746,"context_line":"                exception.InvalidBDMVolume,"},{"line_number":747,"context_line":"                exception.InvalidBDMImage,"},{"line_number":748,"context_line":"                exception.InvalidBDMBootSequence,"},{"line_number":749,"context_line":"                exception.InvalidBDMLocalsLimit,"}],"source_content_type":"text/x-python","patch_set":1,"id":"8574eddb_943f7aab","line":746,"range":{"start_line":746,"start_character":26,"end_line":746,"end_character":42},"updated":"2021-06-01 18:25:21.000000000","message":"Weird, given the trace you posted I\u0027d expect this to be raised first:\n\nhttp://paste.openstack.org/show/806238/\n\nhttps://github.com/openstack/nova/blob/c5619c7e3e1c55b7ac711fff8f7a836c206d96f8/nova/compute/api.py#L1299-L1305\n\nosc didn\u0027t set image_href did it?","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e0e8a684526b11c7d4f1d478d0afcbcd425382f6","unresolved":false,"context_lines":[{"line_number":743,"context_line":"                exception.NetworkNotFound,"},{"line_number":744,"context_line":"                exception.InvalidBDM,"},{"line_number":745,"context_line":"                exception.InvalidBDMSnapshot,"},{"line_number":746,"context_line":"                exception.InvalidBDMVolume,"},{"line_number":747,"context_line":"                exception.InvalidBDMImage,"},{"line_number":748,"context_line":"                exception.InvalidBDMBootSequence,"},{"line_number":749,"context_line":"                exception.InvalidBDMLocalsLimit,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7cab2e2c_97eae183","line":746,"range":{"start_line":746,"start_character":26,"end_line":746,"end_character":42},"in_reply_to":"84a790ae_cfb0de01","updated":"2021-06-02 11:24:33.000000000","message":"Ahhh right sorry I got confused by some output I saw fly past in IRC, it makes sense why this wasn\u0027t raised now.","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0f09dc26c9b7f9e2742bed154fd0e6e289825a4","unresolved":true,"context_lines":[{"line_number":743,"context_line":"                exception.NetworkNotFound,"},{"line_number":744,"context_line":"                exception.InvalidBDM,"},{"line_number":745,"context_line":"                exception.InvalidBDMSnapshot,"},{"line_number":746,"context_line":"                exception.InvalidBDMVolume,"},{"line_number":747,"context_line":"                exception.InvalidBDMImage,"},{"line_number":748,"context_line":"                exception.InvalidBDMBootSequence,"},{"line_number":749,"context_line":"                exception.InvalidBDMLocalsLimit,"}],"source_content_type":"text/x-python","patch_set":1,"id":"84a790ae_cfb0de01","line":746,"range":{"start_line":746,"start_character":26,"end_line":746,"end_character":42},"in_reply_to":"8574eddb_943f7aab","updated":"2021-06-02 10:52:23.000000000","message":"Yes, it did. I wasn\u0027t booting from a volume. I was booting from an image with an additional non-bootable volume attached. Here\u0027s the full command I used:\n\n  openstack server create \\\n    --flavor m1.tiny --image cirros-0.5.1-x86_64-disk --network private \\\n    --block-device source_type\u003dvolume,uuid\u003d44d317a3-6183-4063-868b-aa0728576f5f,destination_type\u003dvolume,delete_on_termination\u003dtrue \\\n    --wait test-serve\n\n(\u002744d317a3-6183-4063-868b-aa0728576f5f\u0027 was actually the UUID of the \u0027cirros-0.5.1-x86_64-disk\u0027 image which I grabbed by mistake)\n\nThis should be perfectly legal, right? I think this would yield two devices: an ephemeral device built from the image of size $flavor.disk, and the volume which will be deleted on termination (due to the flag being set).","commit_id":"b9c992a45c4c0e604623f540d86817d77b3f119b"}]}