)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":18,"context_line":"some redundant code, thus moving this into its own method."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Another test with multiple exceeded quotas has been added, which is"},{"line_number":21,"context_line":"failing without the bugfix."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #2118758"},{"line_number":24,"context_line":"Change-Id: I49ec4c5f6c83f36ce1d38f2f1687081c71488286"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3836e411_0093eef5","line":21,"updated":"2025-07-25 16:53:04.000000000","message":"great commit message, thanks!","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3f1c1b53_6ae28305","updated":"2025-07-25 16:53:04.000000000","message":"LGTM, I\u0027m going to give Matt or Tim chance to check because I think they have had more recent contact with this middleware than I.","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4098e3f4e62a30884d365759249dd7625121612","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"99d9f5d6_1abf8018","updated":"2025-07-25 16:54:46.000000000","message":"haha! in the time between me pulling the patch and posting my review I failed to see that Tim already has +A :","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"}],"swift/common/middleware/account_quotas.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":211,"context_line":"            if quota \u003c new_size:"},{"line_number":212,"context_line":"                resp \u003d HTTPRequestEntityTooLarge(body\u003d\u0027Upload exceeds quota.\u0027)"},{"line_number":213,"context_line":"                if \u0027swift.authorize\u0027 in request.environ:"},{"line_number":214,"context_line":"                    orig_authorize \u003d request.environ[\u0027swift.authorize\u0027]"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"                    def reject_authorize(*args, **kwargs):"},{"line_number":217,"context_line":"                        aresp \u003d orig_authorize(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fb3e3edc_c6843955","side":"PARENT","line":214,"range":{"start_line":214,"start_character":20,"end_line":214,"end_character":21},"updated":"2025-07-25 16:53:04.000000000","message":"right, so this is NOT a closure and ``orig_authorize`` is the *same* variable that may be set at line 235 etc. and if that happens then orig_authorize becomes the reject_authorize at line 216, that will call orig_authorize, which is itself, and so on","commit_id":"8af485775acc9ad97a6cbdca64279aac9f647f8c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":235,"context_line":"                    orig_authorize \u003d request.environ[\u0027swift.authorize\u0027]"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"                    def reject_authorize(*args, **kwargs):"},{"line_number":238,"context_line":"                        aresp \u003d orig_authorize(*args, **kwargs)"},{"line_number":239,"context_line":"                        if aresp:"},{"line_number":240,"context_line":"                            return aresp"},{"line_number":241,"context_line":"                        return resp"}],"source_content_type":"text/x-python","patch_set":1,"id":"ecaeaced_df8126fb","side":"PARENT","line":238,"updated":"2025-07-25 16:53:04.000000000","message":"IIUC the first quote to be exceeded would have determined the response body (putting to one side the recursion problem), so the proposed patch changes nothing there i.e. there seems to be no point in checking other quotas once one has been exceeded, even on master.","commit_id":"8af485775acc9ad97a6cbdca64279aac9f647f8c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":102,"context_line":"    def __init__(self, app, *args, **kwargs):"},{"line_number":103,"context_line":"        self.app \u003d app"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def quota_exceeded(self, request, body):"},{"line_number":106,"context_line":"        # request.environ[\u0027swift.authorize\u0027](req) is delayed and not called"},{"line_number":107,"context_line":"        # immediately to support container acls. However, the middleware should"},{"line_number":108,"context_line":"        # still return immediately if any quota is exceeded."}],"source_content_type":"text/x-python","patch_set":1,"id":"5c93f397_bcdf982f","line":105,"updated":"2025-07-25 16:53:04.000000000","message":"nit: perhaps name this ``resp_body`` to avoid any confusion w.r.t. ``request``","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def quota_exceeded(self, request, body):"},{"line_number":106,"context_line":"        # request.environ[\u0027swift.authorize\u0027](req) is delayed and not called"},{"line_number":107,"context_line":"        # immediately to support container acls. However, the middleware should"},{"line_number":108,"context_line":"        # still return immediately if any quota is exceeded."},{"line_number":109,"context_line":"        resp \u003d HTTPRequestEntityTooLarge(body\u003dbody)"},{"line_number":110,"context_line":"        if \u0027swift.authorize\u0027 in request.environ:"},{"line_number":111,"context_line":"            orig_authorize \u003d request.environ[\u0027swift.authorize\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"eea3181e_9ccb2003","line":108,"range":{"start_line":107,"start_character":62,"end_line":108,"end_character":34},"updated":"2025-07-25 16:53:04.000000000","message":"it took me a while to grok this - i.e. it is the *middleware __call__* that returns immediately, not the response to the client","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce2fd090ea2b8911ab563b6a5ce46683b63b4d30","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        # still return immediately if any quota is exceeded."},{"line_number":109,"context_line":"        resp \u003d HTTPRequestEntityTooLarge(body\u003dbody)"},{"line_number":110,"context_line":"        if \u0027swift.authorize\u0027 in request.environ:"},{"line_number":111,"context_line":"            orig_authorize \u003d request.environ[\u0027swift.authorize\u0027]"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            def reject_authorize(*args, **kwargs):"},{"line_number":114,"context_line":"                aresp \u003d orig_authorize(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"572752a7_d0d1d558","line":111,"updated":"2025-07-25 15:21:30.000000000","message":"I think the refactor means you\u0027ve fixed it twice ;-)\n\nA sizeable part of the trouble before was that `orig_authorize` was scoped to `__call__`, so the second time we try to add a `reject_authorize`, we\u0027d pick up the first `reject_authorize` which still wanted to call `orig_authorize` (but whose reference had changed). Now, it\u0027s scoped to `quota_exceeded`, so each call creates a closure and the `orig_authorize` isn\u0027t shared between `reject_authorize` definitions.","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce2fd090ea2b8911ab563b6a5ce46683b63b4d30","unresolved":true,"context_lines":[{"line_number":116,"context_line":"                    return aresp"},{"line_number":117,"context_line":"                return resp"},{"line_number":118,"context_line":"            request.environ[\u0027swift.authorize\u0027] \u003d reject_authorize"},{"line_number":119,"context_line":"            return self.app"},{"line_number":120,"context_line":"        else:"},{"line_number":121,"context_line":"            return resp"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32a902_5e1d5dd9","line":119,"updated":"2025-07-25 15:21:30.000000000","message":"Halting processing on the rest of the quota rules and just returning the app definitely seems like a win, too, though.","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":118,"context_line":"            request.environ[\u0027swift.authorize\u0027] \u003d reject_authorize"},{"line_number":119,"context_line":"            return self.app"},{"line_number":120,"context_line":"        else:"},{"line_number":121,"context_line":"            return resp"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def validate_and_translate_quotas(self, request, quota_type):"},{"line_number":124,"context_line":"        new_quotas \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"5926d16e_86e96f2a","line":121,"updated":"2025-07-25 16:53:04.000000000","message":"ironically, I think that this refactor alone (or similar) would have been sufficient to fix the bug because ``orig_authorize`` is now local to each instance of this function 😊\n\nbut there seems to be no reason to continue checking other quotas.","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"}],"test/unit/common/middleware/test_account_quotas.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3bb5cd4e51ba10ae57fb9c57fee657f69a351352","unresolved":true,"context_lines":[{"line_number":332,"context_line":"                            environ\u003d{\u0027swift.cache\u0027: cache})"},{"line_number":333,"context_line":"        res \u003d req.get_response(app)"},{"line_number":334,"context_line":"        self.assertEqual(res.status_int, 413)"},{"line_number":335,"context_line":"        self.assertEqual(res.body, b\u0027Upload exceeds quota.\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def test_over_quota_container_create_still_works(self):"},{"line_number":338,"context_line":"        self.app.register(\u0027HEAD\u0027, \u0027/v1/a\u0027, HTTPOk, {"}],"source_content_type":"text/x-python","patch_set":1,"id":"dffcc694_978b18a9","line":335,"updated":"2025-07-25 16:53:04.000000000","message":"off-topic: it\u0027s a shame that when the other quotas were added [1] there was no distinction made in the response messages \n\n[1] 921664: add object-count quota for accounts in middleware | https://review.opendev.org/c/openstack/swift/+/921664","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8301048ec0dfaec362a3d55dd1687bb2277842a6","unresolved":true,"context_lines":[{"line_number":332,"context_line":"                            environ\u003d{\u0027swift.cache\u0027: cache})"},{"line_number":333,"context_line":"        res \u003d req.get_response(app)"},{"line_number":334,"context_line":"        self.assertEqual(res.status_int, 413)"},{"line_number":335,"context_line":"        self.assertEqual(res.body, b\u0027Upload exceeds quota.\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def test_over_quota_container_create_still_works(self):"},{"line_number":338,"context_line":"        self.app.register(\u0027HEAD\u0027, \u0027/v1/a\u0027, HTTPOk, {"}],"source_content_type":"text/x-python","patch_set":1,"id":"aca5359c_1ac7ae40","line":335,"in_reply_to":"dffcc694_978b18a9","updated":"2025-07-25 17:54:13.000000000","message":"There was [some discussion about it at the time](https://review.opendev.org/c/openstack/swift/+/921664/1..4/swift/common/middleware/account_quotas.py#b190), but we punted it to a future change that also touched container quotas.","commit_id":"06a6329793decc58cbe0b2e7c4fdf2e46b20e758"}]}
