)]}'
{"cinder/volume/api.py":[{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"d3feff04e9bf980dc95e474123cd9ee97dc0d0c9","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        if source_volume and volume_type:"},{"line_number":262,"context_line":"            if source_volume.volume_type_id is None and volume_type.id:"},{"line_number":263,"context_line":"                source_volume.volume_type_id \u003d volume_type.id"},{"line_number":264,"context_line":"            elif volume_type.id !\u003d source_volume.volume_type_id:"},{"line_number":265,"context_line":"                if not self._retype_is_possible("},{"line_number":266,"context_line":"                        context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_16e6f417","line":263,"range":{"start_line":263,"start_character":16,"end_line":263,"end_character":61},"updated":"2018-01-22 17:39:31.000000000","message":"I don\u0027t think this is a good idea; we should not be modifying/updating the source object here.  I get the idea; and it does save some steps in the code but modifying the object (even temporarily) is no good.  What about using a variable?","commit_id":"fc3a27be99b13a111c6ef701fb0d75d73ccaf3ea"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c276a9ab4f07a22d6370ffd3955c0ab24e6c25d9","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        if source_volume and volume_type:"},{"line_number":262,"context_line":"            if source_volume.volume_type_id is None and volume_type.id:"},{"line_number":263,"context_line":"                source_volume.volume_type_id \u003d volume_type.id"},{"line_number":264,"context_line":"            elif volume_type.id !\u003d source_volume.volume_type_id:"},{"line_number":265,"context_line":"                if not self._retype_is_possible("},{"line_number":266,"context_line":"                        context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_53d23836","line":263,"range":{"start_line":263,"start_character":16,"end_line":263,"end_character":61},"in_reply_to":"7f96bb07_16e6f417","updated":"2018-01-23 08:00:38.000000000","message":"Thanks for reviewing my code.\nYes, it\u0027s better to use temporary variables.I\u0027ll change it.","commit_id":"fc3a27be99b13a111c6ef701fb0d75d73ccaf3ea"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"d3feff04e9bf980dc95e474123cd9ee97dc0d0c9","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            elif volume_type.id !\u003d source_volume.volume_type_id:"},{"line_number":265,"context_line":"                if not self._retype_is_possible("},{"line_number":266,"context_line":"                        context,"},{"line_number":267,"context_line":"                        source_volume.volume_type,"},{"line_number":268,"context_line":"                        volume_type):"},{"line_number":269,"context_line":"                    msg \u003d _(\"Invalid volume_type provided: %s (requested type \""},{"line_number":270,"context_line":"                            \"is not compatible; either match source volume, \""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_f6d4a8eb","line":267,"range":{"start_line":267,"start_character":24,"end_line":267,"end_character":50},"updated":"2018-01-22 17:39:31.000000000","message":"You can do the same thing you did above, but use a variable instead of changing the source_volume parameter.","commit_id":"fc3a27be99b13a111c6ef701fb0d75d73ccaf3ea"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c276a9ab4f07a22d6370ffd3955c0ab24e6c25d9","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            elif volume_type.id !\u003d source_volume.volume_type_id:"},{"line_number":265,"context_line":"                if not self._retype_is_possible("},{"line_number":266,"context_line":"                        context,"},{"line_number":267,"context_line":"                        source_volume.volume_type,"},{"line_number":268,"context_line":"                        volume_type):"},{"line_number":269,"context_line":"                    msg \u003d _(\"Invalid volume_type provided: %s (requested type \""},{"line_number":270,"context_line":"                            \"is not compatible; either match source volume, \""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_13dcb044","line":267,"range":{"start_line":267,"start_character":24,"end_line":267,"end_character":50},"in_reply_to":"7f96bb07_f6d4a8eb","updated":"2018-01-23 08:00:38.000000000","message":"Done","commit_id":"fc3a27be99b13a111c6ef701fb0d75d73ccaf3ea"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"8bdf730b81ccae384bb02f6b2433d067d56473cc","unresolved":false,"context_lines":[{"line_number":259,"context_line":"                raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        if source_volume and volume_type:"},{"line_number":262,"context_line":"            temp_source_vol \u003d source_volume"},{"line_number":263,"context_line":"            if temp_source_vol.volume_type_id is None and volume_type.id:"},{"line_number":264,"context_line":"                temp_source_vol.volume_type_id \u003d volume_type.id"},{"line_number":265,"context_line":"            elif volume_type.id !\u003d temp_source_vol.volume_type_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f96bb07_3f83aa06","line":262,"range":{"start_line":262,"start_character":12,"end_line":262,"end_character":27},"updated":"2018-01-23 11:32:56.000000000","message":"This just creates another variable that points to the same object, so although you are using temp_source_vol instead of source_vol, any modification to temp_source_vol is going to still be reflected in source_volume.","commit_id":"f83c13ea3e9cdb5ac2d84b5937f6b8320377440d"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"f02ff9f38ca616b5c0d2370f316ba6e58077e174","unresolved":false,"context_lines":[{"line_number":259,"context_line":"                raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        if source_volume and volume_type:"},{"line_number":262,"context_line":"            temp_source_vol \u003d source_volume"},{"line_number":263,"context_line":"            if temp_source_vol.volume_type_id is None and volume_type.id:"},{"line_number":264,"context_line":"                temp_source_vol.volume_type_id \u003d volume_type.id"},{"line_number":265,"context_line":"            elif volume_type.id !\u003d temp_source_vol.volume_type_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f96bb07_7ae86015","line":262,"range":{"start_line":262,"start_character":12,"end_line":262,"end_character":27},"in_reply_to":"7f96bb07_3f83aa06","updated":"2018-01-23 11:53:06.000000000","message":"Thanks for reviewing my code, I will change it.","commit_id":"f83c13ea3e9cdb5ac2d84b5937f6b8320377440d"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"91d9ea6c525257052d0c60e12ce9552e9aeb8d0e","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        if source_volume and volume_type:"},{"line_number":263,"context_line":"            temp_source_vol \u003d copy.deepcopy(source_volume)"},{"line_number":264,"context_line":"            if temp_source_vol.volume_type_id is None and volume_type.id:"},{"line_number":265,"context_line":"                temp_source_vol.volume_type_id \u003d volume_type.id"},{"line_number":266,"context_line":"            elif volume_type.id !\u003d temp_source_vol.volume_type_id:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_6871650c","line":263,"range":{"start_line":263,"start_character":12,"end_line":263,"end_character":58},"updated":"2018-01-23 22:38:08.000000000","message":"I\u0027m sorry, I should\u0027ve made a suggestion here.  What I was thinking was that the only thing the \"self._retype_is_possible\" method needs is the type objects themeselves.  You can get this a couple of different ways including extracting the Volume Type ID and fetching, but you don\u0027t even need to do that because you can just pull it out of the volume object we already have:\n    src_type \u003d source_volume.volume_type\n\nThen you can just say:\n    if not self._retype_is_possible(context, src_type, volume_type):\n        msg \u003d _(\"Invalid volume_type.......\n\nIf we\u0027re going to just go ahead and attempt the retype in the case of None we have two options to fix the actual bug you\u0027re working on:\n1.  If the src_type is None, just set it to the volume_type passed in\n            \n    This basically does the same thing your attempting to do here but it \n    does it without modifying the src volume object, and without doing \n    any sort of unnecessary deep copy of the volume object.\n\n2. Fix the _retype_is_possible helper method\n\n    We\u0027re only using that helper here and in the create from snapshot after this (which I would imagine has the same bug maybe?) . So, perhaps the cleaner way to address this is to modify the _retype_is_possible method to deal with src_type of None and just return \"True\".  In essence we\u0027re just doing a lot of work here manipulating and faking the inputs to get a True response from that helper.\n\nLet me know what you think, I haven\u0027t looked too closely at the actual bug, so maybe I\u0027m missing something that you noticed.","commit_id":"2625eb32fd2539cf73532b7e59afae6ec05b9df0"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"165bd8d7aff4ba7644642f003b133ea7454bffe3","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        if source_volume and volume_type:"},{"line_number":263,"context_line":"            temp_source_vol \u003d copy.deepcopy(source_volume)"},{"line_number":264,"context_line":"            if temp_source_vol.volume_type_id is None and volume_type.id:"},{"line_number":265,"context_line":"                temp_source_vol.volume_type_id \u003d volume_type.id"},{"line_number":266,"context_line":"            elif volume_type.id !\u003d temp_source_vol.volume_type_id:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_b87fa6dd","line":263,"range":{"start_line":263,"start_character":12,"end_line":263,"end_character":58},"in_reply_to":"7f96bb07_3791789a","updated":"2018-01-24 14:14:25.000000000","message":"Agree with John Griffith about fixing _retype_is_possible helper method, however we now only support cloning volumes in the same backend, so we cannot just return True from _retype_is_possible when the source volume type is None, instead the backend of the source volume should be checked againt the dest volume type.","commit_id":"2625eb32fd2539cf73532b7e59afae6ec05b9df0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"9097eb766f4254a20578fee3a1423ffb064a0248","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        if source_volume and volume_type:"},{"line_number":263,"context_line":"            temp_source_vol \u003d copy.deepcopy(source_volume)"},{"line_number":264,"context_line":"            if temp_source_vol.volume_type_id is None and volume_type.id:"},{"line_number":265,"context_line":"                temp_source_vol.volume_type_id \u003d volume_type.id"},{"line_number":266,"context_line":"            elif volume_type.id !\u003d temp_source_vol.volume_type_id:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_3791789a","line":263,"range":{"start_line":263,"start_character":12,"end_line":263,"end_character":58},"in_reply_to":"7f96bb07_6871650c","updated":"2018-01-24 01:37:33.000000000","message":"Hi John Griffith.Thank you for reviewing my code.\nI think there are two places to consider：\n1.When the source_volume_type is None, I want to do an attempt to create a volume.\n  As you said \"Fix the _retype_is_possible helper method(add if src_type is None: return True)\",\n  this contains another interface with create from snapshot.\n2.The other one is selected volume type and source volume type are inconsistent（The back-end of the host is inconsistent）,\n  I need to think twice about this situation.\nDo you agree with my idea? Or have any suggestions?","commit_id":"2625eb32fd2539cf73532b7e59afae6ec05b9df0"}]}