)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"afa0792e4687042635af626c6f17f396a16786fe","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     lihao \u003clihao@cmss.chinamobile.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-04-18 10:09:37 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix a bug in db/sqlalchemy"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There is a logic error in glance. I think"},{"line_number":10,"context_line":"we\u0027d better modify it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f231b9d_0ada5d16","line":7,"updated":"2017-06-10 21:28:15.000000000","message":"This commit message doesn\u0027t help to understand the aim of the commit","commit_id":"9096747d12336d511a1088e800bc11250a26f9ca"}],"glance/db/sqlalchemy/api.py":[{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"daaf53a29f40d9bb459f8756c502b8475c738af9","unresolved":false,"context_lines":[{"line_number":386,"context_line":"    if marker is not None:"},{"line_number":387,"context_line":"        marker_values \u003d []"},{"line_number":388,"context_line":"        for sort_key in sort_keys:"},{"line_number":389,"context_line":"            v \u003d getattr(marker, sort_key, None)"},{"line_number":390,"context_line":"            if v is None:"},{"line_number":391,"context_line":"                v \u003d default"},{"line_number":392,"context_line":"            marker_values.append(v)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_f536a4e7","line":389,"updated":"2017-06-13 10:53:20.000000000","message":"My opinion that this patch is useless, because it covers impossible things.\n\nI\u0027ll try to explain what I mean. \"marker\" is an Image object, and when we sort by some field, \"disk_format\" for instance, there could be a situation when the value is not set for the marker object, i.e. \"marker.disk_format is None\".\nRight now the code considers such situations and converts None to \u0027\u0027, which is right.\n\nThis patch assumes situations when user wants to sort by non-existing field, i.e. \"marker.NONEXISTING\". In this case gettattr will return the default value, which will be converted to \u0027\u0027 on the next line.\n\nWhy this is wrong:\n\n1. User can\u0027t sort by non-existing fields, it\u0027s checked in api https://github.com/openstack/glance/blob/master/glance/api/v2/images.py#L385-L387\nAnd glance will return 400 long before the execution reaches this point. That\u0027s why applying the default value here makes no sense.\n\n2. Even if execution reaches this point and it will be converted from None to \u0027\u0027, sqlalchemy will fail later, because it can\u0027t sort by nonexisting field. And Glance will return 500 anyway.\n\nI hope I put my though clearly. If there any questions I\u0027m glad to discuss them.","commit_id":"3258c1d9f375777909cd2e428fbe2a9e9d956e59"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"6264dc6cb896f2681b90c21a2519f5247f582af4","unresolved":false,"context_lines":[{"line_number":386,"context_line":"    if marker is not None:"},{"line_number":387,"context_line":"        marker_values \u003d []"},{"line_number":388,"context_line":"        for sort_key in sort_keys:"},{"line_number":389,"context_line":"            v \u003d getattr(marker, sort_key, None)"},{"line_number":390,"context_line":"            if v is None:"},{"line_number":391,"context_line":"                v \u003d default"},{"line_number":392,"context_line":"            marker_values.append(v)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_c1e64645","line":389,"in_reply_to":"7f231b9d_f536a4e7","updated":"2017-06-13 12:53:42.000000000","message":"Thanks for the detailed analysis Mike! Happy to stand by you on this one.","commit_id":"3258c1d9f375777909cd2e428fbe2a9e9d956e59"}]}
