)]}'
{"keystone/common/sql/core.py":[{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"4f588727236a7f1fa36fbe9f1bf35772e2d008ba","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                # as it can contain raw SQL."},{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except (KeyError, IndexError) as ee:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_e1e2722e","line":434,"updated":"2016-07-05 23:09:24.000000000","message":"don\u0027t think try/except/else is most readable and maintainable option here (you can check the same thing if hasattr()), also, please give assign a variable for args[2] and add a comment of what is expected in each arg so we can understand better what is going on.","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":20089,"name":"Shawn Berger","email":"slberger@us.ibm.com","username":"slberger"},"change_message_id":"e525273c716436b1fcdf361d5d102a3309130ad1","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                # as it can contain raw SQL."},{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except (KeyError, IndexError) as ee:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_2fe78641","line":434,"in_reply_to":"1aa78d24_e1e2722e","updated":"2016-07-08 21:49:58.000000000","message":"I figured there was a better way to do this.  I\u0027ll work on that implementation then.\nI used args[2] because when this decorator is called on a user it has the actual user object in args[2], which allows me to grab the name of the user being checked.  I agree that is a little hard to glean from this.  I thought about making it easier by adjusting how the decorator was called, but you can\u0027t pass values from the function to the decorator except for args.  If there is a better way to do this let me know.","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":13063,"name":"WEI CHEN","email":"dave.jungler@gmail.com","username":"wei.d.chen"},"change_message_id":"fdce37a2ba45725219a98da4efceeedaa90b3a24","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except (KeyError, IndexError) as ee:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"},{"line_number":438,"context_line":"                                             details\u003d_(\u0027Duplicate Entry\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_b3243360","line":435,"range":{"start_line":435,"start_character":20,"end_line":435,"end_character":35},"updated":"2016-07-06 07:02:56.000000000","message":"It\u0027s not always args[2], the code should be more clever to get what you want, for example, it\u0027s args[1] instead of args[2].\n\npls see https://github.com/openstack/keystone/blob/master/keystone/catalog/backends/sql.py#L151\n\n\nThere are more code affected that should be updated as well.\nfor example,\nhttps://github.com/openstack/keystone/blob/master/keystone/catalog/core.py#L76","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":20089,"name":"Shawn Berger","email":"slberger@us.ibm.com","username":"slberger"},"change_message_id":"e525273c716436b1fcdf361d5d102a3309130ad1","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except (KeyError, IndexError) as ee:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"},{"line_number":438,"context_line":"                                             details\u003d_(\u0027Duplicate Entry\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_af089629","line":435,"range":{"start_line":435,"start_character":20,"end_line":435,"end_character":35},"in_reply_to":"1aa78d24_b3243360","updated":"2016-07-08 21:49:58.000000000","message":"That was part of the reason why I had the try/except/else here.  I knew that some objects don\u0027t have names and others don\u0027t have an args[2] so I just wanted to catch the case when they do, which includes users.  I created this for a specific use case, duplicate users, and I\u0027m hoping that it can be extended to all of the objects.  I will add the code as well for the other backends.  I was working with sql so that is what I changed at first.","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":16046,"name":"Fernando Diaz","email":"diazjf@us.ibm.com","username":"diazjf"},"change_message_id":"956bf1ae5ea1f2b28910655039572b28da504e63","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                # as it can contain raw SQL."},{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except Exception:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_230d88e8","line":434,"updated":"2016-08-01 15:07:37.000000000","message":"Instead of doing a try/catch I would suggest a simple if statement that checks sets `name` if it is anywhere is `args` and then passing the name to the exception if so as such:\n\n    # We traverse in case name is in args[0] .... args[n]\n    name \u003d \u0027 \u0027\n    for item in args:\n         if hasattr(args, \u0027name\u0027):\n              name \u003d getattr(item, \u0027name\u0027)\n\n     raise exception.Conflict(type\u003dconflict_type, name\u003dname,\n                                             details\u003d_(\u0027Duplicate Entry\u0027))\n\nI Rodrigo suggested, using python\u0027s hasattr() is a more readable and maintainable. This may also address Dave\u0027s comment of the value possibly being in different positions.","commit_id":"d9c6267b56fd82b79ce290ac208f998d85b11a8b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"9475b640ea30495318f5011aafe6cb655aa8b52b","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                # as it can contain raw SQL."},{"line_number":432,"context_line":"                LOG.debug(_conflict_msg, {\u0027conflict_type\u0027: conflict_type,"},{"line_number":433,"context_line":"                                          \u0027details\u0027: six.text_type(e)})"},{"line_number":434,"context_line":"                try:"},{"line_number":435,"context_line":"                    args[2][\u0027name\u0027]"},{"line_number":436,"context_line":"                except Exception:"},{"line_number":437,"context_line":"                    raise exception.Conflict(type\u003dconflict_type, name\u003d\u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_86ce9381","line":434,"in_reply_to":"bacf61ea_230d88e8","updated":"2016-11-30 20:00:34.000000000","message":"Well - this also assumes that the unique constraint is built on a name attribute, which might not always be the case. For example, mappings have unique constraints on other things, not just the name: \n\nhttps://github.com/openstack/keystone/blob/fe830ffe61912bd6e3d06c1b1eb82ca23f9314eb/keystone/common/sql/migrate_repo/versions/067_kilo.py#L265-L268","commit_id":"d9c6267b56fd82b79ce290ac208f998d85b11a8b"}],"keystone/exception.py":[{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"4f588727236a7f1fa36fbe9f1bf35772e2d008ba","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class Conflict(Error):"},{"line_number":403,"context_line":"    message_format \u003d _(\"Conflict occurred attempting to store %(type)s\""},{"line_number":404,"context_line":"                       \" %(name)s- %(details)s\")"},{"line_number":405,"context_line":"    code \u003d 409"},{"line_number":406,"context_line":"    title \u003d \u0027Conflict\u0027"},{"line_number":407,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_41d2665d","line":404,"range":{"start_line":404,"start_character":25,"end_line":404,"end_character":36},"updated":"2016-07-05 23:09:24.000000000","message":"what happens if we don\u0027t pass \"name\"? There is an example where you don\u0027t pass it in the wrapper","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":20089,"name":"Shawn Berger","email":"slberger@us.ibm.com","username":"slberger"},"change_message_id":"e525273c716436b1fcdf361d5d102a3309130ad1","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class Conflict(Error):"},{"line_number":403,"context_line":"    message_format \u003d _(\"Conflict occurred attempting to store %(type)s\""},{"line_number":404,"context_line":"                       \" %(name)s- %(details)s\")"},{"line_number":405,"context_line":"    code \u003d 409"},{"line_number":406,"context_line":"    title \u003d \u0027Conflict\u0027"},{"line_number":407,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_2fc82674","line":404,"range":{"start_line":404,"start_character":25,"end_line":404,"end_character":36},"in_reply_to":"1aa78d24_41d2665d","updated":"2016-07-08 21:49:58.000000000","message":"That will probably fail. I need to handle that","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":13063,"name":"WEI CHEN","email":"dave.jungler@gmail.com","username":"wei.d.chen"},"change_message_id":"fdce37a2ba45725219a98da4efceeedaa90b3a24","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class Conflict(Error):"},{"line_number":403,"context_line":"    message_format \u003d _(\"Conflict occurred attempting to store %(type)s\""},{"line_number":404,"context_line":"                       \" %(name)s- %(details)s\")"},{"line_number":405,"context_line":"    code \u003d 409"},{"line_number":406,"context_line":"    title \u003d \u0027Conflict\u0027"},{"line_number":407,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_d36c5f37","line":404,"range":{"start_line":404,"start_character":25,"end_line":404,"end_character":36},"in_reply_to":"1aa78d24_41d2665d","updated":"2016-07-06 07:02:56.000000000","message":"The change here can also applied to limited cases, for example, there is no \u0027name\u0027 for creating/updating region, and most update APIs doesn\u0027t need \u0027name\u0027 to be presented.","commit_id":"e7bcec68a57e1bba46b80c3eb4d26b742d15ada4"},{"author":{"_account_id":16046,"name":"Fernando Diaz","email":"diazjf@us.ibm.com","username":"diazjf"},"change_message_id":"956bf1ae5ea1f2b28910655039572b28da504e63","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    pass"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class Conflict(Error):"},{"line_number":403,"context_line":"    message_format \u003d _(\"Conflict occurred attempting to store %(type)s\""},{"line_number":404,"context_line":"                       \" %(name)s- %(details)s\")"},{"line_number":405,"context_line":"    code \u003d 409"}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_e305307a","line":402,"updated":"2016-08-01 15:07:37.000000000","message":"For examples on why tests are failing I would take a look at https://github.com/openstack/keystone/blob/master/keystone/tests/unit/identity/backends/test_base.py#L128 where exception.Conflict is called.\n\nShould we have 1 generic Conflict Error and have specific conflict error for each type of conflict? Maybe that can avoid the error of items not having the name field.","commit_id":"d9c6267b56fd82b79ce290ac208f998d85b11a8b"}]}
