OpenStack Object Storage (swift)

Merge lp:~dweimer/swift/lp837428 into lp:~hudson-openstack/swift/trunk

  1. lp837428
  2. Merge into trunk
Proposed by Doug Weimer
Status: Merged
Approved by: John Dickinson
Approved revision: 350
Merged at revision: 351
Proposed branch: lp:~dweimer/swift/lp837428
Merge into: lp:~hudson-openstack/swift/trunk
Diff against target: 170 lines (+25/-15)
2 files modified
swift/proxy/server.py (+6/-4)
test/unit/proxy/test_server.py (+19/-11)
To merge this branch: bzr merge lp:~dweimer/swift/lp837428
Reviewer Review Type Date Requested Status
John Dickinson Approve
gholt (community) Approve
Review via email: mp+73429@code.launchpad.net

Description of the change

Proposed fix for bug #837428.

To test:
Create an object without an x-timestamp or x-put-timestamp header and request the object with the X-newest header.

Without the patch, the proxy will return a 404 object not found even though the storage nodes return the object to the proxy.

With the patch, the proxy will return the first object received unless an object with a newer timestamp header exists.

To post a comment you must log in.
Revision history for this message
gholt (gholt) wrote :

I've proposed a merge to your branch that adds a test that would fail if your code wasn't present and does a quick PEP8 update. The merge proposal is at https://code.launchpad.net/~gholt/swift/lp837428/+merge/73439 and you can merge it into your branch by 'cd <branch>; bzr merge lp:~gholt/swift/lp837428' and then push when you're ready.

Also, if you could ensure you or your company has signed the CLA http://wiki.openstack.org/HowToContribute and add your name and info to http://wiki.openstack.org/Contributors

Thanks!

review: Needs Fixing
lp:~dweimer/swift/lp837428 updated
350. By Doug Weimer

PEP8 fixes and unit tests merged from https://code.launchpad.net/~gholt/swift/lp837428/+merge/73439 . Added additional unittests for GET in case it is handled by a separate code path from HEAD in the future.

Revision history for this message
gholt (gholt) wrote :

:)

review: Approve
Revision history for this message
John Dickinson (notmyname) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'swift/proxy/server.py'
--- swift/proxy/server.py 2011-08-19 22:14:43 +0000
+++ swift/proxy/server.py 2011-08-31 13:11:36 +0000
@@ -645,13 +645,15 @@
645 possible_source.status in (200, 206)) or \645 possible_source.status in (200, 206)) or \
646 200 <= possible_source.status <= 399:646 200 <= possible_source.status <= 399:
647 if newest:647 if newest:
648 ts = 0
649 if source:648 if source:
650 ts = float(source.getheader('x-put-timestamp') or649 ts = float(source.getheader('x-put-timestamp') or
651 source.getheader('x-timestamp') or 0)650 source.getheader('x-timestamp') or 0)
652 pts = float(possible_source.getheader('x-put-timestamp') or651 pts = float(
653 possible_source.getheader('x-timestamp') or 0)652 possible_source.getheader('x-put-timestamp') or
654 if pts > ts:653 possible_source.getheader('x-timestamp') or 0)
654 if pts > ts:
655 source = possible_source
656 else:
655 source = possible_source657 source = possible_source
656 continue658 continue
657 else:659 else:
658660
=== modified file 'test/unit/proxy/test_server.py'
--- test/unit/proxy/test_server.py 2011-08-22 15:22:50 +0000
+++ test/unit/proxy/test_server.py 2011-08-31 13:11:36 +0000
@@ -182,6 +182,8 @@
182 self.etag or '"68b329da9893e34099c7d8ad5cb9c940"',182 self.etag or '"68b329da9893e34099c7d8ad5cb9c940"',
183 'x-works': 'yes',183 'x-works': 'yes',
184 }184 }
185 if not self.timestamp:
186 del headers['x-timestamp']
185 try:187 try:
186 if container_ts_iter.next() is False:188 if container_ts_iter.next() is False:
187 headers['x-container-timestamp'] = '1'189 headers['x-container-timestamp'] = '1'
@@ -321,8 +323,8 @@
321 proxy_server.http_connect = orig_http_connect323 proxy_server.http_connect = orig_http_connect
322 proxy_server.Controller.account_info = orig_account_info324 proxy_server.Controller.account_info = orig_account_info
323325
326
324# tests327# tests
325
326class TestController(unittest.TestCase):328class TestController(unittest.TestCase):
327329
328 def setUp(self):330 def setUp(self):
@@ -355,7 +357,8 @@
355 partition, nodes = self.controller.account_info(self.account)357 partition, nodes = self.controller.account_info(self.account)
356 proxy_server.http_connect = fake_http_connect(201,358 proxy_server.http_connect = fake_http_connect(201,
357 raise_timeout_exc=True)359 raise_timeout_exc=True)
358 self.controller._make_request(nodes, partition, 'POST','/','','')360 self.controller._make_request(nodes, partition, 'POST',
361 '/', '', '')
359362
360 # tests if 200 is cached and used363 # tests if 200 is cached and used
361 def test_account_info_200(self):364 def test_account_info_200(self):
@@ -713,7 +716,6 @@
713 object_ring=FakeRing())716 object_ring=FakeRing())
714 monkey_patch_mimetools()717 monkey_patch_mimetools()
715718
716
717 def tearDown(self):719 def tearDown(self):
718 proxy_server.CONTAINER_LISTING_LIMIT = _orig_container_listing_limit720 proxy_server.CONTAINER_LISTING_LIMIT = _orig_container_listing_limit
719721
@@ -758,6 +760,7 @@
758 'text/html', 'text/html']))760 'text/html', 'text/html']))
759 test_content_type('test.css', iter(['', '', '', 'text/css',761 test_content_type('test.css', iter(['', '', '', 'text/css',
760 'text/css', 'text/css']))762 'text/css', 'text/css']))
763
761 def test_custom_mime_types_files(self):764 def test_custom_mime_types_files(self):
762 swift_dir = mkdtemp()765 swift_dir = mkdtemp()
763 try:766 try:
@@ -1079,6 +1082,8 @@
1079 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')1082 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')
1080 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')1083 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')
1081 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')1084 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
1085 test_status_map((200, 200, 200), 200, (None, None, None), None)
1086 test_status_map((200, 200, 200), 200, (None, None, '1'), '1')
10821087
1083 def test_GET_newest(self):1088 def test_GET_newest(self):
1084 with save_globals():1089 with save_globals():
@@ -1102,6 +1107,8 @@
1102 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')1107 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')
1103 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')1108 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')
1104 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')1109 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
1110 test_status_map((200, 200, 200), 200, (None, None, None), None)
1111 test_status_map((200, 200, 200), 200, (None, None, '1'), '1')
11051112
1106 with save_globals():1113 with save_globals():
1107 controller = proxy_server.ObjectController(self.app, 'account',1114 controller = proxy_server.ObjectController(self.app, 'account',
@@ -1124,6 +1131,7 @@
1124 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '1')1131 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '1')
1125 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '1')1132 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '1')
1126 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')1133 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
1134 test_status_map((200, 200, 200), 200, (None, '1', '2'), None)
11271135
1128 def test_POST_meta_val_len(self):1136 def test_POST_meta_val_len(self):
1129 with save_globals():1137 with save_globals():
@@ -1433,7 +1441,7 @@
1433 resp = controller.best_response(req, [200] * 3, ['OK'] * 3, [''] * 3,1441 resp = controller.best_response(req, [200] * 3, ['OK'] * 3, [''] * 3,
1434 'Object', etag='68b329da9893e34099c7d8ad5cb9c940')1442 'Object', etag='68b329da9893e34099c7d8ad5cb9c940')
1435 self.assertEquals(resp.etag, '68b329da9893e34099c7d8ad5cb9c940')1443 self.assertEquals(resp.etag, '68b329da9893e34099c7d8ad5cb9c940')
1436 1444
1437 def test_proxy_passes_content_type(self):1445 def test_proxy_passes_content_type(self):
1438 with save_globals():1446 with save_globals():
1439 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})1447 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})
@@ -2181,7 +2189,7 @@
2181 self.assert_('\r\nContent-Length: 0\r\n' in headers)2189 self.assert_('\r\nContent-Length: 0\r\n' in headers)
21822190
2183 def test_client_ip_logging(self):2191 def test_client_ip_logging(self):
2184 # test that the client ip field in the log gets populated with the 2192 # test that the client ip field in the log gets populated with the
2185 # ip instead of being blank2193 # ip instead of being blank
2186 (prosrv, acc1srv, acc2srv, con2srv, con2srv, obj1srv, obj2srv) = \2194 (prosrv, acc1srv, acc2srv, con2srv, con2srv, obj1srv, obj2srv) = \
2187 _test_servers2195 _test_servers
@@ -2734,7 +2742,7 @@
2734 self.assert_(res.client_disconnect)2742 self.assert_(res.client_disconnect)
2735 finally:2743 finally:
2736 self.app.object_chunk_size = orig_object_chunk_size2744 self.app.object_chunk_size = orig_object_chunk_size
2737 2745
2738 def test_response_get_accept_ranges_header(self):2746 def test_response_get_accept_ranges_header(self):
2739 with save_globals():2747 with save_globals():
2740 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})2748 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})
@@ -2756,7 +2764,7 @@
2756 resp = controller.HEAD(req)2764 resp = controller.HEAD(req)
2757 self.assert_('accept-ranges' in resp.headers)2765 self.assert_('accept-ranges' in resp.headers)
2758 self.assertEquals(resp.headers['accept-ranges'], 'bytes')2766 self.assertEquals(resp.headers['accept-ranges'], 'bytes')
2759 2767
2760 def test_GET_calls_authorize(self):2768 def test_GET_calls_authorize(self):
2761 called = [False]2769 called = [False]
27622770
@@ -3137,7 +3145,7 @@
3137 res = controller.HEAD(req)3145 res = controller.HEAD(req)
3138 self.assert_('accept-ranges' in res.headers)3146 self.assert_('accept-ranges' in res.headers)
3139 self.assertEqual(res.headers['accept-ranges'], 'bytes')3147 self.assertEqual(res.headers['accept-ranges'], 'bytes')
3140 3148
3141 def test_PUT_metadata(self):3149 def test_PUT_metadata(self):
3142 self.metadata_helper('PUT')3150 self.metadata_helper('PUT')
31433151
@@ -3473,7 +3481,7 @@
3473 res.body3481 res.body
3474 self.assert_(hasattr(res, 'bytes_transferred'))3482 self.assert_(hasattr(res, 'bytes_transferred'))
3475 self.assertEquals(res.bytes_transferred, 2)3483 self.assertEquals(res.bytes_transferred, 2)
3476 3484
3477 def test_response_get_accept_ranges_header(self):3485 def test_response_get_accept_ranges_header(self):
3478 with save_globals():3486 with save_globals():
3479 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')3487 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')
@@ -3483,7 +3491,7 @@
3483 res = controller.GET(req)3491 res = controller.GET(req)
3484 self.assert_('accept-ranges' in res.headers)3492 self.assert_('accept-ranges' in res.headers)
3485 self.assertEqual(res.headers['accept-ranges'], 'bytes')3493 self.assertEqual(res.headers['accept-ranges'], 'bytes')
3486 3494
3487 def test_response_head_accept_ranges_header(self):3495 def test_response_head_accept_ranges_header(self):
3488 with save_globals():3496 with save_globals():
3489 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')3497 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')
@@ -3494,7 +3502,7 @@
3494 res.body3502 res.body
3495 self.assert_('accept-ranges' in res.headers)3503 self.assert_('accept-ranges' in res.headers)
3496 self.assertEqual(res.headers['accept-ranges'], 'bytes')3504 self.assertEqual(res.headers['accept-ranges'], 'bytes')
3497 3505
3498 def test_response_client_disconnect_attr(self):3506 def test_response_client_disconnect_attr(self):
3499 with save_globals():3507 with save_globals():
3500 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')3508 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')