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
1=== modified file 'swift/proxy/server.py'
2--- swift/proxy/server.py 2011-08-19 22:14:43 +0000
3+++ swift/proxy/server.py 2011-08-31 13:11:36 +0000
4@@ -645,13 +645,15 @@
5 possible_source.status in (200, 206)) or \
6 200 <= possible_source.status <= 399:
7 if newest:
8- ts = 0
9 if source:
10 ts = float(source.getheader('x-put-timestamp') or
11 source.getheader('x-timestamp') or 0)
12- pts = float(possible_source.getheader('x-put-timestamp') or
13- possible_source.getheader('x-timestamp') or 0)
14- if pts > ts:
15+ pts = float(
16+ possible_source.getheader('x-put-timestamp') or
17+ possible_source.getheader('x-timestamp') or 0)
18+ if pts > ts:
19+ source = possible_source
20+ else:
21 source = possible_source
22 continue
23 else:
24
25=== modified file 'test/unit/proxy/test_server.py'
26--- test/unit/proxy/test_server.py 2011-08-22 15:22:50 +0000
27+++ test/unit/proxy/test_server.py 2011-08-31 13:11:36 +0000
28@@ -182,6 +182,8 @@
29 self.etag or '"68b329da9893e34099c7d8ad5cb9c940"',
30 'x-works': 'yes',
31 }
32+ if not self.timestamp:
33+ del headers['x-timestamp']
34 try:
35 if container_ts_iter.next() is False:
36 headers['x-container-timestamp'] = '1'
37@@ -321,8 +323,8 @@
38 proxy_server.http_connect = orig_http_connect
39 proxy_server.Controller.account_info = orig_account_info
40
41+
42 # tests
43-
44 class TestController(unittest.TestCase):
45
46 def setUp(self):
47@@ -355,7 +357,8 @@
48 partition, nodes = self.controller.account_info(self.account)
49 proxy_server.http_connect = fake_http_connect(201,
50 raise_timeout_exc=True)
51- self.controller._make_request(nodes, partition, 'POST','/','','')
52+ self.controller._make_request(nodes, partition, 'POST',
53+ '/', '', '')
54
55 # tests if 200 is cached and used
56 def test_account_info_200(self):
57@@ -713,7 +716,6 @@
58 object_ring=FakeRing())
59 monkey_patch_mimetools()
60
61-
62 def tearDown(self):
63 proxy_server.CONTAINER_LISTING_LIMIT = _orig_container_listing_limit
64
65@@ -758,6 +760,7 @@
66 'text/html', 'text/html']))
67 test_content_type('test.css', iter(['', '', '', 'text/css',
68 'text/css', 'text/css']))
69+
70 def test_custom_mime_types_files(self):
71 swift_dir = mkdtemp()
72 try:
73@@ -1079,6 +1082,8 @@
74 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')
75 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')
76 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
77+ test_status_map((200, 200, 200), 200, (None, None, None), None)
78+ test_status_map((200, 200, 200), 200, (None, None, '1'), '1')
79
80 def test_GET_newest(self):
81 with save_globals():
82@@ -1102,6 +1107,8 @@
83 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '3')
84 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '3')
85 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
86+ test_status_map((200, 200, 200), 200, (None, None, None), None)
87+ test_status_map((200, 200, 200), 200, (None, None, '1'), '1')
88
89 with save_globals():
90 controller = proxy_server.ObjectController(self.app, 'account',
91@@ -1124,6 +1131,7 @@
92 test_status_map((200, 200, 200), 200, ('1', '3', '2'), '1')
93 test_status_map((200, 200, 200), 200, ('1', '3', '1'), '1')
94 test_status_map((200, 200, 200), 200, ('3', '3', '1'), '3')
95+ test_status_map((200, 200, 200), 200, (None, '1', '2'), None)
96
97 def test_POST_meta_val_len(self):
98 with save_globals():
99@@ -1433,7 +1441,7 @@
100 resp = controller.best_response(req, [200] * 3, ['OK'] * 3, [''] * 3,
101 'Object', etag='68b329da9893e34099c7d8ad5cb9c940')
102 self.assertEquals(resp.etag, '68b329da9893e34099c7d8ad5cb9c940')
103-
104+
105 def test_proxy_passes_content_type(self):
106 with save_globals():
107 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})
108@@ -2181,7 +2189,7 @@
109 self.assert_('\r\nContent-Length: 0\r\n' in headers)
110
111 def test_client_ip_logging(self):
112- # test that the client ip field in the log gets populated with the
113+ # test that the client ip field in the log gets populated with the
114 # ip instead of being blank
115 (prosrv, acc1srv, acc2srv, con2srv, con2srv, obj1srv, obj2srv) = \
116 _test_servers
117@@ -2734,7 +2742,7 @@
118 self.assert_(res.client_disconnect)
119 finally:
120 self.app.object_chunk_size = orig_object_chunk_size
121-
122+
123 def test_response_get_accept_ranges_header(self):
124 with save_globals():
125 req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})
126@@ -2756,7 +2764,7 @@
127 resp = controller.HEAD(req)
128 self.assert_('accept-ranges' in resp.headers)
129 self.assertEquals(resp.headers['accept-ranges'], 'bytes')
130-
131+
132 def test_GET_calls_authorize(self):
133 called = [False]
134
135@@ -3137,7 +3145,7 @@
136 res = controller.HEAD(req)
137 self.assert_('accept-ranges' in res.headers)
138 self.assertEqual(res.headers['accept-ranges'], 'bytes')
139-
140+
141 def test_PUT_metadata(self):
142 self.metadata_helper('PUT')
143
144@@ -3473,7 +3481,7 @@
145 res.body
146 self.assert_(hasattr(res, 'bytes_transferred'))
147 self.assertEquals(res.bytes_transferred, 2)
148-
149+
150 def test_response_get_accept_ranges_header(self):
151 with save_globals():
152 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')
153@@ -3483,7 +3491,7 @@
154 res = controller.GET(req)
155 self.assert_('accept-ranges' in res.headers)
156 self.assertEqual(res.headers['accept-ranges'], 'bytes')
157-
158+
159 def test_response_head_accept_ranges_header(self):
160 with save_globals():
161 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')
162@@ -3494,7 +3502,7 @@
163 res.body
164 self.assert_('accept-ranges' in res.headers)
165 self.assertEqual(res.headers['accept-ranges'], 'bytes')
166-
167+
168 def test_response_client_disconnect_attr(self):
169 with save_globals():
170 proxy_server.http_connect = fake_http_connect(200, 200, body='{}')