)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":23140,"name":"Jess Egler","email":"jess.egler@gmail.com","username":"jessegler"},"change_message_id":"eb27c73c551251a96de806657cf566ec9b94e3bf","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Improve limit sql backend"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch does:"},{"line_number":10,"context_line":"1. Imporve the error message as Morgan suggested before."},{"line_number":11,"context_line":"2. Add a new error type: RegisteredLimitError."},{"line_number":12,"context_line":"3. Catch the DBReferenceError in update/delete resigtered limit"},{"line_number":13,"context_line":"functions."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7f96bb07_100c8d39","line":10,"range":{"start_line":10,"start_character":3,"end_line":10,"end_character":10},"updated":"2018-01-23 20:11:33.000000000","message":"nit: Improve","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"}],"keystone/exception.py":[{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"06560e3cd94e0363dae23340aadcd61676ab3afd","unresolved":false,"context_lines":[{"line_number":471,"context_line":""},{"line_number":472,"context_line":"class ReferencedLimitExist(Forbidden):"},{"line_number":473,"context_line":"    message_format \u003d _(\"Unable to update or delete the registered limit that \""},{"line_number":474,"context_line":"                       \"have corresponding limits.\")"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"class DomainConfigNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_d7a98290","line":474,"updated":"2018-01-21 20:57:00.000000000","message":"What is the reason this would occur? Missing service or region? Can you explain more what this means to a user and how they should fix it?","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"cae12ebcbeb86ad51438df948e1803eb9b659851","unresolved":false,"context_lines":[{"line_number":471,"context_line":""},{"line_number":472,"context_line":"class ReferencedLimitExist(Forbidden):"},{"line_number":473,"context_line":"    message_format \u003d _(\"Unable to update or delete the registered limit that \""},{"line_number":474,"context_line":"                       \"have corresponding limits.\")"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"class DomainConfigNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_c95fa670","line":474,"in_reply_to":"7f96bb07_491dd6bb","updated":"2018-01-22 01:35:55.000000000","message":"Sorry. I misread, it\u0027s registered limit!, you are right.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"77487e4abdee0f2b4b8acec996f816cf39654f89","unresolved":false,"context_lines":[{"line_number":471,"context_line":""},{"line_number":472,"context_line":"class ReferencedLimitExist(Forbidden):"},{"line_number":473,"context_line":"    message_format \u003d _(\"Unable to update or delete the registered limit that \""},{"line_number":474,"context_line":"                       \"have corresponding limits.\")"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"class DomainConfigNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_491dd6bb","line":474,"in_reply_to":"7f96bb07_d70c2296","updated":"2018-01-22 01:32:30.000000000","message":"sure, a nit that there can be a lot of limits to override one registered limit. So we can not use \"limit %(id)s\" here.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"3b63947488fe44ed664aa59b388d318ce26acc5b","unresolved":false,"context_lines":[{"line_number":471,"context_line":""},{"line_number":472,"context_line":"class ReferencedLimitExist(Forbidden):"},{"line_number":473,"context_line":"    message_format \u003d _(\"Unable to update or delete the registered limit that \""},{"line_number":474,"context_line":"                       \"have corresponding limits.\")"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"class DomainConfigNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_d70c2296","line":474,"in_reply_to":"7f96bb07_d7a98290","updated":"2018-01-21 21:43:42.000000000","message":"Oh, I think I understand this better. The idea is that if a project limit already exists, the registered one shouldn\u0027t be changed?\n\nCould you change the name of the exception class to `RegisteredLimitError` and the message to \"Unable to update or delete registered limit %(id)s because there are project limits associated with it.\" ? I think you could also change this to inherit from ForbiddenNotSecurity instead of Forbidden","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"}],"keystone/limit/backends/sql.py":[{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"06560e3cd94e0363dae23340aadcd61676ab3afd","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_9793fa5c","line":149,"updated":"2018-01-21 20:57:00.000000000","message":"Why would this occur? Why does it only occur in update and delete and not create?","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"3b63947488fe44ed664aa59b388d318ce26acc5b","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_b7239605","line":149,"in_reply_to":"7f96bb07_9793fa5c","updated":"2018-01-21 21:43:42.000000000","message":"I think I understand the intention now but I\u0027m worried that this will catch foreign key problems with the region and service and not just between the registered limit and project limit.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"ad7567f510e15d8a85d3a5a6d79bb5dfe751458a","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_1a0dc036","line":149,"in_reply_to":"7f96bb07_a5bb4e0c","updated":"2018-01-22 14:06:28.000000000","message":"Yeah. I missed the different driver part. It\u0027s reasonable now. I\u0027ll add a migration script and remove it in the next ps.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"06e37e0b36bc66a25deb618bde1700acdf9f6619","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_e9b622dd","line":149,"in_reply_to":"7f96bb07_b7239605","updated":"2018-01-22 01:27:44.000000000","message":"I add this function: https://review.openstack.org/#/c/524109/37/keystone/limit/core.py@38 to ensure the referenced region and service exist. So DBReferenceError here will be only caused by related limits.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"0a094d2d14cb266c3b301f037081223d07732ee9","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_f3ccc862","line":149,"in_reply_to":"7f96bb07_e9b622dd","updated":"2018-01-22 09:21:12.000000000","message":"oh. I just realize that you mean we can\u0027t delete the service or region if there are some registered limit or limit rely on it. Right? emm, it\u0027s an interesting question. \nObvioursly, we have problem here: Now if we create a limit for a service and region. Then when we try to delete this service or region, we\u0027ll meet DBReferenceError. So should we remove the FK? If it\u0027s removed, what should we do when the related service or region is deleted but the limit still exists?\nThere a similar problem that \"user\" has a FK to domain_id, once we try to delete the domain with some users in it, keystone will raise DBReferenceError as well( Now keystone doesn\u0027t catch this error and will return 500), there is a patch to solve it now: https://review.openstack.org/#/c/506340/ , it try to delete the user automatically when deleting the domain. So can we do the same thing for service or region? i.e. delete the related limit automatically  when deleting the related service or region.\n\nSo the main problem here IMO is can we treat server/region separately with limit? My idea is to keep the FK and do some change in deleting service/region flow.(like the domain does)","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"4eefa064a5c784d257154300c5a9d5ffb86d1e59","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                        if attr !\u003d \u0027id\u0027:"},{"line_number":147,"context_line":"                            setattr(ref, attr, getattr(new_registered_limit,"},{"line_number":148,"context_line":"                                                       attr))"},{"line_number":149,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":150,"context_line":"            raise exception.ReferencedLimitExist()"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @driver_hints.truncated"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_a5bb4e0c","line":149,"in_reply_to":"7f96bb07_f3ccc862","updated":"2018-01-22 13:02:21.000000000","message":"The problem is there should never be foreign keys referencing different backends. If someone wanted to use a non-sql backend for the catalog driver, for example, then they wouldn\u0027t be able to use the limits backend because it is referencing a table that is not there in that deployment. A lot of the time this is just a theoretical problem but in the case of the catalog driver we actually have a non-sql driver in-tree (I think it doesn\u0027t really work right now but that\u0027s a different problem). In the case of the user table referencing the project table, that was a mistake that went undetected for a couple of cycles and now we just don\u0027t allow different drivers for the resource backend. It\u0027s not a pattern we should follow.\n\nI would like to see the service and region foreign keys removed from both the limits tables. We can manage deletion of resources through notifications at the manager layer. The foreign key between the limit and registered_limit table is fine since it\u0027s part of the same backend.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"06560e3cd94e0363dae23340aadcd61676ab3afd","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                for limit in limits:"},{"line_number":187,"context_line":"                    if limit.get(\u0027region_id\u0027) is None:"},{"line_number":188,"context_line":"                        self._check_unified_limit_without_region("},{"line_number":189,"context_line":"                            limit, is_registered_limit\u003dFalse)"},{"line_number":190,"context_line":"                    ref \u003d LimitModel.from_dict(limit)"},{"line_number":191,"context_line":"                    session.add(ref)"},{"line_number":192,"context_line":"                    return_ref.append(ref.to_dict())"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_579d7264","line":189,"updated":"2018-01-21 20:57:00.000000000","message":"Seems like this should have been caught by a test, is it possible to add one?","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"2131198f1dedb223d8834de303366a8a0ff7a695","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                for limit in limits:"},{"line_number":187,"context_line":"                    if limit.get(\u0027region_id\u0027) is None:"},{"line_number":188,"context_line":"                        self._check_unified_limit_without_region("},{"line_number":189,"context_line":"                            limit, is_registered_limit\u003dFalse)"},{"line_number":190,"context_line":"                    ref \u003d LimitModel.from_dict(limit)"},{"line_number":191,"context_line":"                    session.add(ref)"},{"line_number":192,"context_line":"                    return_ref.append(ref.to_dict())"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_29e3aaa6","line":189,"in_reply_to":"7f96bb07_579d7264","updated":"2018-01-22 01:38:06.000000000","message":"actually I added many tests in the follow patch. That\u0027s the reason I find this code error.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"06560e3cd94e0363dae23340aadcd61676ab3afd","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                    return_ref.append(ref.to_dict())"},{"line_number":193,"context_line":"                return return_ref"},{"line_number":194,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":195,"context_line":"            raise exception.NoLimitReference()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    @sql.handle_conflicts(conflict_type\u003d\u0027limit\u0027)"},{"line_number":198,"context_line":"    def update_limits(self, limits):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_779aee6e","line":195,"updated":"2018-01-21 20:57:00.000000000","message":"Why does this raise a different error?","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"2131198f1dedb223d8834de303366a8a0ff7a695","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                    return_ref.append(ref.to_dict())"},{"line_number":193,"context_line":"                return return_ref"},{"line_number":194,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":195,"context_line":"            raise exception.NoLimitReference()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    @sql.handle_conflicts(conflict_type\u003d\u0027limit\u0027)"},{"line_number":198,"context_line":"    def update_limits(self, limits):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_a9cfba27","line":195,"in_reply_to":"7f96bb07_779aee6e","updated":"2018-01-22 01:38:06.000000000","message":"it\u0027s for creating limits, not registered limits.","commit_id":"0adf98acda3f15497ef3098e6753eb058024de11"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"f77777289cab8753f44ec7296c78a4892d0e9c5b","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_2b1a7a7c","line":167,"updated":"2018-01-23 21:28:29.000000000","message":"What if this try/except was pulled inside the for loop? Then you\u0027d be able to pull the ID from the `registered_limit` variable that\u0027s currently failing. Or would that cause a problem with the sql transaction?","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"eb8faf8dfa5adf241c670f11b5c53e01de3c280d","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_923bb7c6","line":167,"in_reply_to":"7f96bb07_1d633db3","updated":"2018-01-24 03:48:31.000000000","message":"It sounds like what you\u0027re saying is that the exception is raised when the context manager handles the interaction with the database. If that\u0027s the case, maybe we can remove the context manager and handle the session exiting manually with session.close() and wrap that in a try/except?\n\nI believe we resort to using session.close() in other areas of keystone.","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"bfcf01f17723b51935a1785b3de811715ebcc586","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_cbcbd3a2","line":167,"in_reply_to":"7f96bb07_2b1a7a7c","updated":"2018-01-23 23:02:51.000000000","message":"++","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"3b773dfd1121a20178d498a7cb62566dd605ae80","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_1d633db3","line":167,"in_reply_to":"7f96bb07_2eecc527","updated":"2018-01-24 02:44:07.000000000","message":"The DBReferenceError error is only raised after the \"with....as session\". So we can\u0027t move try/except inside.\n\nHere is solution like:\nfor registered_limit in registered_limits:\n    try:    \n        with sql.session_for_write() as session:\n            ....\n    except:\n         raise ...registered_limi[\u0027id\u0027]\n\nBut if use this way, it means that when users try to create more than one registered limits in one request, the correct one which before the wrong one will be created. others will fail.\n\nBut my purpose here is that when users try to create more than one registered limits in one request, if one of them is wrong, all of them should not be created.\n\nAny idea?","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"a67c8e4e9982f1faca951c5ed5f8f318b75e86dd","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_ce21a1a1","line":167,"in_reply_to":"7f96bb07_8be62b16","updated":"2018-01-23 23:34:34.000000000","message":"It just wouldn\u0027t be sensible to update a registered limit that\u0027s already being used. The only thing that makes any sense to update is the default limit value, which I think would not be caught by this foreign key constraint.","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"ac5ac45d5500b8373c4c798256121db0c22f9ca2","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_11e31e36","line":167,"in_reply_to":"7f96bb07_923bb7c6","updated":"2018-01-24 09:10:21.000000000","message":"still have no perfect solution. ;(\nPlease review the new ps.\nActually only sqlite has problem. Maybe we can deal with it later.","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"82e3c45bab3e95644e033082f6caaa6a273fc4bd","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_8be62b16","line":167,"in_reply_to":"7f96bb07_cbcbd3a2","updated":"2018-01-23 23:09:58.000000000","message":"So what happens when an operator needs to update a registered limit? Do they have to delete all limits before doing so? Or do we not expect this to be a resource where updates are frequent?","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7ec616967ad4730440df919aa22dc4b06e47f966","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError:"},{"line_number":165,"context_line":"            # TODO(wxy): It\u0027s not easy to get the registered limit id from the"},{"line_number":166,"context_line":"            # DB error message, because the message\u0027s format is quite different"},{"line_number":167,"context_line":"            # from different kind of sql. Find a generic way in the future."},{"line_number":168,"context_line":"            error_message \u003d _(\"Unable to update the registered limits because \""},{"line_number":169,"context_line":"                              \"there are project limits associated with it.\")"},{"line_number":170,"context_line":"            raise exception.RegisteredLimitError(message\u003derror_message)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_2eecc527","line":167,"in_reply_to":"7f96bb07_ce21a1a1","updated":"2018-01-23 23:42:30.000000000","message":"Oh - fair enough. That makes sense.","commit_id":"2389175410bd55a4a34397d425b4d1f359090629"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"d1baa137c506e0aab132a3e07866a8b256b077f6","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        except db_exception.DBReferenceError as e:"},{"line_number":165,"context_line":"            try:"},{"line_number":166,"context_line":"                reg_id \u003d e.inner_exception.params[\u0027registered_limit_id\u0027]"},{"line_number":167,"context_line":"            except Exception:"},{"line_number":168,"context_line":"                # TODO(wxy): For sqlite, the registered_limit_id is not"},{"line_number":169,"context_line":"                # contained in the error message. We should find a way to"},{"line_number":170,"context_line":"                # handle it, maybe to improve the message in oslo.db."}],"source_content_type":"text/x-python","patch_set":6,"id":"7f96bb07_8ef620d7","line":167,"range":{"start_line":167,"start_character":19,"end_line":167,"end_character":28},"updated":"2018-01-24 15:11:30.000000000","message":"This is really generic, even though we\u0027re in a specific case at this point. Looking at the line above, this feels like it should be a KeyError.\n\nExcepting `Exception`s like this worries me because it usually makes debugging problems harder if an unexpected exception slips through it.","commit_id":"45ea075a311cd32d42bdee2188805d3a86166f74"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"d1baa137c506e0aab132a3e07866a8b256b077f6","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            except Exception:"},{"line_number":168,"context_line":"                # TODO(wxy): For sqlite, the registered_limit_id is not"},{"line_number":169,"context_line":"                # contained in the error message. We should find a way to"},{"line_number":170,"context_line":"                # handle it, maybe to improve the message in oslo.db."},{"line_number":171,"context_line":"                error_message \u003d _(\"Unable to update the registered limits \""},{"line_number":172,"context_line":"                                  \"because there are project limits \""},{"line_number":173,"context_line":"                                  \"associated with it.\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f96bb07_6ee71414","line":170,"updated":"2018-01-24 15:11:30.000000000","message":"The exception raised from oslo.db doesn\u0027t include the parameters when running on SQlite? But it does for Postgres and MySQL?","commit_id":"45ea075a311cd32d42bdee2188805d3a86166f74"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"fbe17b70d2ac09417ba178cbeb92c76791b83991","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            except Exception:"},{"line_number":168,"context_line":"                # TODO(wxy): For sqlite, the registered_limit_id is not"},{"line_number":169,"context_line":"                # contained in the error message. We should find a way to"},{"line_number":170,"context_line":"                # handle it, maybe to improve the message in oslo.db."},{"line_number":171,"context_line":"                error_message \u003d _(\"Unable to update the registered limits \""},{"line_number":172,"context_line":"                                  \"because there are project limits \""},{"line_number":173,"context_line":"                                  \"associated with it.\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f93b717_ebc86f75","line":170,"in_reply_to":"7f96bb07_6ee71414","updated":"2018-01-25 03:24:54.000000000","message":"it includes parameters, but for SQLite, \"registered_limit_id\" doesn\u0027t in the parameters.","commit_id":"45ea075a311cd32d42bdee2188805d3a86166f74"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3db4a003f48309900f2bcde07d8ed2f376c3256d","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if registered_limit.get(\u0027region_id\u0027) is None:"},{"line_number":140,"context_line":"                    self._check_unified_limit_without_region(registered_limit)"},{"line_number":141,"context_line":"                ref \u003d RegisteredLimitModel.from_dict(registered_limit)"},{"line_number":142,"context_line":"                session.add(ref)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @sql.handle_conflicts(conflict_type\u003d\u0027registered_limit\u0027)"},{"line_number":145,"context_line":"    def update_registered_limits(self, registered_limits):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f93b717_95d0d8f9","line":142,"updated":"2018-01-25 14:28:20.000000000","message":"I thought we were going to keep returning things from the driver? But I suppose that is only going to return the ones being created, not all of them.","commit_id":"0cde8da14e90afc60fc3423b8bd7ccb1e338eafc"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"f76defa81ab3810547f95c3dfe63dc308faf1861","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if registered_limit.get(\u0027region_id\u0027) is None:"},{"line_number":140,"context_line":"                    self._check_unified_limit_without_region(registered_limit)"},{"line_number":141,"context_line":"                ref \u003d RegisteredLimitModel.from_dict(registered_limit)"},{"line_number":142,"context_line":"                session.add(ref)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @sql.handle_conflicts(conflict_type\u003d\u0027registered_limit\u0027)"},{"line_number":145,"context_line":"    def update_registered_limits(self, registered_limits):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f93b717_30a6b2c3","line":142,"in_reply_to":"5f93b717_95d0d8f9","updated":"2018-01-25 14:59:00.000000000","message":"Yeah. limits is quite different. It returns all of the record. I think we should leave this logic to manager so that if others add a new backend, he doesn\u0027t need to deal with this action. So I return nothing here and call list_registered_limit at manager layer.","commit_id":"0cde8da14e90afc60fc3423b8bd7ccb1e338eafc"}]}