openstack/swift
Code Issues Proposed changes

Remove fake _get_part and use the real thing

Browse Source 
The get_part method is fast and stable given a consistent hash path
suffix/prefix, so there's no absolute requirement for the fake
implementation other than convenience.  OTOH, removing the fake
implementation and fixing the tests that were relying on it should make
it easier to write better tests going forward and harder to hide bugs
that don't show up when using the fakes.

There may be some overhead when writing new tests that use the ring if
you're making assertions on partitions or paths, but with a part power
of zero it's normally trivially obvious when a 1 needs to be a 0 or vice
versa.  Or you can just drop the assertions around the parts you were
faking anyway.

Change-Id: I8bfc388a04eff6491038991cdfd7686c9d961545
This commit is contained in:
Clay Gerrard 2014-07-01 11:23:48 -07:00 committed by Christian Schwede
parent 2f45600c7d
commit e114a628dc
3 changed files with 46 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
test/unit/__init__.py
View File

@ -139,13 +139,6 @@ class FakeRing(Ring):
self._part_shift = 32 - part_power
self._reload()
def get_part(self, *args, **kwargs):
# always call the real method, even if the fake ignores the result
real_part = super(FakeRing, self).get_part(*args, **kwargs)
if self._part_shift == 32:
return 1
return real_part
def _reload(self):
self._rtime = time.time()

2
test/unit/account/test_reaper.py
View File

@ -294,7 +294,7 @@ class TestReaper(unittest.TestCase):
'X-Backend-Storage-Policy-Index': policy.idx
}
ring = r.get_object_ring(policy.idx)
expected = call(ring.devs[i], 1, 'a', 'c', 'o',
expected = call(ring.devs[i], 0, 'a', 'c', 'o',
headers=headers, conn_timeout=0.5,
response_timeout=10)
self.assertEqual(call_args, expected)

88
test/unit/proxy/test_server.py
View File

@ -1728,15 +1728,17 @@ class TestObjectController(unittest.TestCase):
req_method, req_path, req_headers = req
self.assertEqual(method, req_method)
# caller can ignore leading path parts
self.assertTrue(req_path.endswith(path))
self.assertTrue(req_path.endswith(path),
'expected path to end with %s, it was %s' % (
path, req_path))
headers = headers or {}
# caller can ignore some headers
for k, v in headers.items():
self.assertEqual(req_headers[k], v)
account_request = backend_requests.pop(0)
check_request(account_request, method='HEAD', path='/sda/1/a')
check_request(account_request, method='HEAD', path='/sda/0/a')
container_request = backend_requests.pop(0)
check_request(container_request, method='HEAD', path='/sda/1/a/c')
check_request(container_request, method='HEAD', path='/sda/0/a/c')
# make sure backend requests included expected container headers
container_headers = {}
for request in backend_requests:
@ -1746,9 +1748,9 @@ class TestObjectController(unittest.TestCase):
container_headers[device] = host
expectations = {
'method': 'POST',
'path': '/1/a/c/o',
'path': '/0/a/c/o',
'headers': {
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'Connection': 'close',
'User-Agent': 'proxy-server %s' % os.getpid(),
'Host': 'localhost:80',
@ -1781,7 +1783,7 @@ class TestObjectController(unittest.TestCase):
for request in backend_requests[2:]:
expectations = {
'method': 'POST',
'path': '/1/a/c/o', # ignore device bit
'path': '/0/a/c/o', # ignore device bit
'headers': {
'X-Object-Meta-Color': 'Blue',
'X-Backend-Storage-Policy-Index': '0',
@ -1808,17 +1810,17 @@ class TestObjectController(unittest.TestCase):
policy1 = {'X-Backend-Storage-Policy-Index': '1'}
expected = [
# account info
{'method': 'HEAD', 'path': '/1/a'},
{'method': 'HEAD', 'path': '/0/a'},
# container info
{'method': 'HEAD', 'path': '/1/a/c'},
{'method': 'HEAD', 'path': '/0/a/c'},
# x-newests
{'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1},
{'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1},
{'method': 'GET', 'path': '/1/a/c/o', 'headers': policy1},
{'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1},
{'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1},
{'method': 'GET', 'path': '/0/a/c/o', 'headers': policy1},
# new writes
{'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0},
{'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0},
{'method': 'PUT', 'path': '/1/a/c/o', 'headers': policy0},
{'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0},
{'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0},
{'method': 'PUT', 'path': '/0/a/c/o', 'headers': policy0},
]
for request, expectations in zip(backend_requests, expected):
check_request(request, **expectations)
@ -4664,13 +4666,13 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(
seen_headers, [
{'X-Container-Host': '10.0.0.0:1000',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sda'},
{'X-Container-Host': '10.0.0.1:1001',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdb'},
{'X-Container-Host': '10.0.0.2:1002',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdc'}])
def test_PUT_x_container_headers_with_fewer_container_replicas(self):
@ -4686,10 +4688,10 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(
seen_headers, [
{'X-Container-Host': '10.0.0.0:1000',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sda'},
{'X-Container-Host': '10.0.0.1:1001',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdb'},
{'X-Container-Host': None,
'X-Container-Partition': None,
@ -4708,13 +4710,13 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(
seen_headers, [
{'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sda,sdd'},
{'X-Container-Host': '10.0.0.1:1001',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdb'},
{'X-Container-Host': '10.0.0.2:1002',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdc'}])
def test_POST_x_container_headers_with_more_container_replicas(self):
@ -4732,13 +4734,13 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(
seen_headers, [
{'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sda,sdd'},
{'X-Container-Host': '10.0.0.1:1001',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdb'},
{'X-Container-Host': '10.0.0.2:1002',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdc'}])
def test_DELETE_x_container_headers_with_more_container_replicas(self):
@ -4754,13 +4756,13 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(seen_headers, [
{'X-Container-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sda,sdd'},
{'X-Container-Host': '10.0.0.1:1001',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdb'},
{'X-Container-Host': '10.0.0.2:1002',
'X-Container-Partition': '1',
'X-Container-Partition': '0',
'X-Container-Device': 'sdc'}
])
@ -4787,11 +4789,11 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(seen_headers, [
{'X-Delete-At-Host': '10.0.0.0:1000',
'X-Delete-At-Container': delete_at_container,
'X-Delete-At-Partition': '1',
'X-Delete-At-Partition': '0',
'X-Delete-At-Device': 'sda'},
{'X-Delete-At-Host': '10.0.0.1:1001',
'X-Delete-At-Container': delete_at_container,
'X-Delete-At-Partition': '1',
'X-Delete-At-Partition': '0',
'X-Delete-At-Device': 'sdb'},
{'X-Delete-At-Host': None,
'X-Delete-At-Container': None,
@ -4823,15 +4825,15 @@ class TestObjectController(unittest.TestCase):
self.assertEqual(seen_headers, [
{'X-Delete-At-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Delete-At-Container': delete_at_container,
'X-Delete-At-Partition': '1',
'X-Delete-At-Partition': '0',
'X-Delete-At-Device': 'sda,sdd'},
{'X-Delete-At-Host': '10.0.0.1:1001',
'X-Delete-At-Container': delete_at_container,
'X-Delete-At-Partition': '1',
'X-Delete-At-Partition': '0',
'X-Delete-At-Device': 'sdb'},
{'X-Delete-At-Host': '10.0.0.2:1002',
'X-Delete-At-Container': delete_at_container,
'X-Delete-At-Partition': '1',
'X-Delete-At-Partition': '0',
'X-Delete-At-Device': 'sdc'}
])
@ -5827,10 +5829,10 @@ class TestContainerController(unittest.TestCase):
200, 201, 201, 201) # HEAD PUT PUT PUT
self.assertEqual(seen_headers, [
{'X-Account-Host': '10.0.0.0:1000',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sda'},
{'X-Account-Host': '10.0.0.1:1001',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdb'},
{'X-Account-Host': None,
'X-Account-Partition': None,
@ -5847,13 +5849,13 @@ class TestContainerController(unittest.TestCase):
200, 201, 201, 201) # HEAD PUT PUT PUT
self.assertEqual(seen_headers, [
{'X-Account-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sda,sdd'},
{'X-Account-Host': '10.0.0.1:1001',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdb'},
{'X-Account-Host': '10.0.0.2:1002',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdc'}
])
@ -5867,10 +5869,10 @@ class TestContainerController(unittest.TestCase):
200, 204, 204, 204) # HEAD DELETE DELETE DELETE
self.assertEqual(seen_headers, [
{'X-Account-Host': '10.0.0.0:1000',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sda'},
{'X-Account-Host': '10.0.0.1:1001',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdb'},
{'X-Account-Host': None,
'X-Account-Partition': None,
@ -5887,13 +5889,13 @@ class TestContainerController(unittest.TestCase):
200, 204, 204, 204) # HEAD DELETE DELETE DELETE
self.assertEqual(seen_headers, [
{'X-Account-Host': '10.0.0.0:1000,10.0.0.3:1003',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sda,sdd'},
{'X-Account-Host': '10.0.0.1:1001',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdb'},
{'X-Account-Host': '10.0.0.2:1002',
'X-Account-Partition': '1',
'X-Account-Partition': '0',
'X-Account-Device': 'sdc'}
])