Skip to content

Commit

Permalink
Drain and close more internal client requests
Browse files Browse the repository at this point in the history
Specifically avoid logging a 499 when object-expirer calls
swift.delete_container after clearing a bucket

Related-Change-Id: I1d5c6e40e1833c53994f8e15b4850871789413ac
Change-Id: Ic82f8ba706a8b6889b37ec4fa40b24bdad3c3521
  • Loading branch information
clayg authored and tipabu committed Jul 7, 2021
1 parent 45a5ecc commit 26c3d81
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 12 deletions.
27 changes: 15 additions & 12 deletions swift/common/internal_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,12 @@ def make_request(
# To make pep8 tool happy, in place of raise t, v, tb:
six.reraise(exc_type, exc_value, exc_traceback)

def handle_request(self, *args, **kwargs):
resp = self.make_request(*args, **kwargs)
# Drain the response body to prevent unexpected disconnect
# in proxy-server
drain_and_close(resp)

def _get_metadata(
self, path, metadata_prefix='', acceptable_statuses=(2,),
headers=None, params=None):
Expand Down Expand Up @@ -363,7 +369,7 @@ def _set_metadata(
headers[k] = v
else:
headers['%s%s' % (metadata_prefix, k)] = v
self.make_request('POST', path, headers, acceptable_statuses)
self.handle_request('POST', path, headers, acceptable_statuses)

# account methods

Expand Down Expand Up @@ -402,7 +408,7 @@ def create_account(self, account):
unexpected way.
"""
path = self.make_path(account)
self.make_request('PUT', path, {}, (201, 202))
self.handle_request('PUT', path, {}, (201, 202))

def delete_account(self, account, acceptable_statuses=(2, HTTP_NOT_FOUND)):
"""
Expand All @@ -417,7 +423,7 @@ def delete_account(self, account, acceptable_statuses=(2, HTTP_NOT_FOUND)):
unexpected way.
"""
path = self.make_path(account)
self.make_request('DELETE', path, {}, acceptable_statuses)
self.handle_request('DELETE', path, {}, acceptable_statuses)

def get_account_info(
self, account, acceptable_statuses=(2, HTTP_NOT_FOUND)):
Expand Down Expand Up @@ -531,7 +537,7 @@ def create_container(

headers = headers or {}
path = self.make_path(account, container)
self.make_request('PUT', path, headers, acceptable_statuses)
self.handle_request('PUT', path, headers, acceptable_statuses)

def delete_container(
self, account, container, headers=None,
Expand All @@ -552,7 +558,7 @@ def delete_container(

headers = headers or {}
path = self.make_path(account, container)
self.make_request('DELETE', path, headers, acceptable_statuses)
self.handle_request('DELETE', path, headers, acceptable_statuses)

def get_container_metadata(
self, account, container, metadata_prefix='',
Expand Down Expand Up @@ -655,11 +661,8 @@ def delete_object(
"""

path = self.make_path(account, container, obj)
resp = self.make_request('DELETE', path, (headers or {}),
acceptable_statuses)
# Drain the response body to prevent unexpected disconnect
# in proxy-server
drain_and_close(resp)
self.handle_request('DELETE', path, (headers or {}),
acceptable_statuses)

def get_object_metadata(
self, account, container, obj, metadata_prefix='',
Expand Down Expand Up @@ -812,8 +815,8 @@ def upload_object(
if 'Content-Length' not in headers:
headers['Transfer-Encoding'] = 'chunked'
path = self.make_path(account, container, obj)
self.make_request('PUT', path, headers, acceptable_statuses, fobj,
params=params)
self.handle_request('PUT', path, headers, acceptable_statuses, fobj,
params=params)


def get_auth(url, user, key, auth_version='1.0', **kwargs):
Expand Down
108 changes: 108 additions & 0 deletions test/unit/common/test_internal_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,13 +1019,29 @@ def test_iter_containers(self):
ret_items.append(container)
self.assertEqual(items, ret_items)

def test_create_account(self):
account, container, obj = path_parts()
path = make_path_info(account)
client, app = get_client_app()
app.register('PUT', path, swob.HTTPCreated, {})
client.create_account(account)
self.assertEqual([('PUT', path, {
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'User-Agent': 'test'
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_delete_account(self):
account, container, obj = path_parts()
path = make_path_info(account)
client, app = get_client_app()
app.register('DELETE', path, swob.HTTPNoContent, {})
client.delete_account(account)
self.assertEqual(1, len(app._calls))
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_get_account_info(self):
class Response(object):
Expand Down Expand Up @@ -1115,6 +1131,22 @@ def test_get_metadadata_with_acceptable_status(self):
acceptable_statuses=(2, 4))

def test_set_account_metadata(self):
account, container, obj = path_parts()
path = make_path_info(account)
client, app = get_client_app()
app.register('POST', path, swob.HTTPAccepted, {})
client.set_account_metadata(account, {'Color': 'Blue'},
metadata_prefix='X-Account-Meta-')
self.assertEqual([('POST', path, {
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'X-Account-Meta-Color': 'Blue',
'User-Agent': 'test',
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_set_account_metadata_plumbing(self):
account, container, obj = path_parts()
path = make_path(account)
metadata = 'some_metadata'
Expand Down Expand Up @@ -1161,6 +1193,20 @@ def make_request(
self.assertEqual(1, client.make_request_called)

def test_create_container(self):
account, container, obj = path_parts()
path = make_path_info(account, container)
client, app = get_client_app()
app.register('PUT', path, swob.HTTPCreated, {})
client.create_container(account, container)
self.assertEqual([('PUT', path, {
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'User-Agent': 'test'
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_create_container_plumbing(self):
class InternalClient(internal_client.InternalClient):
def __init__(self, test, path, headers):
self.test = test
Expand All @@ -1186,6 +1232,16 @@ def make_request(
self.assertEqual(1, client.make_request_called)

def test_delete_container(self):
account, container, obj = path_parts()
path = make_path_info(account, container)
client, app = get_client_app()
app.register('DELETE', path, swob.HTTPNoContent, {})
client.delete_container(account, container)
self.assertEqual(1, len(app._calls))
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_delete_container_plumbing(self):
class InternalClient(internal_client.InternalClient):
def __init__(self, test, path):
self.test = test
Expand Down Expand Up @@ -1238,6 +1294,22 @@ def test_iter_objects(self):
self.assertEqual(items, ret_items)

def test_set_container_metadata(self):
account, container, obj = path_parts()
path = make_path_info(account, container)
client, app = get_client_app()
app.register('POST', path, swob.HTTPAccepted, {})
client.set_container_metadata(account, container, {'Color': 'Blue'},
metadata_prefix='X-Container-Meta-')
self.assertEqual([('POST', path, {
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'X-Container-Meta-Color': 'Blue',
'User-Agent': 'test',
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_set_container_metadata_plumbing(self):
account, container, obj = path_parts()
path = make_path(account, container)
metadata = 'some_metadata'
Expand All @@ -1257,10 +1329,15 @@ def test_delete_object(self):
client.delete_object(account, container, obj)
self.assertEqual(app.unclosed_requests, {})
self.assertEqual(1, len(app._calls))
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

app.register('DELETE', path, swob.HTTPNotFound, {})
client.delete_object(account, container, obj)
self.assertEqual(app.unclosed_requests, {})
self.assertEqual(2, len(app._calls))
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_get_object_metadata(self):
account, container, obj = path_parts()
Expand Down Expand Up @@ -1383,6 +1460,22 @@ def fake_app(self, env, start_response):
self.assertEqual([], lines)

def test_set_object_metadata(self):
account, container, obj = path_parts()
path = make_path_info(account, container, obj)
client, app = get_client_app()
app.register('POST', path, swob.HTTPAccepted, {})
client.set_object_metadata(account, container, obj, {'Color': 'Blue'},
metadata_prefix='X-Object-Meta-')
self.assertEqual([('POST', path, {
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'X-Object-Meta-Color': 'Blue',
'User-Agent': 'test',
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_set_object_metadata_plumbing(self):
account, container, obj = path_parts()
path = make_path(account, container, obj)
metadata = 'some_metadata'
Expand All @@ -1396,6 +1489,21 @@ def test_set_object_metadata(self):
self.assertEqual(1, client.set_metadata_called)

def test_upload_object(self):
account, container, obj = path_parts()
path = make_path_info(account, container, obj)
client, app = get_client_app()
app.register('PUT', path, swob.HTTPCreated, {})
client.upload_object(BytesIO(b'fobj'), account, container, obj)
self.assertEqual([('PUT', path, {
'Transfer-Encoding': 'chunked',
'X-Backend-Allow-Reserved-Names': 'true',
'Host': 'localhost:80',
'User-Agent': 'test'
})], app._calls)
self.assertEqual({}, app.unread_requests)
self.assertEqual({}, app.unclosed_requests)

def test_upload_object_plumbing(self):
class InternalClient(internal_client.InternalClient):
def __init__(self, test, path, headers, fobj):
self.test = test
Expand Down

0 comments on commit 26c3d81

Please sign in to comment.