Merge lp:~mandel/ubuntuone-client/add-watch-deferred into lp:ubuntuone-client
- add-watch-deferred
- Merge into trunk
Proposed by
Manuel de la Peña
Status: | Merged |
---|---|
Approved by: | Alejandro J. Cura |
Approved revision: | 1119 |
Merged at revision: | 1081 |
Proposed branch: | lp:~mandel/ubuntuone-client/add-watch-deferred |
Merge into: | lp:ubuntuone-client |
Prerequisite: | lp:~mandel/ubuntuone-client/fix-fsm |
Diff against target: |
446 lines (+74/-44) 6 files modified
tests/platform/test_filesystem_notifications.py (+40/-20) tests/syncdaemon/test_vm.py (+15/-11) ubuntuone/platform/linux/filesystem_notifications.py (+2/-2) ubuntuone/platform/windows/filesystem_notifications.py (+4/-2) ubuntuone/syncdaemon/local_rescan.py (+11/-7) ubuntuone/syncdaemon/volume_manager.py (+2/-2) |
To merge this branch: | bzr merge lp:~mandel/ubuntuone-client/add-watch-deferred |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alejandro J. Cura (community) | Approve | ||
Natalia Bidart (community) | Approve | ||
Review via email: mp+69816@code.launchpad.net |
Commit message
Changed add_watch to return a deferred so that we will be waiting for a successful callback before we assume that the watch is running.
Description of the change
Changed add_watch to return a deferred so that we will be waiting for a successful callback before we assume that the watch is running.
To post a comment you must log in.
- 1117. By Manuel de la Peña
-
Merged fix-fsm into add-watch-deferred.
- 1118. By Manuel de la Peña
-
Merged fix-fsm into add-watch-deferred.
- 1119. By Manuel de la Peña
-
Merged fix-fsm into add-watch-deferred.
Revision history for this message
Alejandro J. Cura (alecu) wrote : | # |
Code looks fine, and all tests pass!
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'tests/platform/test_filesystem_notifications.py' |
2 | --- tests/platform/test_filesystem_notifications.py 2011-07-28 14:51:42 +0000 |
3 | +++ tests/platform/test_filesystem_notifications.py 2011-07-29 17:35:26 +0000 |
4 | @@ -239,6 +239,7 @@ |
5 | |
6 | @skipIfOS('win32', 'There is no reasonable way to know when a file is ' |
7 | + 'opened on windows.') |
8 | + @defer.inlineCallbacks |
9 | def test_file_open(self): |
10 | """Test receiving the open signal on files.""" |
11 | testfile = os.path.join(self.root_dir, "foo") |
12 | @@ -246,15 +247,17 @@ |
13 | self.monitor.add_to_mute_filter("FS_FILE_OPEN", path=testfile) |
14 | self.monitor.add_to_mute_filter("FS_FILE_CLOSE_NOWRITE", path=testfile) |
15 | self.assertEqual(self._how_many_muted(), 2) |
16 | - self.monitor.add_watch(self.root_dir) |
17 | + yield self.monitor.add_watch(self.root_dir) |
18 | |
19 | # generate the event |
20 | open(testfile) |
21 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
22 | - return self._deferred |
23 | + test_result = yield self._deferred |
24 | + defer.returnValue(test_result) |
25 | |
26 | @skipIfOS('win32', 'There is no reasonable way to know when a file was ' |
27 | 'opened on windows') |
28 | + @defer.inlineCallbacks |
29 | def test_file_close_nowrite(self): |
30 | """Test receiving the close_nowrite signal on files.""" |
31 | testfile = os.path.join(self.root_dir, "foo") |
32 | @@ -262,13 +265,15 @@ |
33 | fh = open(testfile) |
34 | self.monitor.add_to_mute_filter("FS_FILE_CLOSE_NOWRITE", path=testfile) |
35 | self.assertEqual(self._how_many_muted(), 1) |
36 | - self.monitor.add_watch(self.root_dir) |
37 | + yield self.monitor.add_watch(self.root_dir) |
38 | |
39 | # generate the event |
40 | fh.close() |
41 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
42 | - return self._deferred |
43 | + test_result = yield self._deferred |
44 | + defer.returnValue(test_result) |
45 | |
46 | + @defer.inlineCallbacks |
47 | def test_file_create_close_write(self): |
48 | """Test receiving the create and close_write signals on files.""" |
49 | testfile = os.path.join(self.root_dir, "foo") |
50 | @@ -276,53 +281,61 @@ |
51 | self.monitor.add_to_mute_filter("FS_FILE_OPEN", path=testfile) |
52 | self.monitor.add_to_mute_filter("FS_FILE_CLOSE_WRITE", path=testfile) |
53 | self.assertEqual(self._how_many_muted(), 3) |
54 | - self.monitor.add_watch(self.root_dir) |
55 | + yield self.monitor.add_watch(self.root_dir) |
56 | |
57 | # generate the event |
58 | fd = open(testfile, "w") |
59 | fd.write('test') |
60 | fd.close() |
61 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
62 | - return self._deferred |
63 | + test_result = yield self._deferred |
64 | + defer.returnValue(test_result) |
65 | |
66 | + @defer.inlineCallbacks |
67 | def test_dir_create(self): |
68 | """Test receiving the create signal on dirs.""" |
69 | testdir = os.path.join(self.root_dir, "foo") |
70 | self.monitor.add_to_mute_filter("FS_DIR_CREATE", path=testdir) |
71 | self.assertEqual(self._how_many_muted(), 1) |
72 | - self.monitor.add_watch(self.root_dir) |
73 | + yield self.monitor.add_watch(self.root_dir) |
74 | |
75 | # generate the event |
76 | os.mkdir(testdir) |
77 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
78 | - return self._deferred |
79 | + test_result = yield self._deferred |
80 | + defer.returnValue(test_result) |
81 | |
82 | + @defer.inlineCallbacks |
83 | def test_file_delete(self): |
84 | """Test the delete signal on a file.""" |
85 | testfile = os.path.join(self.root_dir, "foo") |
86 | open(testfile, "w").close() |
87 | self.monitor.add_to_mute_filter("FS_FILE_DELETE", path=testfile) |
88 | self.assertEqual(self._how_many_muted(), 1) |
89 | - self.monitor.add_watch(self.root_dir) |
90 | + yield self.monitor.add_watch(self.root_dir) |
91 | |
92 | # generate the event |
93 | os.remove(testfile) |
94 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
95 | - return self._deferred |
96 | + test_result = yield self._deferred |
97 | + defer.returnValue(test_result) |
98 | |
99 | + @defer.inlineCallbacks |
100 | def test_dir_delete(self): |
101 | """Test the delete signal on a dir.""" |
102 | testdir = os.path.join(self.root_dir, "foo") |
103 | os.mkdir(testdir) |
104 | self.monitor.add_to_mute_filter("FS_DIR_DELETE", path=testdir) |
105 | self.assertEqual(self._how_many_muted(), 1) |
106 | - self.monitor.add_watch(self.root_dir) |
107 | + yield self.monitor.add_watch(self.root_dir) |
108 | |
109 | # generate the event |
110 | os.rmdir(testdir) |
111 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
112 | - return self._deferred |
113 | + test_result = yield self._deferred |
114 | + defer.returnValue(test_result) |
115 | |
116 | + @defer.inlineCallbacks |
117 | def test_file_moved_inside(self): |
118 | """Test the synthesis of the FILE_MOVE event.""" |
119 | fromfile = os.path.join(self.root_dir, "foo") |
120 | @@ -335,13 +348,15 @@ |
121 | self.monitor.add_to_mute_filter("FS_FILE_MOVE", |
122 | path_from=fromfile, path_to=tofile) |
123 | self.assertEqual(self._how_many_muted(), 1) |
124 | - self.monitor.add_watch(self.root_dir) |
125 | + yield self.monitor.add_watch(self.root_dir) |
126 | |
127 | # generate the event |
128 | os.rename(fromfile, tofile) |
129 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
130 | - return self._deferred |
131 | + test_result = yield self._deferred |
132 | + defer.returnValue(test_result) |
133 | |
134 | + @defer.inlineCallbacks |
135 | def test_dir_moved_inside(self): |
136 | """Test the synthesis of the DIR_MOVE event.""" |
137 | fromdir = os.path.join(self.root_dir, "foo") |
138 | @@ -354,13 +369,15 @@ |
139 | self.monitor.add_to_mute_filter("FS_DIR_MOVE", |
140 | path_from=fromdir, path_to=todir) |
141 | self.assertEqual(self._how_many_muted(), 1) |
142 | - self.monitor.add_watch(self.root_dir) |
143 | + yield self.monitor.add_watch(self.root_dir) |
144 | |
145 | # generate the event |
146 | os.rename(fromdir, todir) |
147 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
148 | - return self._deferred |
149 | + test_result = yield self._deferred |
150 | + defer.returnValue(test_result) |
151 | |
152 | + @defer.inlineCallbacks |
153 | def test_file_moved_from_conflict(self): |
154 | """Test the handling of the FILE_MOVE event when source is conflict.""" |
155 | fromfile = os.path.join(self.root_dir, "foo.u1conflict") |
156 | @@ -373,13 +390,15 @@ |
157 | self.monitor.add_to_mute_filter("FS_FILE_MOVE", |
158 | path_from=fromfile, path_to=tofile) |
159 | self.assertEqual(self._how_many_muted(), 2) |
160 | - self.monitor.add_watch(self.root_dir) |
161 | + yield self.monitor.add_watch(self.root_dir) |
162 | |
163 | # generate the event |
164 | os.rename(fromfile, tofile) |
165 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
166 | - return self._deferred |
167 | + test_result = yield self._deferred |
168 | + defer.returnValue(test_result) |
169 | |
170 | + @defer.inlineCallbacks |
171 | def test_file_moved_from_partial(self): |
172 | """Test the handling of the FILE_MOVE event when source is partial.""" |
173 | fromfile = os.path.join(self.root_dir, "mdid.u1partial.foo") |
174 | @@ -390,9 +409,10 @@ |
175 | self.monitor.add_to_mute_filter("FS_FILE_CREATE", path=tofile) |
176 | self.monitor.add_to_mute_filter("FS_FILE_CLOSE_WRITE", path=tofile) |
177 | self.assertEqual(self._how_many_muted(), 2) |
178 | - self.monitor.add_watch(root_dir) |
179 | + yield self.monitor.add_watch(root_dir) |
180 | |
181 | # generate the event |
182 | os.rename(fromfile, tofile) |
183 | reactor.callLater(self.timeout - 0.2, self.check_filter) |
184 | - return self._deferred |
185 | + test_result = yield self._deferred |
186 | + defer.returnValue(test_result) |
187 | |
188 | === modified file 'tests/syncdaemon/test_vm.py' |
189 | --- tests/syncdaemon/test_vm.py 2011-07-28 15:43:05 +0000 |
190 | +++ tests/syncdaemon/test_vm.py 2011-07-29 17:35:26 +0000 |
191 | @@ -421,7 +421,7 @@ |
192 | self.main.fs.create(path, share.volume_id, is_dir=True) |
193 | self.main.fs.set_node_id(path, 'dir_node_id'+str(i)) |
194 | # add a inotify watch to the dir |
195 | - self.main.event_q.add_watch(path) |
196 | + yield self.main.event_q.add_watch(path) |
197 | files = ['a_file', os.path.join('dir', 'file'), |
198 | os.path.join('dir','subdir','file')] |
199 | for i, file in enumerate(files): |
200 | @@ -824,15 +824,17 @@ |
201 | self.assertEquals('fake_share_uuid', share.node_id) |
202 | self.assertEquals(None, self.vm.shares.get('share_id_1')) |
203 | |
204 | + @defer.inlineCallbacks |
205 | def test_remove_watch(self): |
206 | """Test for VolumeManager._remove_watch""" |
207 | path = os.path.join(self.root_dir, 'dir') |
208 | make_dir(path, recursive=True) |
209 | - self.main.event_q.add_watch(path) |
210 | + yield self.main.event_q.add_watch(path) |
211 | self.assertIn(path, self.main.event_q.monitor._general_watchs) |
212 | self.vm._remove_watch(path) |
213 | self.assertNotIn(path, self.main.event_q.monitor._general_watchs) |
214 | |
215 | + @defer.inlineCallbacks |
216 | def test_remove_watches(self): |
217 | """Test for VolumeManager._remove_watches""" |
218 | dirs = ['dir', os.path.join('dir', 'subdir'), 'emptydir'] |
219 | @@ -843,9 +845,9 @@ |
220 | make_dir(path, recursive=True) |
221 | self.main.fs.create(path, "", is_dir=True) |
222 | self.main.fs.set_node_id(path, 'dir_node_id'+str(i)) |
223 | - self.main.event_q.add_watch(path) |
224 | + yield self.main.event_q.add_watch(path) |
225 | # insert the root_dir in the list |
226 | - self.main.event_q.add_watch(self.root_dir) |
227 | + yield self.main.event_q.add_watch(self.root_dir) |
228 | paths.insert(0, self.root_dir) |
229 | for path in paths: |
230 | self.assertIn(path, self.main.event_q.monitor._general_watchs) |
231 | @@ -854,13 +856,14 @@ |
232 | for path in paths: |
233 | self.assertNotIn(path, self.main.event_q.monitor._general_watchs) |
234 | |
235 | + @defer.inlineCallbacks |
236 | def test_remove_watches_after_dir_rename(self): |
237 | """Test for VolumeManager._remove_watches after dir rename.""" |
238 | path = os.path.join(self.root_dir, 'testit') |
239 | os.mkdir(path) |
240 | self.main.fs.create(path, "", is_dir=True) |
241 | self.main.fs.set_node_id(path, 'dir_node_id') |
242 | - self.main.event_q.add_watch(path) |
243 | + yield self.main.event_q.add_watch(path) |
244 | |
245 | os.rename(path, path+'.old') |
246 | # remove the watches |
247 | @@ -869,13 +872,14 @@ |
248 | self.assertNotIn(path, self.main.event_q.monitor._general_watchs, |
249 | 'watch should not be present') |
250 | |
251 | + @defer.inlineCallbacks |
252 | def test_delete_fsm_object(self): |
253 | """Test for VolumeManager._delete_fsm_object""" |
254 | path = os.path.abspath(os.path.join(self.root_dir, 'dir')) |
255 | make_dir(path, recursive=True) |
256 | self.main.fs.create(path, "", is_dir=True) |
257 | self.main.fs.set_node_id(path, 'dir_node_id') |
258 | - self.main.event_q.add_watch(path) |
259 | + yield self.main.event_q.add_watch(path) |
260 | self.assertIn(path, self.main.event_q.monitor._general_watchs) |
261 | self.assertTrue(self.main.fs.get_by_path(path), path) |
262 | # remove the watch |
263 | @@ -1082,7 +1086,7 @@ |
264 | self.main.fs.create(path, share.volume_id, is_dir=True) |
265 | self.main.fs.set_node_id(path, 'dir_node_id'+str(i)) |
266 | # add a inotify watch to the dir |
267 | - self.main.event_q.add_watch(path) |
268 | + yield self.main.event_q.add_watch(path) |
269 | files = ['a_file', os.path.join('dir', 'file'), |
270 | os.path.join('dir','subdir','file')] |
271 | for i, file in enumerate(files): |
272 | @@ -1207,7 +1211,7 @@ |
273 | # add a inotify watch to the dirs |
274 | for path, is_dir in paths: |
275 | if is_dir: |
276 | - self.main.event_q.add_watch(path) |
277 | + yield self.main.event_q.add_watch(path) |
278 | self.assertEquals(len(paths), len(dirs+files)+1, paths) |
279 | |
280 | # unsubscribe from it |
281 | @@ -1420,7 +1424,7 @@ |
282 | self.main.fs.create(path, udf.volume_id, is_dir=True) |
283 | self.main.fs.set_node_id(path, 'dir_node_id'+str(i)) |
284 | # add a inotify watch to the dir |
285 | - self.main.event_q.add_watch(path) |
286 | + yield self.main.event_q.add_watch(path) |
287 | files = ['a_file', os.path.join('dir','file'), |
288 | os.path.join('dir','subdir','file')] |
289 | for i, file in enumerate(files): |
290 | @@ -2038,7 +2042,7 @@ |
291 | self.main.fs.create(path, udf.volume_id, is_dir=True) |
292 | self.main.fs.set_node_id(path, 'dir_node_id'+str(i)) |
293 | # add a inotify watch to the dir |
294 | - self.main.event_q.add_watch(path) |
295 | + yield self.main.event_q.add_watch(path) |
296 | files = ['a_file', os.path.join('dir','file'), |
297 | os.path.join('dir','subdir','file')] |
298 | for i, file in enumerate(files): |
299 | @@ -2095,7 +2099,7 @@ |
300 | # add a inotify watch to the dirs |
301 | for path, is_dir in paths: |
302 | if is_dir: |
303 | - self.main.event_q.add_watch(path) |
304 | + yield self.main.event_q.add_watch(path) |
305 | self.assertEquals(len(paths), len(dirs+files)+1, paths) |
306 | |
307 | # unsubscribe from it |
308 | |
309 | === modified file 'ubuntuone/platform/linux/filesystem_notifications.py' |
310 | --- ubuntuone/platform/linux/filesystem_notifications.py 2011-07-27 17:10:57 +0000 |
311 | +++ ubuntuone/platform/linux/filesystem_notifications.py 2011-07-29 17:35:26 +0000 |
312 | @@ -457,13 +457,13 @@ |
313 | # not add it if already there |
314 | if dirpath in w_dict: |
315 | self.log.debug("Watch already there for %r", dirpath) |
316 | - return False |
317 | + return defer.succeed(False) |
318 | |
319 | # add the watch! |
320 | self.log.debug("Adding %s inotify watch to %r", w_type, dirpath) |
321 | result = w_manager.add_watch(dirpath, events) |
322 | w_dict[dirpath] = result[dirpath] |
323 | - return True |
324 | + return defer.succeed(True) |
325 | |
326 | def inotify_watch_fix(self, pathfrom, pathto): |
327 | """Fix the path in inotify structures.""" |
328 | |
329 | === modified file 'ubuntuone/platform/windows/filesystem_notifications.py' |
330 | --- ubuntuone/platform/windows/filesystem_notifications.py 2011-07-29 17:35:26 +0000 |
331 | +++ ubuntuone/platform/windows/filesystem_notifications.py 2011-07-29 17:35:26 +0000 |
332 | @@ -695,8 +695,10 @@ |
333 | # is also watching it kids |
334 | if not self._watch_manager.get_wd(dirpath): |
335 | # we need to add a watch which will also watch its kids |
336 | - self._watch_manager.add_watch(dirpath, FILESYSTEM_MONITOR_MASK, |
337 | - auto_add=True) |
338 | + return self._watch_manager.add_watch(dirpath, |
339 | + FILESYSTEM_MONITOR_MASK, |
340 | + auto_add=True) |
341 | + return defer.succeed(True) |
342 | |
343 | def is_frozen(self): |
344 | """Checks if there's something frozen.""" |
345 | |
346 | === modified file 'ubuntuone/syncdaemon/local_rescan.py' |
347 | --- ubuntuone/syncdaemon/local_rescan.py 2011-07-27 17:10:57 +0000 |
348 | +++ ubuntuone/syncdaemon/local_rescan.py 2011-07-29 17:35:26 +0000 |
349 | @@ -235,6 +235,7 @@ |
350 | self._process_next_queue(None) |
351 | return self._previous_deferred |
352 | |
353 | + @defer.inlineCallbacks |
354 | def _add_watches_to_udf_ancestors(self, volume): |
355 | """Add a inotify watch to volume's ancestors if it's an UDF.""" |
356 | added_watches = [] |
357 | @@ -252,10 +253,10 @@ |
358 | if not access(ancestor): |
359 | log_info("Tree broken at path: %r", volume.path) |
360 | revert_watches() |
361 | - return False |
362 | + defer.returnValue(False) |
363 | |
364 | log_debug("Adding watch to UDF's ancestor %r", ancestor) |
365 | - really_added = self.eq.add_watch(ancestor) |
366 | + really_added = yield self.eq.add_watch(ancestor) |
367 | # only note it for the revert if the watch was not there before |
368 | if really_added: |
369 | added_watches.append(ancestor) |
370 | @@ -263,10 +264,10 @@ |
371 | # finally, check that UDF is ok in disk |
372 | if not access(volume.path): |
373 | revert_watches() |
374 | - return False |
375 | + defer.returnValue(False) |
376 | |
377 | # all is ok |
378 | - return True |
379 | + defer.returnValue(True) |
380 | |
381 | def _process_next_queue(self, _): |
382 | """Process the next item in the queue, if any.""" |
383 | @@ -280,13 +281,14 @@ |
384 | # more to scan |
385 | scan_info = self._queue.pop() |
386 | |
387 | + @defer.inlineCallbacks |
388 | def safe_scan(): |
389 | """Scan safely""" |
390 | try: |
391 | # add watches to UDF ancestors and check UDF is ok |
392 | volume = scan_info[0] |
393 | if isinstance(volume, volume_manager.UDF): |
394 | - udf_ok = self._add_watches_to_udf_ancestors(volume) |
395 | + udf_ok = yield self._add_watches_to_udf_ancestors(volume) |
396 | if not udf_ok: |
397 | self._process_next_queue(None) |
398 | return |
399 | @@ -615,12 +617,13 @@ |
400 | filesdirs[fname] = is_dir, statinfo, changed |
401 | return filesdirs |
402 | |
403 | + @defer.inlineCallbacks |
404 | def _scan_one_dir(self, scan_info): |
405 | """Gets one dir and compares with fsm.""" |
406 | share, dirpath, udf_mode = scan_info |
407 | |
408 | log_debug("Adding watch to %r", dirpath) |
409 | - self.eq.add_watch(dirpath) |
410 | + yield self.eq.add_watch(dirpath) |
411 | |
412 | to_later = [] |
413 | self.eq.freeze_begin(dirpath) |
414 | @@ -698,4 +701,5 @@ |
415 | d.addCallback(filter_delete_events) |
416 | d.addCallback(self.eq.freeze_commit) |
417 | d.addCallback(control) |
418 | - return d |
419 | + result = yield d |
420 | + defer.returnValue(result) |
421 | |
422 | === modified file 'ubuntuone/syncdaemon/volume_manager.py' |
423 | --- ubuntuone/syncdaemon/volume_manager.py 2011-07-28 15:43:05 +0000 |
424 | +++ ubuntuone/syncdaemon/volume_manager.py 2011-07-29 17:35:26 +0000 |
425 | @@ -943,6 +943,7 @@ |
426 | self.shares[share.volume_id] = share |
427 | self.m.event_q.push('VM_SHARE_CHANGED', share_id=share.volume_id) |
428 | |
429 | + @defer.inlineCallbacks |
430 | def _create_share_dir(self, share): |
431 | """ Creates the share root dir, and set the permissions. """ |
432 | # XXX: verterok: This is going to be moved into fsm |
433 | @@ -953,7 +954,7 @@ |
434 | # add the watch after the mkdir |
435 | if share.can_write(): |
436 | self.log.debug('adding inotify watch to: %s', share.path) |
437 | - self.m.event_q.add_watch(share.path) |
438 | + yield self.m.event_q.add_watch(share.path) |
439 | # if it's a ro share, change the perms |
440 | if not share.can_write(): |
441 | set_dir_readonly(share.path) |
442 | @@ -1949,4 +1950,3 @@ |
443 | def _serialize(self, value): |
444 | """Serialize value to string using protocol.""" |
445 | return cPickle.dumps(value.__dict__, protocol=2) |
446 | - |
Looks good!