)]}'
{"openstack_dashboard/views.py":[{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"cea86201ea2451f954aba59007996a9347c34f55","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        dashboard \u003d horizon.get_default_dashboard()"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    # Domain Admin, Project Admin will default to identity"},{"line_number":40,"context_line":"    if ((hasattr(user, \u0027token\u0027) and user.token.project.get(\u0027id\u0027) is None or"},{"line_number":41,"context_line":"            (user.is_superuser and user.token.project.get(\u0027id\u0027)))):"},{"line_number":42,"context_line":"        dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_d921204f","line":40,"updated":"2017-01-27 13:37:57.000000000","message":"You don\u0027t need the extra ()","commit_id":"c9fa629da3e61293a5faf23d9ddb8a6ab69934c7"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"25d40dd62748121919f7d35ba10b1d293f78adc7","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    # Domain Admin, Project Admin will default to identity"},{"line_number":40,"context_line":"    if (hasattr(user, \u0027token\u0027) and user.token.project.get(\u0027id\u0027) is None or"},{"line_number":41,"context_line":"            (user.is_superuser and user.token.project.get(\u0027id\u0027))):"},{"line_number":42,"context_line":"        dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    return dashboard.get_absolute_url()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_9a8e7177","line":41,"updated":"2017-01-30 12:27:43.000000000","message":"This will still fail on the second user.token.project.get(\u0027id\u0027)\nHave you tested this?\n\nI recommend moving the logic that gets the project id onto a separate line, to make the logic a bit more clear.\n\nI\u0027m sorry for not noticing this on my previous review.","commit_id":"acf91f94fe99f6ec4ffc97bc3bc6dad1ad30fd61"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"b0de19e5f86a1a79f142a826e0c601a93bee81b6","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        if (user.token.project.get(\u0027id\u0027) is None or"},{"line_number":33,"context_line":"            user.is_superuser):"},{"line_number":34,"context_line":"            dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)"},{"line_number":35,"context_line":"    except (base.NotRegistered, AttributeError):"},{"line_number":36,"context_line":"        pass"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    if dashboard is None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_1abdf3d7","line":35,"updated":"2017-01-31 13:35:44.000000000","message":"This will also catch and hide any AttributeError that might happen inside the horizon.get_dahboard() call, which will make it very hard to find. Can you please split this try-except in two, one for each of the errors? Ideally, catch as little as you can, for example:\n\n    try:\n        project \u003d user.token.project\n    except AttributeError:\n        project \u003d {}\n    if project.get(\u0027id\u0027) is None or user.is_superuser:\n        try:\n            dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)\n        except base.NotRegistered:\n            pass","commit_id":"6a6f283bda53efe52387e72c04a0cbd885479a3b"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"f715df896b9ff476c78414ab47778753504ad6ad","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    except AttributeError:"},{"line_number":33,"context_line":"        project \u003d {}"},{"line_number":34,"context_line":"    # Domain Admin, Project Admin will default to identity"},{"line_number":35,"context_line":"    if project.get(\u0027id\u0027, []) is None or user.is_superuser:"},{"line_number":36,"context_line":"        try:"},{"line_number":37,"context_line":"            dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)"},{"line_number":38,"context_line":"        except base.NotRegistered:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_1602f96f","line":35,"updated":"2017-02-01 16:05:07.000000000","message":"This changes the behavior. Previously, if there was an \u0027id\u0027 key with the value of None, it would be considered True. With your code, it is false.","commit_id":"c124bb20b3bae2c9bbed899918df358373d76138"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"7ecdf800eaf6104c129e5b48bb7ad349a986bfa4","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    except AttributeError:"},{"line_number":33,"context_line":"        project \u003d {}"},{"line_number":34,"context_line":"    # Domain Admin, Project Admin will default to identity"},{"line_number":35,"context_line":"    if project.get(\u0027id\u0027, []) is None or user.is_superuser:"},{"line_number":36,"context_line":"        try:"},{"line_number":37,"context_line":"            dashboard \u003d horizon.get_dashboard(\u0027identity\u0027)"},{"line_number":38,"context_line":"        except base.NotRegistered:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_3625350a","line":35,"in_reply_to":"3a461143_1602f96f","updated":"2017-02-01 16:07:10.000000000","message":"Sorry, I mean previously if there was no \"id\" key, it would be True, but now it is False.\n\nYou don\u0027t have to follow my suggestion on the previous patch literally, it was just an example of how to only catch the relevant errors.","commit_id":"c124bb20b3bae2c9bbed899918df358373d76138"}]}
