Merge lp:~mandel/ubuntuone-client/add-watch-deferred into lp:ubuntuone-client

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
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
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve
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-

Subscribers

People subscribed via source and target branches