)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6cfb03505dbb56af1b66397bdaea81dcdfd0d62d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cb0d6aa8_3048da52","updated":"2024-04-04 14:58:35.000000000","message":"I had it in mind to push on this harder to have  FakeInternalClient that wraps a FakeSwift. Clay\u0027s feedback encouraged me to bite that bullet now, but I pretty soon realised that it is easier that I feared (I had missed that we can pass ``app`` to the InternalClient constructor!!).\n\nIn fact I ended up not needing a helper class at all (see later in chain https://review.opendev.org/c/openstack/swift/+/913835/6/test/unit/container/test_mpu_auditor.py#133)\n\nI\u0027m therefore abandoning this (and sadly we didn\u0027t need to move the FakeInternalClient either)\n\n Sorry @Tim @Clay for time wasted.","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a7dd1ca87811f8979a96e545d87a34873aa7b9a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b46eb08c_d500b477","updated":"2024-04-03 15:26:08.000000000","message":"tracking both the query and params is super weird.\n\nI think our Fake should just normalize to query \u003d params or query and callers sending both will learn quickly that real can\u0027t combine them.","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"}],"test/unit/__init__.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a7dd1ca87811f8979a96e545d87a34873aa7b9a0","unresolved":true,"context_lines":[{"line_number":1614,"context_line":"        else:"},{"line_number":1615,"context_line":"            body \u003d body_file.read()"},{"line_number":1616,"context_line":"        path, _, query \u003d path.partition(\u0027?\u0027)"},{"line_number":1617,"context_line":"        self.calls.append(AppCall(method, path, query, headers, body, params))"},{"line_number":1618,"context_line":"        resp \u003d next(self.resp_iter)"},{"line_number":1619,"context_line":"        if isinstance(resp, swob.HTTPException):"},{"line_number":1620,"context_line":"            return resp"}],"source_content_type":"text/x-python","patch_set":1,"id":"edf37bd6_a0a5ffa2","line":1617,"updated":"2024-04-03 15:26:08.000000000","message":"yeah having query and params is wonky; makes me thing we\u0027d probably generally be happier if tests consistently used a real InternalClient and a FakeSwift.","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6cfb03505dbb56af1b66397bdaea81dcdfd0d62d","unresolved":true,"context_lines":[{"line_number":1614,"context_line":"        else:"},{"line_number":1615,"context_line":"            body \u003d body_file.read()"},{"line_number":1616,"context_line":"        path, _, query \u003d path.partition(\u0027?\u0027)"},{"line_number":1617,"context_line":"        self.calls.append(AppCall(method, path, query, headers, body, params))"},{"line_number":1618,"context_line":"        resp \u003d next(self.resp_iter)"},{"line_number":1619,"context_line":"        if isinstance(resp, swob.HTTPException):"},{"line_number":1620,"context_line":"            return resp"}],"source_content_type":"text/x-python","patch_set":1,"id":"ed97b184_8048259c","line":1617,"in_reply_to":"edf37bd6_a0a5ffa2","updated":"2024-04-04 14:58:35.000000000","message":"It\u0027s not clear to me why the path and query string need to be split here.\n\nI agree re having a fake internal client that wraps a FakeSwift, AppCall smells like a FakeSwiftCall","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a7dd1ca87811f8979a96e545d87a34873aa7b9a0","unresolved":true,"context_lines":[{"line_number":1616,"context_line":"        path, _, query \u003d path.partition(\u0027?\u0027)"},{"line_number":1617,"context_line":"        self.calls.append(AppCall(method, path, query, headers, body, params))"},{"line_number":1618,"context_line":"        resp \u003d next(self.resp_iter)"},{"line_number":1619,"context_line":"        if isinstance(resp, swob.HTTPException):"},{"line_number":1620,"context_line":"            return resp"},{"line_number":1621,"context_line":"        elif isinstance(resp, Exception):"},{"line_number":1622,"context_line":"            raise resp"}],"source_content_type":"text/x-python","patch_set":1,"id":"de917a44_ab992960","line":1619,"updated":"2024-04-03 15:26:08.000000000","message":"the dynamism of this resp type is maybe a little confusing\n\nthe real \"make_request\" calls req.get_response(self.app) - which always *returns* a swob.Response (or maybe raises an exception if there\u0027s a bug) - but since an HTTPException is an instance of both swob.Response AND Exception maybe this could just be:\n\n        if isinstance(resp, swob.Response):\n            # this is the only valid return type from real make_request\n            return resp\n        else:\n            # this better be testing some kind of UnexpectedResponse\n            raise resp\n\n^ seems to work on test/unit/cli and test/unit/common","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6cfb03505dbb56af1b66397bdaea81dcdfd0d62d","unresolved":true,"context_lines":[{"line_number":1616,"context_line":"        path, _, query \u003d path.partition(\u0027?\u0027)"},{"line_number":1617,"context_line":"        self.calls.append(AppCall(method, path, query, headers, body, params))"},{"line_number":1618,"context_line":"        resp \u003d next(self.resp_iter)"},{"line_number":1619,"context_line":"        if isinstance(resp, swob.HTTPException):"},{"line_number":1620,"context_line":"            return resp"},{"line_number":1621,"context_line":"        elif isinstance(resp, Exception):"},{"line_number":1622,"context_line":"            raise resp"}],"source_content_type":"text/x-python","patch_set":1,"id":"30010c50_11d9efab","line":1619,"in_reply_to":"de917a44_ab992960","updated":"2024-04-04 14:58:35.000000000","message":"turns out the original user of this helper (test_container_deleter) never wanted to raise exceptions anyway, so this can all go","commit_id":"0b23eeea116cf5216a611d2dfc753d0c1072aeb1"}]}