)]}'
{"keystone/exception.py":[{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"class InvalidLimit(Forbidden):"},{"line_number":350,"context_line":"    message_format \u003d _(\"Invalid resource limit: %(reason)s.\")"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"class NotFound(Error):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_db28c237","line":350,"updated":"2018-07-12 10:24:20.000000000","message":"Have we talked about whether this should be throwing a 400 or a 403?\n\nFrom the API guidelines (https://specs.openstack.org/openstack/api-wg/guidelines/http/response-codes.html#failure-code-clarifications):\n\n* If the request results in the OpenStack user exceeding his or her quota, the return code should be 403 Forbidden. Do not use 413 Request Entity Too Large.\n* For badly formatted requests, the return code should be 400 Bad Request. Do not use 422 Unprocessable Entity.\n-   If the API limits the length of a property that is a collection, the return code should be 400 Bad Request when the request exceeds the length limit. The client should adjust requests to achieve success, and shouldn’t expect to repeat the request and have it work. Do not use 403 Forbidden for this case, because this is different than exceeding quota – for a subsequent request to succeed when quotas are exceeded the server environment must change.\n\nSince this is about setting the limit, not a request that would increase usage of quota, the first point doesn\u0027t apply, even though it\u0027s tempting to think it should.\n\nI could see this falling under the scope of a badly formatted request. The user needs to adjust their request (make the number smaller) in order for it to succeed. On the other hand, you could look at it from the other side and say that the server environment (the parent quota) needs to change in order for the request to succeed. I think I lean toward using the 403 which is what you have here, I just want to check that this has been thought out and discussed.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"e98840225f869f677eaa9ab163fcdb33f27fe1a8","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"class InvalidLimit(Forbidden):"},{"line_number":350,"context_line":"    message_format \u003d _(\"Invalid resource limit: %(reason)s.\")"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"class NotFound(Error):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_2c93a3a6","line":350,"in_reply_to":"5f7c97a3_db28c237","updated":"2018-07-13 06:32:49.000000000","message":"I think your thought is really cool. ++ for it.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"}],"keystone/limit/backends/sql.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                                                hint_copy)"},{"line_number":328,"context_line":"                if project_filter:"},{"line_number":329,"context_line":"                    limits \u003d limits.filter("},{"line_number":330,"context_line":"                        LimitModel.project_id \u003d\u003d project_filter[\u0027value\u0027])"},{"line_number":331,"context_line":"                new_format_data \u003d [s.to_dict() for s in limits]"},{"line_number":332,"context_line":"            return old_format_data + new_format_data"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_9757c8c9","line":330,"updated":"2018-07-09 20:26:47.000000000","message":"This seems like something that should be it\u0027s own change? Is the required for the strict_two_level model somehow?","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                                                hint_copy)"},{"line_number":328,"context_line":"                if project_filter:"},{"line_number":329,"context_line":"                    limits \u003d limits.filter("},{"line_number":330,"context_line":"                        LimitModel.project_id \u003d\u003d project_filter[\u0027value\u0027])"},{"line_number":331,"context_line":"                new_format_data \u003d [s.to_dict() for s in limits]"},{"line_number":332,"context_line":"            return old_format_data + new_format_data"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_faf7fccf","line":330,"in_reply_to":"5f7c97a3_9757c8c9","updated":"2018-07-10 03:38:31.000000000","message":"Sorry, it\u0027s a new bug when refactor the limit db schema, I can separate it into a new patch.","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"}],"keystone/limit/core.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def create_limits(self, limits):"},{"line_number":101,"context_line":"        for limit in limits:"},{"line_number":102,"context_line":"            self._assert_resource_exist(limit, \u0027limit\u0027)"},{"line_number":103,"context_line":"        self._check_hierarchy_creating(limits)"},{"line_number":104,"context_line":"        return self.driver.create_limits(limits)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _check_hierarchy_updating(self, limit_id, limit):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d706a0cd","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":46},"updated":"2018-07-09 20:26:47.000000000","message":"nit: Is there a reason to not call the self.enforcement_model object directly from here (just wondering if this is an extra hop that might not be needed)?","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def create_limits(self, limits):"},{"line_number":101,"context_line":"        for limit in limits:"},{"line_number":102,"context_line":"            self._assert_resource_exist(limit, \u0027limit\u0027)"},{"line_number":103,"context_line":"        self._check_hierarchy_creating(limits)"},{"line_number":104,"context_line":"        return self.driver.create_limits(limits)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _check_hierarchy_updating(self, limit_id, limit):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_ba2f443c","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":46},"in_reply_to":"5f7c97a3_d706a0cd","updated":"2018-07-10 03:38:31.000000000","message":"++, will remove it","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"}],"keystone/limit/models/base.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        raise ImportError(msg % {\u0027name\u0027: driver_name, \u0027namespace\u0027: namespace})"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class ModelBase(object):"},{"line_number":35,"context_line":"    \"\"\"Interface for a limit model driver.\"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    NAME \u003d None"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_37e5940a","line":34,"range":{"start_line":34,"start_character":6,"end_line":34,"end_character":15},"updated":"2018-07-09 20:26:47.000000000","message":"We should make sure this is an abstract base class [0]. That will just ensure any changes to the interface will require the implementations to update.\n\n[0] https://git.openstack.org/cgit/openstack/keystone/tree/keystone/token/providers/base.py#n22","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        raise ImportError(msg % {\u0027name\u0027: driver_name, \u0027namespace\u0027: namespace})"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class ModelBase(object):"},{"line_number":35,"context_line":"    \"\"\"Interface for a limit model driver.\"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    NAME \u003d None"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7ada0c4a","line":34,"range":{"start_line":34,"start_character":6,"end_line":34,"end_character":15},"in_reply_to":"5f7c97a3_37e5940a","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            doesn\u0027t satisfy the limit model."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        \"\"\""},{"line_number":53,"context_line":"        raise exception.NotImplemented()  # pragma: no cover"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def check_limit_hierarchy_updating(self, limit_id, limit):"},{"line_number":56,"context_line":"        \"\"\"Check the new updating limit whether satisfy the limit model."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_57c93076","line":53,"range":{"start_line":53,"start_character":14,"end_line":53,"end_character":38},"updated":"2018-07-09 20:26:47.000000000","message":"Morgan stumbled across a subtlety here that we could implement:\n\n  raise NotImplementedError()\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-keystone/%23openstack-keystone.2018-07-09.log.html#t2018-07-09T14:17:19","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            doesn\u0027t satisfy the limit model."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        \"\"\""},{"line_number":53,"context_line":"        raise exception.NotImplemented()  # pragma: no cover"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def check_limit_hierarchy_updating(self, limit_id, limit):"},{"line_number":56,"context_line":"        \"\"\"Check the new updating limit whether satisfy the limit model."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_5ad71023","line":53,"range":{"start_line":53,"start_character":14,"end_line":53,"end_character":38},"in_reply_to":"5f7c97a3_57c93076","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"bf202f289f2eeebfb26d39a3ec82c18a0ebd7050","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        :param limits: A list of the limit objects need to be check."},{"line_number":48,"context_line":"        :type limits: A list of the limits. Each limit is a dict that contains"},{"line_number":49,"context_line":"            \"resource_limit\", \"resource_name\", \"project_id\", \"service_id\" and"},{"line_number":50,"context_line":"            optional \"region_id\", \"description\"."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        :raises keystone.exception.InvalidLimit: If any of the input limits"},{"line_number":53,"context_line":"            doesn\u0027t satisfy the limit model."}],"source_content_type":"text/x-python","patch_set":25,"id":"5f7c97a3_50badfbf","line":50,"updated":"2018-07-17 20:02:03.000000000","message":"This is actually a limit reference, so it  will include the ID, too i think.\n\nNot something that\u0027s super important to include I don\u0027t think because the business logic of the enforcement models will need the attributes described here.","commit_id":"4b4835a01c662ddb11e9f1aace47a02c6e337978"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"35d68e73ef9b5e3eec36ba1bd00e141f25f4ce98","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        :param limits: A list of the limit objects need to be check."},{"line_number":48,"context_line":"        :type limits: A list of the limits. Each limit is a dict that contains"},{"line_number":49,"context_line":"            \"resource_limit\", \"resource_name\", \"project_id\", \"service_id\" and"},{"line_number":50,"context_line":"            optional \"region_id\", \"description\"."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        :raises keystone.exception.InvalidLimit: If any of the input limits"},{"line_number":53,"context_line":"            doesn\u0027t satisfy the limit model."}],"source_content_type":"text/x-python","patch_set":25,"id":"5f7c97a3_38617311","line":50,"in_reply_to":"5f7c97a3_50badfbf","updated":"2018-07-26 16:35:45.000000000","message":"Done in https://review.openstack.org/#/c/585379/","commit_id":"4b4835a01c662ddb11e9f1aace47a02c6e337978"}],"keystone/limit/models/flat.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# TODO(lbragstad): This should inherit from an abstract interface so that we"},{"line_number":15,"context_line":"# ensure all models implement the same things."},{"line_number":16,"context_line":"class Model(object):"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7751cc0c","side":"PARENT","line":14,"range":{"start_line":14,"start_character":47,"end_line":14,"end_character":65},"updated":"2018-07-09 20:26:47.000000000","message":"Looks like we still need to do this bit. We\u0027re inheriting from another class, but it\u0027s not defined as an abstract base class. If another method is added to the base class, we won\u0027t be required to enforce it here.\n\nWe should update the base class to address that bit.","commit_id":"e8ca7846ffa1d5bdfd5f16a4d3b2fbe3033a5e70"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# TODO(lbragstad): This should inherit from an abstract interface so that we"},{"line_number":15,"context_line":"# ensure all models implement the same things."},{"line_number":16,"context_line":"class Model(object):"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_fae5bc07","side":"PARENT","line":14,"range":{"start_line":14,"start_character":47,"end_line":14,"end_character":65},"in_reply_to":"5f7c97a3_7751cc0c","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"e8ca7846ffa1d5bdfd5f16a4d3b2fbe3033a5e70"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":13,"context_line":"from keystone.limit.models import base"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"class Model(base.ModelBase):"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    NAME \u003d \u0027flat\u0027"},{"line_number":19,"context_line":"    DESCRIPTION \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_776a2cb5","line":16,"range":{"start_line":16,"start_character":6,"end_line":16,"end_character":11},"updated":"2018-07-09 20:26:47.000000000","message":"nit: I wouldn\u0027t be opposed to a more specific name for this model (e.g. FlatEnforcementModel, FlatModel, or even just Flat).","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":13,"context_line":"from keystone.limit.models import base"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"class Model(base.ModelBase):"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    NAME \u003d \u0027flat\u0027"},{"line_number":19,"context_line":"    DESCRIPTION \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_9aba88de","line":16,"range":{"start_line":16,"start_character":6,"end_line":16,"end_character":11},"in_reply_to":"5f7c97a3_776a2cb5","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":20,"context_line":"        \u0027Limit enforcement and validation does not take project hierarchy \u0027"},{"line_number":21,"context_line":"        \u0027into consideration.\u0027"},{"line_number":22,"context_line":"    )"},{"line_number":23,"context_line":"    MAX_PROJECT_TREE_DEPTH \u003d None"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def check_limit_hierarchy_creating(self, limits):"},{"line_number":26,"context_line":"        # Flat limit model is not hierarchical, so don\u0027t need to check the"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_57913099","line":23,"range":{"start_line":23,"start_character":4,"end_line":23,"end_character":33},"updated":"2018-07-09 20:26:47.000000000","message":"Do we always expect this to be something that is defined for every model? Or is this something that is better kept in just the strict_two_level model?","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":20,"context_line":"        \u0027Limit enforcement and validation does not take project hierarchy \u0027"},{"line_number":21,"context_line":"        \u0027into consideration.\u0027"},{"line_number":22,"context_line":"    )"},{"line_number":23,"context_line":"    MAX_PROJECT_TREE_DEPTH \u003d None"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def check_limit_hierarchy_creating(self, limits):"},{"line_number":26,"context_line":"        # Flat limit model is not hierarchical, so don\u0027t need to check the"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_3a8ed431","line":23,"range":{"start_line":23,"start_character":4,"end_line":23,"end_character":33},"in_reply_to":"5f7c97a3_57913099","updated":"2018-07-10 03:38:31.000000000","message":"lol, I saw your comment here: https://review.openstack.org/#/c/580258","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"}],"keystone/limit/models/strict_two_level.py":[{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"f1d65937b01ab6b8972f4c88a21fbf650d739acb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"5f7c97a3_97b05ade","updated":"2018-07-05 23:52:14.000000000","message":"[NIT] this could have been implemented as a separate commit","commit_id":"64d82cccab583f103bec9cb87fbc6a3d0185a175"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":22,"context_line":"PROVIDERS \u003d provider_api.ProviderAPIs"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class Model(base.ModelBase):"},{"line_number":26,"context_line":"    NAME \u003d \u0027strict_two_level\u0027"},{"line_number":27,"context_line":"    DESCRIPTION \u003d ("},{"line_number":28,"context_line":"        \u0027This model requires project hierarchy never exceeds a depth of two\u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_f75ddc15","line":25,"updated":"2018-07-09 20:26:47.000000000","message":"Same comments here as in the flat model.","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":22,"context_line":"PROVIDERS \u003d provider_api.ProviderAPIs"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class Model(base.ModelBase):"},{"line_number":26,"context_line":"    NAME \u003d \u0027strict_two_level\u0027"},{"line_number":27,"context_line":"    DESCRIPTION \u003d ("},{"line_number":28,"context_line":"        \u0027This model requires project hierarchy never exceeds a depth of two\u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_da980077","line":25,"in_reply_to":"5f7c97a3_f75ddc15","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":25,"context_line":"class Model(base.ModelBase):"},{"line_number":26,"context_line":"    NAME \u003d \u0027strict_two_level\u0027"},{"line_number":27,"context_line":"    DESCRIPTION \u003d ("},{"line_number":28,"context_line":"        \u0027This model requires project hierarchy never exceeds a depth of two\u0027"},{"line_number":29,"context_line":"    )"},{"line_number":30,"context_line":"    MAX_PROJECT_TREE_DEPTH \u003d 2"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_37ad94e9","line":28,"updated":"2018-07-09 20:26:47.000000000","message":"There are other constraints about this model (e.g. the limit of the child can\u0027t be greater than the limit of the parent).\n\nI\u0027m wondering how detailed we should get with it though.","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":25,"context_line":"class Model(base.ModelBase):"},{"line_number":26,"context_line":"    NAME \u003d \u0027strict_two_level\u0027"},{"line_number":27,"context_line":"    DESCRIPTION \u003d ("},{"line_number":28,"context_line":"        \u0027This model requires project hierarchy never exceeds a depth of two\u0027"},{"line_number":29,"context_line":"    )"},{"line_number":30,"context_line":"    MAX_PROJECT_TREE_DEPTH \u003d 2"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_5a7e301b","line":28,"in_reply_to":"5f7c97a3_37ad94e9","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    DESCRIPTION \u003d ("},{"line_number":28,"context_line":"        \u0027This model requires project hierarchy never exceeds a depth of two\u0027"},{"line_number":29,"context_line":"    )"},{"line_number":30,"context_line":"    MAX_PROJECT_TREE_DEPTH \u003d 2"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _get_specified_limit_value(self, project_id, resource_name, service_id,"},{"line_number":33,"context_line":"                                   region_id, is_parent\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_57d61055","line":30,"updated":"2018-07-09 20:26:47.000000000","message":"Same comment here as in the previous flat model. I wonder if this should just be something we keep internally to this module. Do we expect the logic outside of this class to call or rely on this attribute?\n\nIf not, then maybe we should keep it here until we have a good reason to expose it as part of the interface or require all enforcement models to define it.","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":54,"context_line":"            limit_value \u003d limits[0][\u0027default_limit\u0027] if limits else None"},{"line_number":55,"context_line":"        return limit_value"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def _check_limit(self, project_id, resource_name, resource_limit,"},{"line_number":58,"context_line":"                     service_id, region_id, parent_id):"},{"line_number":59,"context_line":"        \"\"\"Check the specified limit value satisfies the related project tree."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b74a0480","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":20},"updated":"2018-07-09 20:26:47.000000000","message":"Per the comment below, I wonder if we should just make this the interface instead of requiring enforcement models to accept data structures specific to the unified limit API (even though they are tightly coupled).","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                raise exception.InvalidLimit("},{"line_number":80,"context_line":"                    reason\u003d\"Limit is smaller than child\")"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def check_limit_hierarchy_creating(self, limits):"},{"line_number":83,"context_line":"        \"\"\"Check the input limits satisfy the related project tree or not."},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        1. Ensure the input is legal."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_976a68e9","line":82,"range":{"start_line":82,"start_character":8,"end_line":82,"end_character":38},"updated":"2018-07-09 20:26:47.000000000","message":"This method and the next method seem to really just wrap the really model logic in _check_limit().\n\nI wonder if it wouldn\u0027t be more clear to just define a single method as the interface called check_limit() instead of two separate methods that require a private method to reduce duplication. Then the logic in limit/core.py builds standardizes whatever is passed into check_limit() regardless of create or update requests.","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                raise exception.InvalidLimit("},{"line_number":80,"context_line":"                    reason\u003d\"Limit is smaller than child\")"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def check_limit_hierarchy_creating(self, limits):"},{"line_number":83,"context_line":"        \"\"\"Check the input limits satisfy the related project tree or not."},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        1. Ensure the input is legal."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_ba15049d","line":82,"range":{"start_line":82,"start_character":8,"end_line":82,"end_character":38},"in_reply_to":"5f7c97a3_976a68e9","updated":"2018-07-10 03:38:31.000000000","message":"the input parameters for create and update request is different. create request input is a list for limits, the update request input is a single limit and a specified limit_id, so I separate them and let them both use _check_limit for hierarchy check.\n\nThey can be merged into one of cause,but I\u0027m not sure how to make the common one more generic for input parameters. Any suggestion?","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"dc2c0778cd13d9545bd15422606332acce11747d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            hints.add_filter(\u0027resource_name\u0027, resource_name)"},{"line_number":52,"context_line":"            hints.add_filter(\u0027region_id\u0027, region_id)"},{"line_number":53,"context_line":"            limits \u003d PROVIDERS.unified_limit_api.list_registered_limits(hints)"},{"line_number":54,"context_line":"            limit_value \u003d limits[0][\u0027default_limit\u0027] if limits else None"},{"line_number":55,"context_line":"        return limit_value"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def _check_limit(self, project_id, resource_name, resource_limit,"}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_3bf46488","line":54,"range":{"start_line":54,"start_character":68,"end_line":54,"end_character":72},"updated":"2018-07-11 21:52:45.000000000","message":"Shouldn\u0027t this be an error? If we can\u0027t resolve a sets of values for a limit then we have a bigger problem don\u0027t we?\n\nFor example, would it make sense to set a limit on a project if the limit isn\u0027t registered? We protect against that in the unified limit API?","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            hints.add_filter(\u0027resource_name\u0027, resource_name)"},{"line_number":52,"context_line":"            hints.add_filter(\u0027region_id\u0027, region_id)"},{"line_number":53,"context_line":"            limits \u003d PROVIDERS.unified_limit_api.list_registered_limits(hints)"},{"line_number":54,"context_line":"            limit_value \u003d limits[0][\u0027default_limit\u0027] if limits else None"},{"line_number":55,"context_line":"        return limit_value"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def _check_limit(self, project_id, resource_name, resource_limit,"}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_f1891f1e","line":54,"range":{"start_line":54,"start_character":68,"end_line":54,"end_character":72},"in_reply_to":"5f7c97a3_3bf46488","updated":"2018-07-12 11:50:20.000000000","message":"we can\u0027t set a limit on a project if the limit isn\u0027t registered, but this check is after the enforcement check. So the case that both project limit and registered limit are not exist in the backend can happen\n\nSee my comment below, so if we move the reference check before registered limit, then we don\u0027t need the None here.\n\nI agree to move change the check order. :)","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"dc2c0778cd13d9545bd15422606332acce11747d","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if parent_id:"},{"line_number":66,"context_line":"            parent_limit_value \u003d self._get_specified_limit_value("},{"line_number":67,"context_line":"                parent_id, resource_name, service_id, region_id)"},{"line_number":68,"context_line":"            if parent_limit_value and resource_limit \u003e parent_limit_value:"},{"line_number":69,"context_line":"                raise exception.InvalidLimit("},{"line_number":70,"context_line":"                    reason\u003d\"Limit is bigger than parent\u0027s\")"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_1bd2c8fe","line":68,"range":{"start_line":68,"start_character":15,"end_line":68,"end_character":33},"updated":"2018-07-11 21:52:45.000000000","message":"This should never be None should it? It should either be a reference to a limit or it should be a registered limit I think.","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if parent_id:"},{"line_number":66,"context_line":"            parent_limit_value \u003d self._get_specified_limit_value("},{"line_number":67,"context_line":"                parent_id, resource_name, service_id, region_id)"},{"line_number":68,"context_line":"            if parent_limit_value and resource_limit \u003e parent_limit_value:"},{"line_number":69,"context_line":"                raise exception.InvalidLimit("},{"line_number":70,"context_line":"                    reason\u003d\"Limit is bigger than parent\u0027s\")"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_51133302","line":68,"range":{"start_line":68,"start_character":15,"end_line":68,"end_character":33},"in_reply_to":"5f7c97a3_1bd2c8fe","updated":"2018-07-12 11:50:20.000000000","message":"If we move the unique and reference check before check_limit, yes. But now if there is no both project limit and  referenced limit, the parent_limit_value here can be None","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if parent_id:"},{"line_number":66,"context_line":"            parent_limit_value \u003d self._get_specified_limit_value("},{"line_number":67,"context_line":"                parent_id, resource_name, service_id, region_id)"},{"line_number":68,"context_line":"            if parent_limit_value and resource_limit \u003e parent_limit_value:"},{"line_number":69,"context_line":"                raise exception.InvalidLimit("},{"line_number":70,"context_line":"                    reason\u003d\"Limit is bigger than parent\u0027s\")"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_f8a024a7","line":68,"range":{"start_line":68,"start_character":38,"end_line":68,"end_character":73},"updated":"2018-07-12 10:24:20.000000000","message":"If the parent\u0027s limit is unset, and _get_specified_limit_value returns the registered limit, does that mean the child\u0027s limit can never exceed the registered limit unless the parent\u0027s limit is raised?\n\nI guess that makes sense, and if you didn\u0027t want that you would use the flat model, just trying to make sure I follow.\n\nIs it possible to say the registered limit is infinite, and allow child project limits to be unrestricted by their parents? We\u0027d usually indicate that with a -1 but that wouldn\u0027t work with this arithmetic logic.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"869d0feaff733c079c7f3d3e162202502270066e","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if parent_id:"},{"line_number":66,"context_line":"            parent_limit_value \u003d self._get_specified_limit_value("},{"line_number":67,"context_line":"                parent_id, resource_name, service_id, region_id)"},{"line_number":68,"context_line":"            if parent_limit_value and resource_limit \u003e parent_limit_value:"},{"line_number":69,"context_line":"                raise exception.InvalidLimit("},{"line_number":70,"context_line":"                    reason\u003d\"Limit is bigger than parent\u0027s\")"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_6731cc0a","line":68,"range":{"start_line":68,"start_character":38,"end_line":68,"end_character":73},"in_reply_to":"5f7c97a3_91bd4b05","updated":"2018-07-12 14:01:57.000000000","message":"Would not registering the limit solve the case for the infinite limit?","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if parent_id:"},{"line_number":66,"context_line":"            parent_limit_value \u003d self._get_specified_limit_value("},{"line_number":67,"context_line":"                parent_id, resource_name, service_id, region_id)"},{"line_number":68,"context_line":"            if parent_limit_value and resource_limit \u003e parent_limit_value:"},{"line_number":69,"context_line":"                raise exception.InvalidLimit("},{"line_number":70,"context_line":"                    reason\u003d\"Limit is bigger than parent\u0027s\")"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_91bd4b05","line":68,"range":{"start_line":68,"start_character":38,"end_line":68,"end_character":73},"in_reply_to":"5f7c97a3_f8a024a7","updated":"2018-07-12 11:50:20.000000000","message":"yeah, you are correct.\nFor strict two level model, the child can never exceed parent,\nfor the case -1 means infinite, we can consider it later or a new enforcement model is well to introduce. In my mind, we should not support only one hierarchical limit model. I\u0027d like to introduce more to give users a choice base on Sean and John\u0027s backlog in the future.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            try:"},{"line_number":96,"context_line":"                parent_project \u003d PROVIDERS.resource_api.list_project_parents("},{"line_number":97,"context_line":"                    project_id)[0]"},{"line_number":98,"context_line":"                if not parent_project[\u0027is_domain\u0027]:"},{"line_number":99,"context_line":"                    parent_id \u003d parent_project[\u0027id\u0027]"},{"line_number":100,"context_line":"                    parent_limit \u003d list(filter("},{"line_number":101,"context_line":"                        lambda x: x[\u0027project_id\u0027] \u003d\u003d parent_id, limits))"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_18f88048","line":98,"range":{"start_line":98,"start_character":16,"end_line":98,"end_character":51},"updated":"2018-07-12 10:24:20.000000000","message":"Why does it matter if it\u0027s a domain?","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            try:"},{"line_number":96,"context_line":"                parent_project \u003d PROVIDERS.resource_api.list_project_parents("},{"line_number":97,"context_line":"                    project_id)[0]"},{"line_number":98,"context_line":"                if not parent_project[\u0027is_domain\u0027]:"},{"line_number":99,"context_line":"                    parent_id \u003d parent_project[\u0027id\u0027]"},{"line_number":100,"context_line":"                    parent_limit \u003d list(filter("},{"line_number":101,"context_line":"                        lambda x: x[\u0027project_id\u0027] \u003d\u003d parent_id, limits))"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_91d46b4d","line":98,"range":{"start_line":98,"start_character":16,"end_line":98,"end_character":51},"in_reply_to":"5f7c97a3_18f88048","updated":"2018-07-12 11:50:20.000000000","message":"we don\u0027t support set domain quota for now. So if parent is a domain, we can\u0027t compare the project\u0027s limit with its domain parent.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                    parent_id \u003d parent_project[\u0027id\u0027]"},{"line_number":100,"context_line":"                    parent_limit \u003d list(filter("},{"line_number":101,"context_line":"                        lambda x: x[\u0027project_id\u0027] \u003d\u003d parent_id, limits))"},{"line_number":102,"context_line":"                    if parent_limit:"},{"line_number":103,"context_line":"                        if resource_limit \u003e parent_limit[0][\u0027resource_limit\u0027]:"},{"line_number":104,"context_line":"                            raise exception.InvalidLimit("},{"line_number":105,"context_line":"                                reason\u003d\u0027Invalid Input\u0027)"},{"line_number":106,"context_line":"                        # The limit\u0027s parent is in request body, no need to"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_786e54ff","line":103,"range":{"start_line":102,"start_character":20,"end_line":103,"end_character":78},"updated":"2018-07-12 10:24:20.000000000","message":"Isn\u0027t this what _check_limit is supposed to be doing?","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"869d0feaff733c079c7f3d3e162202502270066e","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                    parent_id \u003d parent_project[\u0027id\u0027]"},{"line_number":100,"context_line":"                    parent_limit \u003d list(filter("},{"line_number":101,"context_line":"                        lambda x: x[\u0027project_id\u0027] \u003d\u003d parent_id, limits))"},{"line_number":102,"context_line":"                    if parent_limit:"},{"line_number":103,"context_line":"                        if resource_limit \u003e parent_limit[0][\u0027resource_limit\u0027]:"},{"line_number":104,"context_line":"                            raise exception.InvalidLimit("},{"line_number":105,"context_line":"                                reason\u003d\u0027Invalid Input\u0027)"},{"line_number":106,"context_line":"                        # The limit\u0027s parent is in request body, no need to"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_6aff5100","line":103,"range":{"start_line":102,"start_character":20,"end_line":103,"end_character":78},"in_reply_to":"5f7c97a3_51b01341","updated":"2018-07-12 14:01:57.000000000","message":"Yeah - I missed this detail when I was playing with this locally yesterday [0].\n\nThe ability to create limits in batches kinda throw a wrench into this since another project in the request could be the parent of the project we\u0027re currently processing.\n\n[0] https://review.openstack.org/#/c/581894/","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                    parent_id \u003d parent_project[\u0027id\u0027]"},{"line_number":100,"context_line":"                    parent_limit \u003d list(filter("},{"line_number":101,"context_line":"                        lambda x: x[\u0027project_id\u0027] \u003d\u003d parent_id, limits))"},{"line_number":102,"context_line":"                    if parent_limit:"},{"line_number":103,"context_line":"                        if resource_limit \u003e parent_limit[0][\u0027resource_limit\u0027]:"},{"line_number":104,"context_line":"                            raise exception.InvalidLimit("},{"line_number":105,"context_line":"                                reason\u003d\u0027Invalid Input\u0027)"},{"line_number":106,"context_line":"                        # The limit\u0027s parent is in request body, no need to"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_51b01341","line":103,"range":{"start_line":102,"start_character":20,"end_line":103,"end_character":78},"in_reply_to":"5f7c97a3_786e54ff","updated":"2018-07-12 11:50:20.000000000","message":"_check_limit is using for compare the input limit with the backend\u0027s limit, here is using for compare the limits in the input limit list, since we support creating limits in batch. \n\nWe should make sure the input limit tree is legal as well.\nfor example: suppose the registered limit is 10,\nwe now create three limits in one request like:\n    A,13\n   /   \\\nB,12    C,11\n\nif we just _check_limit, B will find that A has no limit in the backend and the registered limit is 10, then it\u0027ll raise error because 13 \u003e 10, so as C. But actually A will be set to 13 in this request, this request should be passed.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                    if parent_limit:"},{"line_number":103,"context_line":"                        if resource_limit \u003e parent_limit[0][\u0027resource_limit\u0027]:"},{"line_number":104,"context_line":"                            raise exception.InvalidLimit("},{"line_number":105,"context_line":"                                reason\u003d\u0027Invalid Input\u0027)"},{"line_number":106,"context_line":"                        # The limit\u0027s parent is in request body, no need to"},{"line_number":107,"context_line":"                        # check the backend any more."},{"line_number":108,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_78a97423","line":105,"range":{"start_line":105,"start_character":40,"end_line":105,"end_character":53},"updated":"2018-07-12 10:24:20.000000000","message":"If this is going to be immediately caught on line 114 and the reason is not used there then I guess we don\u0027t really need the reason string to be set here.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                    if parent_limit:"},{"line_number":103,"context_line":"                        if resource_limit \u003e parent_limit[0][\u0027resource_limit\u0027]:"},{"line_number":104,"context_line":"                            raise exception.InvalidLimit("},{"line_number":105,"context_line":"                                reason\u003d\u0027Invalid Input\u0027)"},{"line_number":106,"context_line":"                        # The limit\u0027s parent is in request body, no need to"},{"line_number":107,"context_line":"                        # check the backend any more."},{"line_number":108,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_96a9c126","line":105,"range":{"start_line":105,"start_character":40,"end_line":105,"end_character":53},"in_reply_to":"5f7c97a3_78a97423","updated":"2018-07-12 11:50:20.000000000","message":"sure.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"ed15a4062bcbf126a86984f17fbd5d62385afa41","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                    error \u003d (\"The updating value resource_limit: \""},{"line_number":117,"context_line":"                             \"%(resource_limit)s doesn\u0027t satisfy current \""},{"line_number":118,"context_line":"                             \"hierarchy model.\") % {"},{"line_number":119,"context_line":"                        \u0027resource_limit\u0027: resource_limit}"},{"line_number":120,"context_line":"                elif region_id:"},{"line_number":121,"context_line":"                    error \u003d (\"The input (project_id: %(project_id)s, \""},{"line_number":122,"context_line":"                             \"resource_name: %(resource_name)s, \""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f7c97a3_83ccd862","line":119,"updated":"2018-07-16 14:42:34.000000000","message":"It seems like now the only difference between create and update is the different error message, so maybe it would be simpler to have something generic enough for both cases instead of having a flag to disambiguate them. In fact I think the error message below would work just fine.","commit_id":"742a1aba4c1531ee5666afd975819a8c7f7b8a27"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"ed15a4062bcbf126a86984f17fbd5d62385afa41","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                             \"hierarchy model.\") % {"},{"line_number":119,"context_line":"                        \u0027resource_limit\u0027: resource_limit}"},{"line_number":120,"context_line":"                elif region_id:"},{"line_number":121,"context_line":"                    error \u003d (\"The input (project_id: %(project_id)s, \""},{"line_number":122,"context_line":"                             \"resource_name: %(resource_name)s, \""},{"line_number":123,"context_line":"                             \"resource_limit: %(resource_limit)s, \""},{"line_number":124,"context_line":"                             \"service_id: %(service_id)s, \""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f7c97a3_43efa0b0","line":121,"range":{"start_line":121,"start_character":34,"end_line":121,"end_character":39},"updated":"2018-07-16 14:42:34.000000000","message":"Maybe say \"The resource limit ...\" instead of \"The input ...\" to be clear what kind of input is invalid.","commit_id":"742a1aba4c1531ee5666afd975819a8c7f7b8a27"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"ed15a4062bcbf126a86984f17fbd5d62385afa41","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                        \u0027service_id\u0027: service_id,"},{"line_number":131,"context_line":"                        \u0027region_id\u0027: region_id"},{"line_number":132,"context_line":"                    }"},{"line_number":133,"context_line":"                else:"},{"line_number":134,"context_line":"                    error \u003d (\"The input (project_id: %(project_id)s, \""},{"line_number":135,"context_line":"                             \"resource_name: %(resource_name)s, \""},{"line_number":136,"context_line":"                             \"resource_limit: %(resource_limit)s, \""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f7c97a3_c3fbb078","line":133,"range":{"start_line":133,"start_character":16,"end_line":133,"end_character":20},"updated":"2018-07-16 14:42:34.000000000","message":"I think it\u0027s fine to say region_id: None instead of having a whole other string for an error message with no region.","commit_id":"742a1aba4c1531ee5666afd975819a8c7f7b8a27"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"761cb415f40e882eed0ed7ae85f154c4a7bd35f1","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                self._check_limit(project_id, resource_name, resource_limit,"},{"line_number":113,"context_line":"                                  service_id, region_id, parent_id)"},{"line_number":114,"context_line":"            except exception.InvalidLimit:"},{"line_number":115,"context_line":"                error \u003d (\"The resource limit (project_id: %(project_id)s, \""},{"line_number":116,"context_line":"                         \"resource_name: %(resource_name)s, \""},{"line_number":117,"context_line":"                         \"resource_limit: %(resource_limit)s, \""},{"line_number":118,"context_line":"                         \"service_id: %(service_id)s, \""}],"source_content_type":"text/x-python","patch_set":25,"id":"5f7c97a3_b091f355","line":115,"updated":"2018-07-17 19:57:36.000000000","message":"++ thanks for generalizing this","commit_id":"4b4835a01c662ddb11e9f1aace47a02c6e337978"}],"keystone/tests/unit/test_limits.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"dc2c0778cd13d9545bd15422606332acce11747d","unresolved":false,"context_lines":[{"line_number":780,"context_line":"        #   A        D"},{"line_number":781,"context_line":"        #  / \\      / \\"},{"line_number":782,"context_line":"        # B   C    E   F"},{"line_number":783,"context_line":"        project_ref \u003d {\u0027project\u0027: {\u0027name\u0027: \u0027project_A\u0027, \u0027enabled\u0027: True}}"},{"line_number":784,"context_line":"        response \u003d self.post(\u0027/projects\u0027, body\u003dproject_ref)"},{"line_number":785,"context_line":"        self.project_A \u003d response.json_body[\u0027project\u0027]"},{"line_number":786,"context_line":"        project_ref \u003d {\u0027project\u0027: {\u0027name\u0027: \u0027project_B\u0027, \u0027enabled\u0027: True,"}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_cf732e1c","line":783,"range":{"start_line":783,"start_character":44,"end_line":783,"end_character":52},"updated":"2018-07-11 21:52:45.000000000","message":"nit: we could shorten these up to just be \"A\", \"B\", \"C\", etc.. to match the diagram above (since project is pretty much implied).","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":780,"context_line":"        #   A        D"},{"line_number":781,"context_line":"        #  / \\      / \\"},{"line_number":782,"context_line":"        # B   C    E   F"},{"line_number":783,"context_line":"        project_ref \u003d {\u0027project\u0027: {\u0027name\u0027: \u0027project_A\u0027, \u0027enabled\u0027: True}}"},{"line_number":784,"context_line":"        response \u003d self.post(\u0027/projects\u0027, body\u003dproject_ref)"},{"line_number":785,"context_line":"        self.project_A \u003d response.json_body[\u0027project\u0027]"},{"line_number":786,"context_line":"        project_ref \u003d {\u0027project\u0027: {\u0027name\u0027: \u0027project_B\u0027, \u0027enabled\u0027: True,"}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_51a493a3","line":783,"range":{"start_line":783,"start_character":44,"end_line":783,"end_character":52},"in_reply_to":"5f7c97a3_cf732e1c","updated":"2018-07-12 11:50:20.000000000","message":"sure","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"dc2c0778cd13d9545bd15422606332acce11747d","unresolved":false,"context_lines":[{"line_number":983,"context_line":"                                   region_id\u003dself.region_id,"},{"line_number":984,"context_line":"                                   resource_name\u003d\u0027volume\u0027,"},{"line_number":985,"context_line":"                                   resource_limit\u003d5)"},{"line_number":986,"context_line":"        ref_D \u003d unit.new_limit_ref(project_id\u003dself.project_D[\u0027id\u0027],"},{"line_number":987,"context_line":"                                   service_id\u003dself.service_id,"},{"line_number":988,"context_line":"                                   region_id\u003dself.region_id,"},{"line_number":989,"context_line":"                                   resource_name\u003d\u0027volume\u0027,"},{"line_number":990,"context_line":"                                   resource_limit\u003d9)"},{"line_number":991,"context_line":"        ref_E \u003d unit.new_limit_ref(project_id\u003dself.project_E[\u0027id\u0027],"},{"line_number":992,"context_line":"                                   service_id\u003dself.service_id,"},{"line_number":993,"context_line":"                                   region_id\u003dself.region_id,"}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_166e1f2c","line":990,"range":{"start_line":986,"start_character":7,"end_line":990,"end_character":52},"updated":"2018-07-11 21:52:45.000000000","message":"With the changes I have locally, this fails. The reason why is because project tree D, E, and F already exists. Which means by default they are assuming the registered limit default of 10 volumes.\n\nWhen processing this request, the limit for D should fail because it\u0027s lower than the established registered limit. According to the specification, it should be possible to create a *new* project tree with D being the parent and having a project specific override of 9. This works in theory because it doesn\u0027t have any children assuming the registered limit to fail on (last example here [0]).\n\n[0] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html#enforcement-diagrams","commit_id":"e488cb0bede5dfbc20cd504a9fab0d2dad3e0d13"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":914,"context_line":"            expected_status\u003dhttp_client.FORBIDDEN)"},{"line_number":915,"context_line":""},{"line_number":916,"context_line":"    def test_create_parent_limit(self):"},{"line_number":917,"context_line":"        # When B is 9 , success to set A to 12"},{"line_number":918,"context_line":"        #     A              A,12"},{"line_number":919,"context_line":"        #    / \\    --\u003e      / \\"},{"line_number":920,"context_line":"        #  B,9  C          B,9  C"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_7bc72e67","line":917,"updated":"2018-07-12 10:24:20.000000000","message":"What is the default registered limit in this case?","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":914,"context_line":"            expected_status\u003dhttp_client.FORBIDDEN)"},{"line_number":915,"context_line":""},{"line_number":916,"context_line":"    def test_create_parent_limit(self):"},{"line_number":917,"context_line":"        # When B is 9 , success to set A to 12"},{"line_number":918,"context_line":"        #     A              A,12"},{"line_number":919,"context_line":"        #    / \\    --\u003e      / \\"},{"line_number":920,"context_line":"        #  B,9  C          B,9  C"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_1105bbc1","line":917,"in_reply_to":"5f7c97a3_7bc72e67","updated":"2018-07-12 11:50:20.000000000","message":"in all test, if the registered limit is not changed, the default value is 10. See setUp at L471","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"}],"releasenotes/notes/bp-strict-two-level-model.yaml":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":"   In this model:"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"   1. The project depth is force limited to 2 level."},{"line_number":11,"context_line":"   2. Any child project\u0027s limit can not exceed his parent\u0027s."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"   Please ensure that the previous limit deployment in your Keystone won\u0027t"},{"line_number":14,"context_line":"   break this model before starting to use it."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"5f7c97a3_d761c0a9","line":11,"range":{"start_line":10,"start_character":3,"end_line":11,"end_character":60},"updated":"2018-07-09 20:26:47.000000000","message":"We could provide a link to the specification here, too [0]. It is available via the blueprint, too.\n\n[0] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"6e091998b381c9fdce98295ac9ce20f3f1a513e4","unresolved":false,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":"   In this model:"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"   1. The project depth is force limited to 2 level."},{"line_number":11,"context_line":"   2. Any child project\u0027s limit can not exceed his parent\u0027s."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"   Please ensure that the previous limit deployment in your Keystone won\u0027t"},{"line_number":14,"context_line":"   break this model before starting to use it."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"5f7c97a3_fa161ca1","line":11,"range":{"start_line":10,"start_character":3,"end_line":11,"end_character":60},"in_reply_to":"5f7c97a3_d761c0a9","updated":"2018-07-10 03:38:31.000000000","message":"Done","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a7413bbdb929d18606138c41962ed643fdbbf206","unresolved":false,"context_lines":[{"line_number":10,"context_line":"   1. The project depth is force limited to 2 level."},{"line_number":11,"context_line":"   2. Any child project\u0027s limit can not exceed his parent\u0027s."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"   Please ensure that the previous limit deployment in your Keystone won\u0027t"},{"line_number":14,"context_line":"   break this model before starting to use it."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"5f7c97a3_37491431","line":13,"range":{"start_line":13,"start_character":35,"end_line":13,"end_character":40},"updated":"2018-07-09 20:26:47.000000000","message":"nit: project structure* ?","commit_id":"4bb3604663beb7ba9cc9280df06dd0c7b08a3cb7"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"c5a67215c44b27001a0496e2874055274384264a","unresolved":false,"context_lines":[{"line_number":3,"context_line":" - \u003e"},{"line_number":4,"context_line":"   [`blueprint strict-two-level-model \u003chttps://blueprints.launchpad.net/keystone/+spec/strict-two-level-model\u003e`_]"},{"line_number":5,"context_line":"   A new limit enforcement model called `strict_two_level` is added. Change the"},{"line_number":6,"context_line":"   value of the option `enforcement_model` to `strict_two_level` to enable it."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"   In this [`model \u003chttp://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html\u003e`_]:"},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":21,"id":"5f7c97a3_1b45da44","line":6,"range":{"start_line":6,"start_character":24,"end_line":6,"end_character":41},"updated":"2018-07-12 10:24:20.000000000","message":"I would use `[unified_limit]/enforcement_model` to indicate what group this configuration option is in.","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"021d2d089184ef84fe4ed16b8d4cfed76eb43c8b","unresolved":false,"context_lines":[{"line_number":3,"context_line":" - \u003e"},{"line_number":4,"context_line":"   [`blueprint strict-two-level-model \u003chttps://blueprints.launchpad.net/keystone/+spec/strict-two-level-model\u003e`_]"},{"line_number":5,"context_line":"   A new limit enforcement model called `strict_two_level` is added. Change the"},{"line_number":6,"context_line":"   value of the option `enforcement_model` to `strict_two_level` to enable it."},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"   In this [`model \u003chttp://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html\u003e`_]:"},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":21,"id":"5f7c97a3_91f0ebdc","line":6,"range":{"start_line":6,"start_character":24,"end_line":6,"end_character":41},"in_reply_to":"5f7c97a3_1b45da44","updated":"2018-07-12 11:50:20.000000000","message":"sure","commit_id":"13b2b1ad697cc512f3cb792b578f56152b382b38"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"bf202f289f2eeebfb26d39a3ec82c18a0ebd7050","unresolved":false,"context_lines":[{"line_number":9,"context_line":"   In this [`model \u003chttp://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html\u003e`_]:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"   1. The project depth is force limited to 2 level."},{"line_number":12,"context_line":"   2. Any child project\u0027s limit can not exceed his parent\u0027s."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"   Please ensure that the previous project and limit structure deployment in"},{"line_number":15,"context_line":"   your Keystone won\u0027t break this model before starting to use it."}],"source_content_type":"text/x-yaml","patch_set":25,"id":"5f7c97a3_90bb97bc","line":12,"range":{"start_line":12,"start_character":47,"end_line":12,"end_character":50},"updated":"2018-07-17 20:02:03.000000000","message":"nit: the*","commit_id":"4b4835a01c662ddb11e9f1aace47a02c6e337978"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"4902b1464f445d63dbd9e61ff4ae7ff172357bcf","unresolved":false,"context_lines":[{"line_number":9,"context_line":"   In this [`model \u003chttp://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/strict-two-level-enforcement-model.html\u003e`_]:"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"   1. The project depth is force limited to 2 level."},{"line_number":12,"context_line":"   2. Any child project\u0027s limit can not exceed his parent\u0027s."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"   Please ensure that the previous project and limit structure deployment in"},{"line_number":15,"context_line":"   your Keystone won\u0027t break this model before starting to use it."}],"source_content_type":"text/x-yaml","patch_set":25,"id":"5f7c97a3_6b5cca19","line":12,"range":{"start_line":12,"start_character":47,"end_line":12,"end_character":50},"in_reply_to":"5f7c97a3_90bb97bc","updated":"2018-07-17 20:28:33.000000000","message":"We can fix this in a followup as long as we do it before the branch is cut","commit_id":"4b4835a01c662ddb11e9f1aace47a02c6e337978"}]}