)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b26a54ed79a8bf3454f1139ee8ba984e6e214f37","unresolved":false,"context_lines":[{"line_number":16,"context_line":"intact and unconsumed, trusting expecting the caller to do the right"},{"line_number":17,"context_line":"thing is the only reasonable interface.  We must cleanly close any WSGI"},{"line_number":18,"context_line":"app_iter which we do not return to the client regardless of status code"},{"line_number":19,"context_line":"and allow the logging of the 499 if needed."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Closes-Bug: #1675650"},{"line_number":22,"context_line":"Change-Id: I455b5c38074ad0e72aa5e0b05771e193208905eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7f96bb07_2c7c9b1d","line":19,"updated":"2018-01-16 23:57:49.000000000","message":"I think this captures most of what we\u0027ve learned about our requirements.","commit_id":"63f7bfa32b4e3fbaff02242b5e2e24e01f5a724d"}],"swift/common/internal_client.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5594f3aa2abe8d3b57790ae9b87936c3598799ca","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                resp \u003d req.get_response(self.app)"},{"line_number":195,"context_line":"                if resp.status_int in acceptable_statuses or \\"},{"line_number":196,"context_line":"                        resp.status_int // 100 in acceptable_statuses:"},{"line_number":197,"context_line":"                    return resp"},{"line_number":198,"context_line":"            except (Exception, Timeout):"},{"line_number":199,"context_line":"                exc_type, exc_value, exc_traceback \u003d exc_info()"},{"line_number":200,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_aee0e797","line":197,"updated":"2017-12-27 21:28:50.000000000","message":"It seems like here we\u0027re trusting the caller to consume the body - which may only make sense for download_object and 2XX responses?","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd982f1eaf836456d4c92e79eb9db4e38052531d","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                resp \u003d req.get_response(self.app)"},{"line_number":195,"context_line":"                if resp.status_int in acceptable_statuses or \\"},{"line_number":196,"context_line":"                        resp.status_int // 100 in acceptable_statuses:"},{"line_number":197,"context_line":"                    return resp"},{"line_number":198,"context_line":"            except (Exception, Timeout):"},{"line_number":199,"context_line":"                exc_type, exc_value, exc_traceback \u003d exc_info()"},{"line_number":200,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_594d9d70","line":197,"in_reply_to":"bf8cb3f7_aee0e797","updated":"2018-01-12 06:38:37.000000000","message":"In any case, the caller should take a responsibility on the response body read for the raw response instance, IMO. Otherwise, we may need to load all buffer in memory always.\n\nIf the caller doesn\u0027t read the body, 499 Client Disconnected seems intuitive, isn\u0027t it?","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5594f3aa2abe8d3b57790ae9b87936c3598799ca","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                # Drain all response body to log correct status,"},{"line_number":202,"context_line":"                # even if we\u0027ll do retry"},{"line_number":203,"context_line":"                for iter_body in resp.app_iter:"},{"line_number":204,"context_line":"                    pass"},{"line_number":205,"context_line":"            # sleep only between tries, not after each one"},{"line_number":206,"context_line":"            if attempt \u003c self.request_tries - 1:"},{"line_number":207,"context_line":"                sleep(2 ** (attempt + 1))"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_eed6efc3","line":204,"updated":"2017-12-27 21:28:50.000000000","message":"yeah, failing to consume the response body isn\u0027t great - but I don\u0027t think we can *always* do this.\n\nIt seems possible to pass in a acceptable_statuses that would cause this code to download a bunch of data where we might have preferred the 499... maybe we could do this *only* for non-2XX responses?","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd982f1eaf836456d4c92e79eb9db4e38052531d","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                # Drain all response body to log correct status,"},{"line_number":202,"context_line":"                # even if we\u0027ll do retry"},{"line_number":203,"context_line":"                for iter_body in resp.app_iter:"},{"line_number":204,"context_line":"                    pass"},{"line_number":205,"context_line":"            # sleep only between tries, not after each one"},{"line_number":206,"context_line":"            if attempt \u003c self.request_tries - 1:"},{"line_number":207,"context_line":"                sleep(2 ** (attempt + 1))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_d312efb1","line":204,"in_reply_to":"bf8cb3f7_eed6efc3","updated":"2018-01-12 06:38:37.000000000","message":"Hmm... it may be true thing. I\u0027m not sure how many client expects to avoid 2xx in the acceptable status but it\u0027s going to be safe way.\n\nThanks.","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b0916789bfcaea0c221f6bd96fd710c4d5904447","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                    return resp"},{"line_number":198,"context_line":"            except (Exception, Timeout):"},{"line_number":199,"context_line":"                exc_type, exc_value, exc_traceback \u003d exc_info()"},{"line_number":200,"context_line":"            else:"},{"line_number":201,"context_line":"                if resp.status_int // 100 !\u003d 2:"},{"line_number":202,"context_line":"                    # Drain all response body to log correct status except"},{"line_number":203,"context_line":"                    # 2xx response case. The reason why 2xx ignored here is"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_8129f88a","line":200,"updated":"2018-01-16 23:05:33.000000000","message":"We definitely still want the else Kota has here -- otherwise resp will most likely still be None and resp.status_int will raise an AttributeError.","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"53c0f09431b275f026fada0fb836c45223ba012b","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                    # 2xx response case. The reason why 2xx ignored here is"},{"line_number":204,"context_line":"                    # because it would be a large object body."},{"line_number":205,"context_line":"                    for iter_body in resp.app_iter:"},{"line_number":206,"context_line":"                        pass"},{"line_number":207,"context_line":"            # sleep only between tries, not after each one"},{"line_number":208,"context_line":"            if attempt \u003c self.request_tries - 1:"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_38e4dddf","line":206,"updated":"2018-01-16 21:34:18.000000000","message":"I wonder if we should also do a\n\n close_if_possible(resp.app_iter)\n\nregardless of status...","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"be2c7679e875f7ca7008e748b560e7b6f9f526c2","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                    # 2xx response case. The reason why 2xx ignored here is"},{"line_number":204,"context_line":"                    # because it would be a large object body."},{"line_number":205,"context_line":"                    for iter_body in resp.app_iter:"},{"line_number":206,"context_line":"                        pass"},{"line_number":207,"context_line":"            # sleep only between tries, not after each one"},{"line_number":208,"context_line":"            if attempt \u003c self.request_tries - 1:"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_c66ab67f","line":206,"in_reply_to":"7f96bb07_38e4dddf","updated":"2018-01-16 22:30:38.000000000","message":"Yes, we should. It\u0027s part of the WSGI spec that, if your iterable has a close() method, the WSGI server will call it.","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ed1b17fe390f7515f026878377db39ff23b1bab8","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                    for iter_body in resp.app_iter:"},{"line_number":206,"context_line":"                        pass"},{"line_number":207,"context_line":"            # sleep only between tries, not after each one"},{"line_number":208,"context_line":"            if attempt \u003c self.request_tries - 1:"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            raise UnexpectedResponse("}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_46a226fe","line":208,"updated":"2018-01-16 23:16:10.000000000","message":"Maybe we avoid some interface problems by only doing the drain here before when we\u0027re about to drop the request away before retry?","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ed1b17fe390f7515f026878377db39ff23b1bab8","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            raise UnexpectedResponse("},{"line_number":212,"context_line":"                _(\u0027Unexpected response: %s\u0027) % resp.status, resp)"},{"line_number":213,"context_line":"        if exc_type:"},{"line_number":214,"context_line":"            # To make pep8 tool happy, in place of raise t, v, tb:"},{"line_number":215,"context_line":"            six.reraise(exc_type(*exc_value.args), None, exc_traceback)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_bb1ea7b8","line":212,"updated":"2018-01-16 23:16:10.000000000","message":"idk, I think it\u0027s weird to raise with the resp object attribute having had it\u0027s useful body message information thrown on the floor?\n\nIf we don\u0027t want to burden our callers to have to consume the iterator we could pre-populate resp.body before we return [1]?\n\n1. https://github.com/openstack/swift/blob/6e394bba0a783cf6bf06c6f60d4ccda150a87e67/swift/common/swob.py#L300","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"99dec32069552852b8f0e61f750e905b39ea9a64","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            raise UnexpectedResponse("},{"line_number":212,"context_line":"                _(\u0027Unexpected response: %s\u0027) % resp.status, resp)"},{"line_number":213,"context_line":"        if exc_type:"},{"line_number":214,"context_line":"            # To make pep8 tool happy, in place of raise t, v, tb:"},{"line_number":215,"context_line":"            six.reraise(exc_type(*exc_value.args), None, exc_traceback)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_887cb483","line":212,"in_reply_to":"7f96bb07_bb1ea7b8","updated":"2018-01-17 06:41:37.000000000","message":"+1 to ensure to close the resp.app_iter here. i confirmed the newest patch set is doing that, to drain the body on non 2xx case. I\u0027m not sure the responsibility on the resp body read if the client explicitly sets non 2xx in the acceptable status and UnexpectedResponse raised, though.","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b0916789bfcaea0c221f6bd96fd710c4d5904447","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                if resp.status_int // 100 !\u003d 2:"},{"line_number":202,"context_line":"                    # for non 2XX requests it\u0027s safe and useful to drain the"},{"line_number":203,"context_line":"                    # response body so we log the correct status code"},{"line_number":204,"context_line":"                    with closing_if_possible(resp.app_iter):"},{"line_number":205,"context_line":"                        for iter_body in resp.app_iter:"},{"line_number":206,"context_line":"                            pass"},{"line_number":207,"context_line":"                sleep(2 ** (attempt + 1))"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_016ba8b6","line":204,"updated":"2018-01-16 23:05:33.000000000","message":"...and I\u0027d be totally in favor of moving this `with` up outside of the status check -- we\u0027re about to go attempt another request, we ought to clean up the response that\u0027s *not* going back out to the caller, even if it\u0027s a 2xx.","commit_id":"1d16caa6b5ab056cee62010ccff409878b14d15d"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"9cabb514d6e9bca3e52d29c83846c8a1cd4828f6","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                        # for non 2XX requests it\u0027s safe and useful to drain"},{"line_number":205,"context_line":"                        # the response body so we log the correct status code"},{"line_number":206,"context_line":"                        if resp.status_int // 100 !\u003d 2:"},{"line_number":207,"context_line":"                            for iter_body in resp.app_iter:"},{"line_number":208,"context_line":"                                pass"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            msg \u003d \u0027Unexpected response: %s\u0027 % resp.status"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_7c0c92a4","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":36},"updated":"2018-01-17 01:54:23.000000000","message":"perhaps, i would be wrong, but in my understanding, iteration of resp.app_iter doesn\u0027t set the resp.body used at L212 check.\n\nIf we need it, i think just call resp.body to load the buffer into the resp.body. I\u0027ll check if my idea would be correct (or wrong) in this afternoon, so this is just my memo.","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"894b773d391213e18d1264d5a7d9ef9ddcf7388c","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                        # for non 2XX requests it\u0027s safe and useful to drain"},{"line_number":205,"context_line":"                        # the response body so we log the correct status code"},{"line_number":206,"context_line":"                        if resp.status_int // 100 !\u003d 2:"},{"line_number":207,"context_line":"                            for iter_body in resp.app_iter:"},{"line_number":208,"context_line":"                                pass"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            msg \u003d \u0027Unexpected response: %s\u0027 % resp.status"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_9c45deb6","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":36},"in_reply_to":"7f96bb07_7c0c92a4","updated":"2018-01-17 01:58:31.000000000","message":"Correct -- which is why Clay moved it down to the \"we\u0027re going to make another attempt\" block.\n\nMaybe we should be setting\n\n resp \u003d None\n\nat the start of the loop, though? What do we want to happen if you get 5xx, 5xx, uncaught exception? I kinda feel like the exception ought to be returned, but that isn\u0027t what this (or master) does...","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c07b8afd685dd7970db71be2f7ffd5556c4b3d6","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                        # for non 2XX requests it\u0027s safe and useful to drain"},{"line_number":205,"context_line":"                        # the response body so we log the correct status code"},{"line_number":206,"context_line":"                        if resp.status_int // 100 !\u003d 2:"},{"line_number":207,"context_line":"                            for iter_body in resp.app_iter:"},{"line_number":208,"context_line":"                                pass"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            msg \u003d \u0027Unexpected response: %s\u0027 % resp.status"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_fc25020a","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":36},"in_reply_to":"7f96bb07_7c0c92a4","updated":"2018-01-17 01:57:58.000000000","message":"since we don\u0027t return *this* response I think it\u0027s ok to discard the body - there\u0027s no way for the client to get at it with this interface anyway...","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"99dec32069552852b8f0e61f750e905b39ea9a64","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                        # for non 2XX requests it\u0027s safe and useful to drain"},{"line_number":205,"context_line":"                        # the response body so we log the correct status code"},{"line_number":206,"context_line":"                        if resp.status_int // 100 !\u003d 2:"},{"line_number":207,"context_line":"                            for iter_body in resp.app_iter:"},{"line_number":208,"context_line":"                                pass"},{"line_number":209,"context_line":"                sleep(2 ** (attempt + 1))"},{"line_number":210,"context_line":"        if resp:"},{"line_number":211,"context_line":"            msg \u003d \u0027Unexpected response: %s\u0027 % resp.status"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_0820a447","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":36},"in_reply_to":"7f96bb07_9c45deb6","updated":"2018-01-17 06:41:37.000000000","message":"OOh, i see. I was missing Clay moved the logic from try/except/else block to here.","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"99dec32069552852b8f0e61f750e905b39ea9a64","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            if resp.status_int // 100 !\u003d 2 and resp.body:"},{"line_number":213,"context_line":"                # provide additional context (and drain the response body) for"},{"line_number":214,"context_line":"                # non 2XX responses"},{"line_number":215,"context_line":"                msg +\u003d \u0027 (%s)\u0027 % resp.body"},{"line_number":216,"context_line":"            raise UnexpectedResponse(msg, resp)"},{"line_number":217,"context_line":"        if exc_type:"},{"line_number":218,"context_line":"            # To make pep8 tool happy, in place of raise t, v, tb:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_7e7c8282","line":215,"range":{"start_line":215,"start_character":25,"end_line":215,"end_character":29},"updated":"2018-01-17 06:41:37.000000000","message":"got it, the test failure at patch set 6 is caused by here. no resp.body would be empty \"()\" in the old patch. Make sense.","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"}],"test/unit/common/test_internal_client.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5594f3aa2abe8d3b57790ae9b87936c3598799ca","unresolved":false,"context_lines":[{"line_number":449,"context_line":"            def fake_app(self, env, start_response):"},{"line_number":450,"context_line":"                body \u003d \u0027fake error response\u0027"},{"line_number":451,"context_line":"                start_response(\u0027409 Ok\u0027, [(\u0027Content-Length\u0027, str(len(body)))])"},{"line_number":452,"context_line":"                return [body]"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"        client \u003d InternalClient()"},{"line_number":455,"context_line":"        with self.assertRaises(internal_client.UnexpectedResponse):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_6ecbbf1d","line":452,"updated":"2017-12-27 21:28:50.000000000","message":"these existing tests have a non-trivial amount of boiler plate","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd982f1eaf836456d4c92e79eb9db4e38052531d","unresolved":false,"context_lines":[{"line_number":449,"context_line":"            def fake_app(self, env, start_response):"},{"line_number":450,"context_line":"                body \u003d \u0027fake error response\u0027"},{"line_number":451,"context_line":"                start_response(\u0027409 Ok\u0027, [(\u0027Content-Length\u0027, str(len(body)))])"},{"line_number":452,"context_line":"                return [body]"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"        client \u003d InternalClient()"},{"line_number":455,"context_line":"        with self.assertRaises(internal_client.UnexpectedResponse):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_d9a20d9b","line":452,"in_reply_to":"bf8cb3f7_6ecbbf1d","updated":"2018-01-12 06:38:37.000000000","message":"Exactly, it would be worth to have an generic boiler plate. This just follows the existing tests for now.","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5594f3aa2abe8d3b57790ae9b87936c3598799ca","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"        client \u003d InternalClient()"},{"line_number":455,"context_line":"        with self.assertRaises(internal_client.UnexpectedResponse):"},{"line_number":456,"context_line":"            client.make_request(\u0027DELETE\u0027, \u0027/container\u0027, {}, (200,))"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":459,"context_line":"            self.assertIn(\u0027409\u0027, logline)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_0ed233d3","line":456,"updated":"2017-12-27 21:28:50.000000000","message":"I don\u0027t like how long this test takes...\n\n    diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py\n    index 5eae061..5b2768a 100644\n    --- a/test/unit/common/test_internal_client.py\n    +++ b/test/unit/common/test_internal_client.py\n    @@ -452,7 +452,8 @@ class TestInternalClient(unittest.TestCase):\n                     return [body]\n     \n             client \u003d InternalClient()\n    -        with self.assertRaises(internal_client.UnexpectedResponse):\n    +        with self.assertRaises(internal_client.UnexpectedResponse), \\\n    +                mock.patch(\u0027swift.common.internal_client.sleep\u0027):\n                 client.make_request(\u0027DELETE\u0027, \u0027/container\u0027, {}, (200,))\n     \n             for logline in client.logger.get_lines_for_level(\u0027info\u0027):","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd982f1eaf836456d4c92e79eb9db4e38052531d","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"        client \u003d InternalClient()"},{"line_number":455,"context_line":"        with self.assertRaises(internal_client.UnexpectedResponse):"},{"line_number":456,"context_line":"            client.make_request(\u0027DELETE\u0027, \u0027/container\u0027, {}, (200,))"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":459,"context_line":"            self.assertIn(\u0027409\u0027, logline)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_f91ee90a","line":456,"in_reply_to":"bf8cb3f7_0ed233d3","updated":"2018-01-12 06:38:37.000000000","message":"Good lesson. Thanks!","commit_id":"7531e04204944d0f41a16faa3ada1db8a57b9d0e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"4e5586611ece7981b5faa50ad89c1591387e8e5a","unresolved":false,"context_lines":[{"line_number":486,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":487,"context_line":"            # This will cause 499 Client Disconnect, not 200"},{"line_number":488,"context_line":"            self.assertIn(\u0027499\u0027, logline)"},{"line_number":489,"context_line":"            self.assertNotIn(\u0027200\u0027, logline)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"    def test_make_request_codes(self):"},{"line_number":492,"context_line":"        class InternalClient(internal_client.InternalClient):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f96bb07_373775b3","line":489,"updated":"2018-01-12 09:30:32.000000000","message":"OMG, this matches with \"swift.unit.fake_logger INFO: - - 12/Jan/2018/06/56/00 GET /cont/obj HTTP/1.0 499 - some_agent - - 19 - - - 0.0005 - - 1515740160.200530052 1515740160.200992107 -\" !?\n\nWe can take a couple of ways to avoid this: A) add space before/after the status code, B) mock the timestamp.\n\nHmm... :/","commit_id":"e59fa38b9e6b6dc46f60c6d17f8a6bd201a0c4f4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"53c0f09431b275f026fada0fb836c45223ba012b","unresolved":false,"context_lines":[{"line_number":457,"context_line":"                mock.patch(\u0027swift.common.internal_client.sleep\u0027):"},{"line_number":458,"context_line":"            client.make_request(\u0027DELETE\u0027, \u0027/container\u0027, {}, (200,))"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":461,"context_line":"            # add spaces before/after 409 because it can match with"},{"line_number":462,"context_line":"            # something like timestamp"},{"line_number":463,"context_line":"            self.assertIn(\u0027 409 \u0027, logline)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_f83f5522","line":460,"range":{"start_line":460,"start_character":8,"end_line":460,"end_character":64},"updated":"2018-01-16 21:34:18.000000000","message":"Worth having an assertion for how many times log lines we expect?","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"53c0f09431b275f026fada0fb836c45223ba012b","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            # correct object body with 2xx."},{"line_number":486,"context_line":"            client.make_request(\u0027GET\u0027, \u0027/cont/obj\u0027, {}, (400,))"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":489,"context_line":"            # This will cause 499 Client Disconnect, not 200."},{"line_number":490,"context_line":"            # add spaces before/after 499 because it can match with"},{"line_number":491,"context_line":"            # something like timestamp"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_b8454db6","line":488,"updated":"2018-01-16 21:34:18.000000000","message":"ditto","commit_id":"86cd97ef75c0c146b33c9cc69204ca96523a71b5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b0916789bfcaea0c221f6bd96fd710c4d5904447","unresolved":false,"context_lines":[{"line_number":489,"context_line":"            # This will cause 499 Client Disconnect, not 200."},{"line_number":490,"context_line":"            # add spaces before/after 499 because it can match with"},{"line_number":491,"context_line":"            # something like timestamp"},{"line_number":492,"context_line":"            self.assertIn(\u0027 499 \u0027, logline)"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def test_make_request_codes(self):"},{"line_number":495,"context_line":"        class InternalClient(internal_client.InternalClient):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_e15d5c15","line":492,"updated":"2018-01-16 23:05:33.000000000","message":"... and this is the assertion that made me think of it. We\u0027ve got 3x 499 getting logged, and only one of those has any hope of getting exposed to the caller for them to close.","commit_id":"1d16caa6b5ab056cee62010ccff409878b14d15d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b26a54ed79a8bf3454f1139ee8ba984e6e214f37","unresolved":false,"context_lines":[{"line_number":487,"context_line":"        client \u003d InternalClient()"},{"line_number":488,"context_line":"        with self.assertRaises(internal_client.UnexpectedResponse) as ctx, \\"},{"line_number":489,"context_line":"                mock.patch(\u0027swift.common.internal_client.sleep\u0027):"},{"line_number":490,"context_line":"            # This is obvious strage tests to expect only 400 Bad Request"},{"line_number":491,"context_line":"            # but this test intended to avoid extra body drain if it\u0027s"},{"line_number":492,"context_line":"            # correct object body with 2xx."},{"line_number":493,"context_line":"            client.make_request(\u0027GET\u0027, \u0027/cont/obj\u0027, {}, (400,))"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f96bb07_cc9bb770","line":490,"updated":"2018-01-16 23:57:49.000000000","message":"s/strage/strange/","commit_id":"63f7bfa32b4e3fbaff02242b5e2e24e01f5a724d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"16af3acbcbf552c4efefcdc9259d0e8a577e68b5","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        self.assertEqual(closed_paths, [\u0027/cont/obj\u0027] * 3)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        # Retries like this will cause 499 because internal_client won\u0027t"},{"line_number":500,"context_line":"        # automatically consume (potentially large) 2XX responses."},{"line_number":501,"context_line":"        expected \u003d (\u0027 499 \u0027, \u0027 499 \u0027, \u0027 200 \u0027)"},{"line_number":502,"context_line":"        loglines \u003d client.logger.get_lines_for_level(\u0027info\u0027)"},{"line_number":503,"context_line":"        for expected, logline in izip_longest(expected, loglines):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f96bb07_8c85af5c","line":500,"updated":"2018-01-16 23:51:06.000000000","message":"This will have interesting interactions with https://review.openstack.org/#/c/534399/ ...","commit_id":"63f7bfa32b4e3fbaff02242b5e2e24e01f5a724d"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"99dec32069552852b8f0e61f750e905b39ea9a64","unresolved":false,"context_lines":[{"line_number":461,"context_line":"        for logline in client.logger.get_lines_for_level(\u0027info\u0027):"},{"line_number":462,"context_line":"            # add spaces before/after 409 because it can match with"},{"line_number":463,"context_line":"            # something like timestamp"},{"line_number":464,"context_line":"            self.assertIn(\u0027 409 \u0027, logline)"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def test_make_request_acceptable_status_not_2xx(self):"},{"line_number":467,"context_line":"        closed_paths \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_2343416e","line":464,"updated":"2018-01-17 06:41:37.000000000","message":"I thought I should add an assertion that how many times the 409 appears here but decided to pend it until https://review.openstack.org/#/c/534399 landed because it will change the retry counts here.","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"99dec32069552852b8f0e61f750e905b39ea9a64","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # but this test intended to avoid extra body drain if it\u0027s"},{"line_number":492,"context_line":"            # correct object body with 2xx."},{"line_number":493,"context_line":"            client.make_request(\u0027GET\u0027, \u0027/cont/obj\u0027, {}, (400,))"},{"line_number":494,"context_line":"        self.assertEqual(closed_paths, [\u0027/cont/obj\u0027] * 2)"},{"line_number":495,"context_line":"        # swob\u0027s body resp property will call close"},{"line_number":496,"context_line":"        self.assertEqual(ctx.exception.resp.body, \u0027fake error response\u0027)"},{"line_number":497,"context_line":"        self.assertEqual(closed_paths, [\u0027/cont/obj\u0027] * 3)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f96bb07_a3c2f132","line":494,"updated":"2018-01-17 06:41:37.000000000","message":"ok. if the client sets non-2xx code as acceptable response, the responsibility on the response read and closing app_iter is onto the client. I\u0027m a bit worried about the existing code but any existing clients ( the container-reconciler, the container-sync, and the object-expirer) uses either successful response codes plus alpha or defaults. The defaults include \"(2,\", i checked.","commit_id":"71d1828e936946e4b382f34a336079cd75c4333a"}]}
