)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"ed8c4322977c577c100f73bcfad84dc793d2a5a4","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"If no volume type can be set a volume is given the \u0027None\u0027"},{"line_number":15,"context_line":"type and the scheduler is left to figure out where the volume"},{"line_number":16,"context_line":"can be created.  When this happens an exception is thrown to"},{"line_number":17,"context_line":"the cinder-api.log file and the volume is still created."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"To make it clearer that this is intentional behavior, this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bacf61ea_f1b47a83","line":16,"updated":"2016-08-02 13:04:51.000000000","message":"I wonder if we should change this exception as well? It seems confusing to throw an exception when this is intentional behavior....","commit_id":"1956a05501e876cda5a7f28e60674f7f76dd8f31"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0e991148056db1cfde2efc1904d5c962b85ee29a","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"If no volume type can be set a volume is given the \u0027None\u0027"},{"line_number":15,"context_line":"type and the scheduler is left to figure out where the volume"},{"line_number":16,"context_line":"can be created.  When this happens an exception is thrown to"},{"line_number":17,"context_line":"the cinder-api.log file and the volume is still created."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"To make it clearer that this is intentional behavior, this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bacf61ea_c12b5393","line":16,"in_reply_to":"bacf61ea_f1b47a83","updated":"2016-08-02 18:13:44.000000000","message":"We could ... maybe change it to a LOG.error .  We still want a message in there that puts the trail of breadcrumbs in, the exception does seem a little odd.","commit_id":"1956a05501e876cda5a7f28e60674f7f76dd8f31"}],"cinder/volume/flows/api/create_volume.py":[{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"a1b42fcdac8a586a2a0f13b3b6f5cc29a5f617e0","unresolved":false,"context_lines":[{"line_number":409,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":410,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"},{"line_number":411,"context_line":"                           def_vol_type)"},{"line_number":412,"context_line":"            if not volume_type:"},{"line_number":413,"context_line":"                LOG.warning(_LW(\"Unable to determine a volume type. \""},{"line_number":414,"context_line":"                                \"Continuing with \u0027None\u0027 for the volume type.\"))"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_682aea2d","line":412,"updated":"2016-07-29 13:22:10.000000000","message":"I think we need to warn only if the default volume type config option is not empty.\n\nif conf.default_volume_type and not volume_type:","commit_id":"c43b7e122f47658128a2deadc2e43be1d4664273"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"e3d5d234cc3308ffe0d9133874ae55d57d1ff67d","unresolved":false,"context_lines":[{"line_number":409,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":410,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"},{"line_number":411,"context_line":"                           def_vol_type)"},{"line_number":412,"context_line":"            if not volume_type:"},{"line_number":413,"context_line":"                LOG.warning(_LW(\"Unable to determine a volume type. \""},{"line_number":414,"context_line":"                                \"Continuing with \u0027None\u0027 for the volume type.\"))"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_e3233754","line":412,"in_reply_to":"bacf61ea_682aea2d","updated":"2016-08-02 12:50:03.000000000","message":"Fair enough.  I can do that.","commit_id":"c43b7e122f47658128a2deadc2e43be1d4664273"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"05ff0c9108c31c489d8e9ebdec9bdba1c592a62c","unresolved":false,"context_lines":[{"line_number":409,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":410,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"},{"line_number":411,"context_line":"                           def_vol_type)"},{"line_number":412,"context_line":"            if CONF.default_volume_type and not volume_type:"},{"line_number":413,"context_line":"                LOG.warning(_LW(\"Unable to determine a volume type. \""},{"line_number":414,"context_line":"                                \"Continuing with \u0027None\u0027 for the volume type.\"))"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_6134274a","line":412,"updated":"2016-08-02 18:06:58.000000000","message":"if default_volume_type is set, shouldn\u0027t we use it?  \n\nshouldn\u0027t this be a test of\nif not CONF.default_volume_type and not volume_type:","commit_id":"1956a05501e876cda5a7f28e60674f7f76dd8f31"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0e991148056db1cfde2efc1904d5c962b85ee29a","unresolved":false,"context_lines":[{"line_number":409,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":410,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"},{"line_number":411,"context_line":"                           def_vol_type)"},{"line_number":412,"context_line":"            if CONF.default_volume_type and not volume_type:"},{"line_number":413,"context_line":"                LOG.warning(_LW(\"Unable to determine a volume type. \""},{"line_number":414,"context_line":"                                \"Continuing with \u0027None\u0027 for the volume type.\"))"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_a1761fd6","line":412,"in_reply_to":"bacf61ea_6134274a","updated":"2016-08-02 18:13:44.000000000","message":"So, if the default_volume_type is set that is what will be used because it is obtained on line 407.  That is what will be used on line 410.  In the case that it is not set, then we may end up with no volume_type set on line 412.  At that point we want to report an error because the administrator is expecting a default volume type but we couldn\u0027t find it ... I.E. it doesn\u0027t exist and thus the reason we throw the warning.","commit_id":"1956a05501e876cda5a7f28e60674f7f76dd8f31"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"4fe33a0518e04e4cba56ca6cbc70655ac40eaae0","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        # should copy encryption metadata from the encrypted volume type to the"},{"line_number":417,"context_line":"        # volume upon creation and propagate that information to each snapshot."},{"line_number":418,"context_line":"        # This strategy avoids any dependency upon the encrypted volume type."},{"line_number":419,"context_line":"        def_vol_type \u003d volume_types.get_default_volume_type()"},{"line_number":420,"context_line":"        if not volume_type and not source_volume and not snapshot:"},{"line_number":421,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":422,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"}],"source_content_type":"text/x-python","patch_set":3,"id":"3ac371cc_72406108","line":419,"updated":"2016-08-19 09:38:40.000000000","message":"I\u0027d rather add try/except to above line to catch VolumeTypeNotFoundByName exception and add the warning log here and then assign def_vol_type to None.","commit_id":"b6bcee83c8f910eb6bbe7240a7efe4e307431024"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"3ca8bd66fb0fc7ba3e3d067b240bccd0ed24076e","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        # should copy encryption metadata from the encrypted volume type to the"},{"line_number":417,"context_line":"        # volume upon creation and propagate that information to each snapshot."},{"line_number":418,"context_line":"        # This strategy avoids any dependency upon the encrypted volume type."},{"line_number":419,"context_line":"        def_vol_type \u003d volume_types.get_default_volume_type()"},{"line_number":420,"context_line":"        if not volume_type and not source_volume and not snapshot:"},{"line_number":421,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":422,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_9ea4e4a6","line":419,"in_reply_to":"1ac06dbe_03090e32","updated":"2016-08-22 00:51:15.000000000","message":"You are right.  get_default_volume_type() doesn\u0027t raise any exceptions, I think it should so that caller of this function doesn\u0027t have to do what Bryant\u0027s doing here to learn something goes wrong.","commit_id":"b6bcee83c8f910eb6bbe7240a7efe4e307431024"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8fafb0d4cfa7496042658ae3986165630c7768dd","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        # should copy encryption metadata from the encrypted volume type to the"},{"line_number":417,"context_line":"        # volume upon creation and propagate that information to each snapshot."},{"line_number":418,"context_line":"        # This strategy avoids any dependency upon the encrypted volume type."},{"line_number":419,"context_line":"        def_vol_type \u003d volume_types.get_default_volume_type()"},{"line_number":420,"context_line":"        if not volume_type and not source_volume and not snapshot:"},{"line_number":421,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":422,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_f55d297a","line":419,"in_reply_to":"1ac06dbe_9ea4e4a6","updated":"2016-08-22 09:30:19.000000000","message":"I agree with Huang, it\u0027s better to make get_default_volume_type not catch the VolumeTypeNotFoundByName exception and catch it here.\n\nAnd I think we should also raise the exception if default_volume_type is None.","commit_id":"b6bcee83c8f910eb6bbe7240a7efe4e307431024"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"fec17ac438c5e54922e65d0440ef3c694bc5a6f4","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        # should copy encryption metadata from the encrypted volume type to the"},{"line_number":417,"context_line":"        # volume upon creation and propagate that information to each snapshot."},{"line_number":418,"context_line":"        # This strategy avoids any dependency upon the encrypted volume type."},{"line_number":419,"context_line":"        def_vol_type \u003d volume_types.get_default_volume_type()"},{"line_number":420,"context_line":"        if not volume_type and not source_volume and not snapshot:"},{"line_number":421,"context_line":"            image_volume_type \u003d self._get_image_volume_type(context, image_id)"},{"line_number":422,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_03090e32","line":419,"in_reply_to":"3ac371cc_72406108","updated":"2016-08-19 10:17:52.000000000","message":"\u003e add the warning log here\n\nvolume_types.get_default_volume_type doesn\u0027t raise any exception.\n\nAlso, we need to warn only if all the below conditions are true:\n* no volume type specified for the volume\n\n* no volume type in the source image\n\n* no default volume type","commit_id":"b6bcee83c8f910eb6bbe7240a7efe4e307431024"}]}
