)]}'
{"manila/share/manager.py":[{"author":{"_account_id":31506,"name":"Lin PeiWen","email":"591171850@qq.com","username":"LinPeiWen"},"change_message_id":"f7fea298a64ec8f066c1c28d083ca9bcdec2d064","unresolved":true,"context_lines":[{"line_number":2028,"context_line":"                    # model_update will return only the export locations"},{"line_number":2029,"context_line":"                    export_locations \u003d model_update"},{"line_number":2030,"context_line":"                elif isinstance(model_update, str):"},{"line_number":2031,"context_line":"                    export_locations \u003d model_update"},{"line_number":2032,"context_line":"                else:"},{"line_number":2033,"context_line":"                    # NOTE(dviroel): share status is mandatory when answering"},{"line_number":2034,"context_line":"                    # a model update. If not provided, won\u0027t be possible to"}],"source_content_type":"text/x-python","patch_set":2,"id":"fd684796_500d23a9","line":2031,"updated":"2021-04-27 07:35:11.000000000","message":"Thanks Tom Barron, I update the patch, you can review if you have time","commit_id":"e4d0a99c6cc0a6789a21184677bfedd2692f8271"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"af3a4968d3238cfc6d6e300b5d4101cd9560beca","unresolved":true,"context_lines":[{"line_number":2028,"context_line":"                    # model_update will return only the export locations"},{"line_number":2029,"context_line":"                    export_locations \u003d model_update"},{"line_number":2030,"context_line":"                elif isinstance(model_update, str):"},{"line_number":2031,"context_line":"                    export_locations \u003d model_update"},{"line_number":2032,"context_line":"                else:"},{"line_number":2033,"context_line":"                    # NOTE(dviroel): share status is mandatory when answering"},{"line_number":2034,"context_line":"                    # a model update. If not provided, won\u0027t be possible to"}],"source_content_type":"text/x-python","patch_set":2,"id":"66befd3f_e12af5f2","line":2031,"updated":"2021-04-08 16:12:34.000000000","message":"Would it make sense to do this for line 2031?\n       export_locations \u003d [model_update]\nWhat you have may work fine but it seems to me that our code is expecting to handle a *list* of export locations.","commit_id":"e4d0a99c6cc0a6789a21184677bfedd2692f8271"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"a25eba9311898a0240296dc308461f431437764b","unresolved":true,"context_lines":[{"line_number":2028,"context_line":"                    # model_update will return only the export locations"},{"line_number":2029,"context_line":"                    export_locations \u003d model_update"},{"line_number":2030,"context_line":"                elif isinstance(model_update, str):"},{"line_number":2031,"context_line":"                    export_locations \u003d model_update"},{"line_number":2032,"context_line":"                else:"},{"line_number":2033,"context_line":"                    # NOTE(dviroel): share status is mandatory when answering"},{"line_number":2034,"context_line":"                    # a model update. If not provided, won\u0027t be possible to"}],"source_content_type":"text/x-python","patch_set":2,"id":"a5325e30_6a9c136e","line":2031,"in_reply_to":"3cf17ebe_ea779f2d","updated":"2021-04-20 11:28:23.000000000","message":"Hmm, I don\u0027t think line 2040 will be str data type unless your driver is returning that rather than a list.  In the reference driver [1] export_locations is a list and in the sqlalchemy DB model share export locations it\u0027s a list [2].  There is compatibility code in the sqlalchemy api to accommodate drivers that still return a single export location as a string rather than a list but the expectation nowadays is that it\u0027s not just a string so I prefer that we keep that expectation where we can.\n\nI\u0027m not \"dug in\" about this so let\u0027s see what other reviewers think :D\n\n[1] https://opendev.org/openstack/manila/src/branch/master/manila/share/driver.py#L677\n[2] https://opendev.org/openstack/manila/blob/master/manila/db/sqlalchemy/models.py#L210\n[3] https://opendev.org/openstack/manila/src/branch/master/manila/db/sqlalchemy/api.py#L3545","commit_id":"e4d0a99c6cc0a6789a21184677bfedd2692f8271"},{"author":{"_account_id":31506,"name":"Lin PeiWen","email":"591171850@qq.com","username":"LinPeiWen"},"change_message_id":"c3b247f7f20e4596e4557acf290531f71af37ca1","unresolved":true,"context_lines":[{"line_number":2028,"context_line":"                    # model_update will return only the export locations"},{"line_number":2029,"context_line":"                    export_locations \u003d model_update"},{"line_number":2030,"context_line":"                elif isinstance(model_update, str):"},{"line_number":2031,"context_line":"                    export_locations \u003d model_update"},{"line_number":2032,"context_line":"                else:"},{"line_number":2033,"context_line":"                    # NOTE(dviroel): share status is mandatory when answering"},{"line_number":2034,"context_line":"                    # a model update. If not provided, won\u0027t be possible to"}],"source_content_type":"text/x-python","patch_set":2,"id":"3cf17ebe_ea779f2d","line":2031,"in_reply_to":"66befd3f_e12af5f2","updated":"2021-04-20 10:19:51.000000000","message":"\u003e Would it make sense to do this for line 2031?\n\u003e        export_locations \u003d [model_update]\n\u003e What you have may work fine but it seems to me that our code is expecting to handle a *list* of export locations.\n\nThe export_locations (line 2040) of create share instance is the str data type, I just keep it consistent","commit_id":"e4d0a99c6cc0a6789a21184677bfedd2692f8271"}]}