)]}'
{"glance/db/sqlalchemy/api.py":[{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"023c596d473944bd63796d59ac9981300ffc417d","unresolved":false,"context_lines":[{"line_number":505,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":506,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr !\u003d None,"},{"line_number":507,"context_line":"                                              model_attr), ],"},{"line_number":508,"context_line":"                                              else_\u003dNone if sort_keys[j] in"},{"line_number":509,"context_line":"                                              dt_columns else default)"},{"line_number":510,"context_line":"                crit_attrs.append((attr \u003d\u003d marker_values[j]))"},{"line_number":511,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BW%2Bs%3D","line":508,"updated":"2013-11-26 23:31:58.000000000","message":"Unfortunately, this change is making some code that is already convoluted even more so. And based on what its changing, I can\u0027t really see how we can write a unit test without a major restructuring.\n\nIs there at least any way we can make what\u0027s going on a bit more clear?\n\nOne idea I had was, it looks like we could defer setting default to  right after line 504. Then we could base whether or not default\u003d\u0027\u0027 or None to the column name there.\n\nAlso, it might make more sense if we check the type of model_attr from line 505 rather than just relying on column names.\n\nFinally, this method looks like it might benefit from judicious use of the builtin enumerate(). If you like (ie not a blocking request at all) it might be nice to see if enumerate cleans things up here a bit.\n\nThanks!","commit_id":"3fe1adc0a74fe20ba1caee598ade2548b3c4635c"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"7c6d711b12a9091ef747fa15784fe2577948623b","unresolved":false,"context_lines":[{"line_number":505,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":506,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr !\u003d None,"},{"line_number":507,"context_line":"                                              model_attr), ],"},{"line_number":508,"context_line":"                                              else_\u003dNone if sort_keys[j] in"},{"line_number":509,"context_line":"                                              dt_columns else default)"},{"line_number":510,"context_line":"                crit_attrs.append((attr \u003d\u003d marker_values[j]))"},{"line_number":511,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BJs4%3D","line":508,"in_reply_to":"AAAATn%2F%2BW%2Bs%3D","updated":"2013-12-01 16:00:57.000000000","message":"Thanks, Mark. I think it\u0027s a good suggestion. I will investigate how to get the column type right after line 505 and then set the \u0027default\u0027 value based on the column type. Will update it in next PS.","commit_id":"3fe1adc0a74fe20ba1caee598ade2548b3c4635c"},{"author":{"_account_id":7884,"name":"Noboru Arai","email":"arai@mxa.nes.nec.co.jp","username":"NoboruArai"},"change_message_id":"9df03f5562097d5a6d2701c507c8d22765e8e11e","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":500,"context_line":"                default \u003d None if isinstance("},{"line_number":501,"context_line":"                            model_attr.property.columns[0].type,"},{"line_number":502,"context_line":"                            sqlalchemy.DateTime) else \u0027\u0027"},{"line_number":503,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr !\u003d None,"},{"line_number":504,"context_line":"                                              model_attr), ],"},{"line_number":505,"context_line":"                                              else_\u003ddefault)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F93ss%3D","line":502,"updated":"2013-12-05 01:15:43.000000000","message":"in line483, variable \"default\" is defined empty string(\u0027\u0027). so, I would prefer that  empty string is replaced with \"defalut\" variable  in this \"else\" clause.","commit_id":"3f533a28fa79e5ad7d2703123565f34839cd9ff3"},{"author":{"_account_id":7884,"name":"Noboru Arai","email":"arai@mxa.nes.nec.co.jp","username":"NoboruArai"},"change_message_id":"234a6bbea89ef91743f30ceca30c1e20a8f992d5","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":500,"context_line":"                default \u003d None if isinstance("},{"line_number":501,"context_line":"                            model_attr.property.columns[0].type,"},{"line_number":502,"context_line":"                            sqlalchemy.DateTime) else \u0027\u0027"},{"line_number":503,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr !\u003d None,"},{"line_number":504,"context_line":"                                              model_attr), ],"},{"line_number":505,"context_line":"                                              else_\u003ddefault)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F92q0%3D","line":502,"in_reply_to":"AAAATn%2F92%2F8%3D","updated":"2013-12-05 08:03:31.000000000","message":"sorry,  I was wrong.","commit_id":"3f533a28fa79e5ad7d2703123565f34839cd9ff3"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"ad3198255e8b0b56104f034f6c250314567281bb","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":500,"context_line":"                default \u003d None if isinstance("},{"line_number":501,"context_line":"                            model_attr.property.columns[0].type,"},{"line_number":502,"context_line":"                            sqlalchemy.DateTime) else \u0027\u0027"},{"line_number":503,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr !\u003d None,"},{"line_number":504,"context_line":"                                              model_attr), ],"},{"line_number":505,"context_line":"                                              else_\u003ddefault)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F92%2F8%3D","line":502,"in_reply_to":"AAAATn%2F93ss%3D","updated":"2013-12-05 06:12:18.000000000","message":"I would say no. Because we\u0027re in a loop and the value of \u0027default\u0027 is changing.","commit_id":"3f533a28fa79e5ad7d2703123565f34839cd9ff3"}]}