)]}'
{"cliff/lister.py":[{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"8e454f8e36886e7164b3d87bc827b4b7fcabf4d2","unresolved":true,"context_lines":[{"line_number":73,"context_line":"                except Exception:"},{"line_number":74,"context_line":"                    # simply log and then ignore this; sorting is best effort"},{"line_number":75,"context_line":"                    LOG.warning("},{"line_number":76,"context_line":"                        \"Could not sort on field \u0027%s\u0027; unsortable types\","},{"line_number":77,"context_line":"                        parsed_args.sort_columns[index],"},{"line_number":78,"context_line":"                    )"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f0db2e9b_708b6ae0","line":76,"updated":"2021-01-28 15:29:40.000000000","message":"line 73 catches all exceptions, but this message implies that the error is always because of an unsortable type. Could there be other reasons?\n\nI can\u0027t remember, does LOG.warning() show the exception automatically?","commit_id":"609b2fc987664b77a51ab9f099461440a04202f9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2bd029095cfe7171bb60771c96e2718d92401ce0","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                except Exception:"},{"line_number":74,"context_line":"                    # simply log and then ignore this; sorting is best effort"},{"line_number":75,"context_line":"                    LOG.warning("},{"line_number":76,"context_line":"                        \"Could not sort on field \u0027%s\u0027; unsortable types\","},{"line_number":77,"context_line":"                        parsed_args.sort_columns[index],"},{"line_number":78,"context_line":"                    )"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c55088d7_8ec10be0","line":76,"in_reply_to":"f0db2e9b_708b6ae0","updated":"2021-01-28 16:18:25.000000000","message":"Whoops, that should have be \u0027TypeError\u0027.\n\nLooking at the output of e.g. \u0027openstack server list --debug\u0027 I see some things that correspond to log entries from osc-lib so I think we\u0027re good.","commit_id":"609b2fc987664b77a51ab9f099461440a04202f9"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"5f0018ba729a57e874e3c964976b643a95145b01","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            for index in indexes[::-1]:"},{"line_number":69,"context_line":"                try:"},{"line_number":70,"context_line":"                    data \u003d sorted("},{"line_number":71,"context_line":"                        data, key\u003dlambda k: (k[index] is None, k[index]),"},{"line_number":72,"context_line":"                    )"},{"line_number":73,"context_line":"                except TypeError:"},{"line_number":74,"context_line":"                    # simply log and then ignore this; sorting is best effort"}],"source_content_type":"text/x-python","patch_set":2,"id":"56c6ac97_81f598ca","line":71,"updated":"2021-01-28 17:33:57.000000000","message":"This bit of trickery (building a tuple for each value to avoid comparing against None) would benefit from a comment in the source. What you have in the commit message would be a good place to start. That much of a comment is going to make this block of code pretty long, so it might be more cleanly organized by sticking it into a function with a name like `safelySortData()`.\n\nI also suspect that if you replace the first item of the tuple with `type(k[index])` you would avoid all exceptions, but that\u0027s a guess and I haven\u0027t tested it.","commit_id":"e86f653b91c210ddcc9136d8fba61ae1c44b8e92"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fa4ecbf3be5c67c8df25091db018e19dad02e0f0","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            for index in indexes[::-1]:"},{"line_number":69,"context_line":"                try:"},{"line_number":70,"context_line":"                    data \u003d sorted("},{"line_number":71,"context_line":"                        data, key\u003dlambda k: (k[index] is None, k[index]),"},{"line_number":72,"context_line":"                    )"},{"line_number":73,"context_line":"                except TypeError:"},{"line_number":74,"context_line":"                    # simply log and then ignore this; sorting is best effort"}],"source_content_type":"text/x-python","patch_set":2,"id":"5edd6ca3_b559447d","line":71,"in_reply_to":"56c6ac97_81f598ca","updated":"2021-01-29 15:53:24.000000000","message":"\u003e This bit of trickery (building a tuple for each value to avoid comparing against None) would benefit from a comment in the source. What you have in the commit message would be a good place to start. That much of a comment is going to make this block of code pretty long, so it might be more cleanly organized by sticking it into a function with a name like `safelySortData()`.\n\nFair point. Done.\n\n\u003e I also suspect that if you replace the first item of the tuple with `type(k[index])` you would avoid all exceptions, but that\u0027s a guess and I haven\u0027t tested it.\n\nI tried. No go. \u0027type\u0027 isn\u0027t comparable 😞 I could initialize the type, which would work for the primitive types and containers, but won\u0027t for objects (like the \u0027FormattableColumn\u0027 subclasses)\n\n  \u003e\u003e\u003e x \u003d [(\u0027test\u0027, \u0027foo\u0027), (None, None), (\u0027boo\u0027, \u0027bar\u0027), (1, 2)]\n  \u003e\u003e\u003e sorted(x, key\u003dlambda k: (type(k[0]), k[0]))\n  Traceback (most recent call last):\n    File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  TypeError: \u0027\u003c\u0027 not supported between instances of \u0027type\u0027 and \u0027type\u0027","commit_id":"e86f653b91c210ddcc9136d8fba61ae1c44b8e92"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"816b4deed03cbf86f787079df3f466a208302ec9","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            for index in indexes[::-1]:"},{"line_number":69,"context_line":"                try:"},{"line_number":70,"context_line":"                    data \u003d sorted("},{"line_number":71,"context_line":"                        data, key\u003dlambda k: (k[index] is None, k[index]),"},{"line_number":72,"context_line":"                    )"},{"line_number":73,"context_line":"                except TypeError:"},{"line_number":74,"context_line":"                    # simply log and then ignore this; sorting is best effort"}],"source_content_type":"text/x-python","patch_set":2,"id":"58c12917_430abbda","line":71,"in_reply_to":"5edd6ca3_b559447d","updated":"2021-01-29 15:57:59.000000000","message":"Well, bother. We could use the name, but that feels hacky. I guess catching the TypeError is the best option.","commit_id":"e86f653b91c210ddcc9136d8fba61ae1c44b8e92"}]}