From ddaa29ce39118ce713847edf439aa22773c19fc6 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 24 Oct 2016 14:47:22 -0600 Subject: [PATCH 1/3] refactor/white-space MagicFolder creation --- src/allmydata/client.py | 22 +++++----- src/allmydata/frontends/magic_folder.py | 47 +++++++++------------ src/allmydata/test/cli/test_magic_folder.py | 4 +- src/allmydata/test/test_magic_folder.py | 10 ++--- 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index d412fdac7..f2fc101d8 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -561,18 +561,18 @@ class Client(node.Node, pollmixin.PollMixin): if self.get_config("magic_folder", "enabled", False, boolean=True): #print "magic folder enabled" - upload_dircap = self.get_private_config("magic_folder_dircap") - collective_dircap = self.get_private_config("collective_dircap") - - local_dir_config = self.get_config("magic_folder", "local.directory").decode("utf-8") - local_dir = abspath_expanduser_unicode(local_dir_config, base=self.basedir) - - dbfile = os.path.join(self.basedir, "private", "magicfolderdb.sqlite") - dbfile = abspath_expanduser_unicode(dbfile) - from allmydata.frontends import magic_folder - umask = self.get_config("magic_folder", "download.umask", 0077) - s = magic_folder.MagicFolder(self, upload_dircap, collective_dircap, local_dir, dbfile, umask) + + db_filename = os.path.join(self.basedir, "private", "magicfolderdb.sqlite") + local_dir_config = self.get_config("magic_folder", "local.directory").decode("utf-8") + s = magic_folder.MagicFolder( + client=self, + upload_dircap=self.get_private_config("magic_folder_dircap"), + collective_dircap=self.get_private_config("collective_dircap"), + local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), + dbfile=abspath_expanduser_unicode(db_filename), + umask=self.get_config("magic_folder", "download.umask", 0077), + ) self._magic_folder = s s.setServiceParent(self) s.startService() diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index ce4db581a..c0db2af89 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -63,7 +63,7 @@ class MagicFolder(service.MultiService): name = 'magic-folder' def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, - pending_delay=1.0, clock=None): + pending_delay=1.0, clock=None, poll_interval=3): precondition_abspath(local_path_u) service.MultiService.__init__(self) @@ -83,7 +83,7 @@ class MagicFolder(service.MultiService): self.uploader = Uploader(client, local_path_u, db, upload_dirnode, pending_delay, clock) self.downloader = Downloader(client, local_path_u, db, collective_dirnode, upload_dirnode.get_readonly_uri(), clock, self.uploader.is_pending, umask, - self.set_public_status) + self.set_public_status, poll_interval=poll_interval) self._public_status = (False, ['Magic folder has not yet started']) def enable_debug_log(self, enabled=True): @@ -131,9 +131,8 @@ class MagicFolder(service.MultiService): class QueueMixin(HookMixin): - scan_interval = 0 - def __init__(self, client, local_path_u, db, name, clock, delay=0): + def __init__(self, client, local_path_u, db, name, clock): self._client = client self._local_path_u = local_path_u self._local_filepath = to_filepath(local_path_u) @@ -163,9 +162,6 @@ class QueueMixin(HookMixin): # do we also want to bound on "maximum age"? self._process_history = deque(maxlen=20) self._stopped = False - # XXX pass in an initial value for this; it seems like .10 broke this and it's always 0 - self._turn_delay = delay - self._log('delay is %f' % self._turn_delay) # a Deferred to wait for the _do_processing() loop to exit # (gets set to the return from _do_processing() if we get that @@ -212,35 +208,31 @@ class QueueMixin(HookMixin): (processing each item). After that we yield for _turn_deque seconds. """ - # we subtract here so there's a scan on the very first iteration - last_scan = self._clock.seconds() - self.scan_interval while not self._stopped: self._log("doing iteration") - d = task.deferLater(self._clock, self._turn_delay, lambda: None) - # ">=" is important here if scan scan_interval is 0 - if self._clock.seconds() - last_scan >= self.scan_interval: - # XXX can't we unify the "_full_scan" vs what - # Downloader does... - last_scan = self._clock.seconds() - yield self._when_queue_is_empty() # (this no-op for us, only Downloader uses it...) - self._log("did scan; now %d" % last_scan) - else: - self._log("skipped scan") + d = task.deferLater(self._clock, self._next_scan_delay(), lambda: None) + + # adds items to our deque + yield self._when_queue_is_empty() # process anything in our queue yield self._process_deque() - self._log("one loop; call_hook iteration %r" % self) - self._call_hook(None, 'iteration') + # we want to have our callLater queued in the reactor # *before* we trigger the 'iteration' hook, so that hook # can successfully advance the Clock and bypass the delay # if required (e.g. in the tests). + self._log("one loop; call_hook iteration %r" % self) + self._call_hook(None, 'iteration') if not self._stopped: self._log("waiting... %r" % d) yield d self._log("stopped") + def _next_scan_delay(self): + return self._turn_delay + def _when_queue_is_empty(self): return @@ -346,7 +338,7 @@ class UploadItem(QueuedItem): class Uploader(QueueMixin): def __init__(self, client, local_path_u, db, upload_dirnode, pending_delay, clock): - QueueMixin.__init__(self, client, local_path_u, db, 'uploader', clock, delay=pending_delay) + QueueMixin.__init__(self, client, local_path_u, db, 'uploader', clock) self.is_ready = False @@ -364,6 +356,7 @@ class Uploader(QueueMixin): self._periodic_full_scan_duration = 10 * 60 # perform a full scan every 10 minutes self._periodic_callid = None + self._turn_delay = pending_delay if hasattr(self._notifier, 'set_pending_delay'): self._notifier.set_pending_delay(pending_delay) @@ -753,12 +746,11 @@ class DownloadItem(QueuedItem): class Downloader(QueueMixin, WriteFileMixin): - scan_interval = 3 def __init__(self, client, local_path_u, db, collective_dirnode, upload_readonly_dircap, clock, is_upload_pending, umask, - status_reporter): - QueueMixin.__init__(self, client, local_path_u, db, 'downloader', clock, delay=self.scan_interval) + status_reporter, poll_interval=3): + QueueMixin.__init__(self, client, local_path_u, db, 'downloader', clock) if not IDirectoryNode.providedBy(collective_dirnode): raise AssertionError("The URI in '%s' does not refer to a directory." @@ -772,11 +764,12 @@ class Downloader(QueueMixin, WriteFileMixin): self._is_upload_pending = is_upload_pending self._umask = umask self._status_reporter = status_reporter + self._poll_interval = poll_interval @defer.inlineCallbacks def start_downloading(self): self._log("start_downloading") - self._turn_delay = self.scan_interval + self._turn_delay = self._poll_interval files = self._db.get_all_relpaths() self._log("all files %s" % files) @@ -794,7 +787,7 @@ class Downloader(QueueMixin, WriteFileMixin): "Last tried at %s" % self.nice_current_time(), ) twlog.msg("Magic Folder failed initial scan: %s" % (e,)) - yield task.deferLater(self._clock, self.scan_interval, lambda: None) + yield task.deferLater(self._clock, self._poll_interval, lambda: None) def nice_current_time(self): return format_time(datetime.fromtimestamp(self._clock.seconds()).timetuple()) diff --git a/src/allmydata/test/cli/test_magic_folder.py b/src/allmydata/test/cli/test_magic_folder.py index 16c85d91c..7d6ea0d21 100644 --- a/src/allmydata/test/cli/test_magic_folder.py +++ b/src/allmydata/test/cli/test_magic_folder.py @@ -122,8 +122,8 @@ class MagicFolderCLITestMixin(CLITestMixin, GridTestMixin, NonASCIIPathMixin): d = defer.succeed(None) def _clean(ign): d = self.magicfolder.finish() - self.magicfolder.uploader._clock.advance(self.magicfolder.uploader.scan_interval + 1) - self.magicfolder.downloader._clock.advance(self.magicfolder.downloader.scan_interval + 1) + self.magicfolder.uploader._clock.advance(self.magicfolder.uploader._turn_delay + 1) + self.magicfolder.downloader._clock.advance(self.magicfolder.downloader._turn_delay + 1) return d d.addCallback(_clean) diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index 38d36d228..a4178aa83 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -111,13 +111,13 @@ def iterate_downloader(magic): # can do either of these: #d = magic.downloader._process_deque() d = magic.downloader.set_hook('iteration') - magic.downloader._clock.advance(magic.downloader.scan_interval + 1) + magic.downloader._clock.advance(magic.downloader._turn_delay + 1) return d def iterate_uploader(magic): d = magic.uploader.set_hook('iteration') - magic.uploader._clock.advance(magic.uploader.scan_interval + 1) + magic.uploader._clock.advance(magic.uploader._turn_delay + 1) return d @defer.inlineCallbacks @@ -308,7 +308,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea self.alice_fileops = FileOperationsHelper(self.alice_magicfolder.uploader, self.inject_inotify) d0 = self.alice_magicfolder.uploader.set_hook('iteration') d1 = self.alice_magicfolder.downloader.set_hook('iteration') - self.alice_clock.advance(self.alice_magicfolder.uploader.scan_interval + 1) + self.alice_clock.advance(self.alice_magicfolder.uploader._turn_delay + 1) d0.addCallback(lambda ign: d1) d0.addCallback(lambda ign: result) return d0 @@ -332,7 +332,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea self.bob_fileops = FileOperationsHelper(self.bob_magicfolder.uploader, self.inject_inotify) d0 = self.bob_magicfolder.uploader.set_hook('iteration') d1 = self.bob_magicfolder.downloader.set_hook('iteration') - self.bob_clock.advance(self.alice_magicfolder.uploader.scan_interval + 1) + self.bob_clock.advance(self.alice_magicfolder.uploader._turn_delay + 1) d0.addCallback(lambda ign: d1) d0.addCallback(lambda ign: result) return d0 @@ -347,7 +347,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea for mf in [self.alice_magicfolder, self.bob_magicfolder]: for loader in [mf.uploader, mf.downloader]: - loader._clock.advance(loader.scan_interval + 1) + loader._clock.advance(loader._turn_delay + 1) yield d0 yield d1 From da4e1589bba394eb2bf8f3d16b77d7ebb17530d2 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 24 Oct 2016 17:09:57 -0600 Subject: [PATCH 2/3] Rename _turn_delay and friends to make things more clear This forces the Uploader and Downloader to implement a _scan_delay method and makes the naming more consistent with what's actually happening. Also, fix a few "bugs" in the names of args in the mocks for some tests. --- src/allmydata/frontends/magic_folder.py | 39 +++++++++++---------- src/allmydata/test/cli/test_magic_folder.py | 8 ++--- src/allmydata/test/test_client.py | 10 +++--- src/allmydata/test/test_magic_folder.py | 14 ++++---- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index c0db2af89..1a784020a 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -63,7 +63,7 @@ class MagicFolder(service.MultiService): name = 'magic-folder' def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, - pending_delay=1.0, clock=None, poll_interval=3): + uploader_delay=1.0, clock=None, downloader_delay=3): precondition_abspath(local_path_u) service.MultiService.__init__(self) @@ -80,10 +80,10 @@ class MagicFolder(service.MultiService): upload_dirnode = self._client.create_node_from_uri(upload_dircap) collective_dirnode = self._client.create_node_from_uri(collective_dircap) - self.uploader = Uploader(client, local_path_u, db, upload_dirnode, pending_delay, clock) + self.uploader = Uploader(client, local_path_u, db, upload_dirnode, uploader_delay, clock) self.downloader = Downloader(client, local_path_u, db, collective_dirnode, upload_dirnode.get_readonly_uri(), clock, self.uploader.is_pending, umask, - self.set_public_status, poll_interval=poll_interval) + self.set_public_status, poll_interval=downloader_delay) self._public_status = (False, ['Magic folder has not yet started']) def enable_debug_log(self, enabled=True): @@ -204,16 +204,16 @@ class QueueMixin(HookMixin): This is an infinite loop that processes things out of the _deque. One iteration runs self._process_deque which calls - _when_queue_is_empty() and then completely drains the _deque + _perform_scan() and then completely drains the _deque (processing each item). After that we yield for _turn_deque seconds. """ while not self._stopped: self._log("doing iteration") - d = task.deferLater(self._clock, self._next_scan_delay(), lambda: None) + d = task.deferLater(self._clock, self._scan_delay(), lambda: None) # adds items to our deque - yield self._when_queue_is_empty() + yield self._perform_scan() # process anything in our queue yield self._process_deque() @@ -230,10 +230,10 @@ class QueueMixin(HookMixin): self._log("stopped") - def _next_scan_delay(self): - return self._turn_delay + def _scan_delay(self): + raise NotImplementedError - def _when_queue_is_empty(self): + def _perform_scan(self): return @defer.inlineCallbacks @@ -352,11 +352,11 @@ class Uploader(QueueMixin): self._upload_dirnode = upload_dirnode self._inotify = get_inotify_module() self._notifier = self._inotify.INotify() - self._pending = set() # of unicode relpaths + self._pending = set() # of unicode relpaths + self._pending_delay = pending_delay self._periodic_full_scan_duration = 10 * 60 # perform a full scan every 10 minutes self._periodic_callid = None - self._turn_delay = pending_delay if hasattr(self._notifier, 'set_pending_delay'): self._notifier.set_pending_delay(pending_delay) @@ -416,6 +416,9 @@ class Uploader(QueueMixin): # *really* just call this synchronously. return self._begin_processing(None) + def _scan_delay(self): + return self._pending_delay + def _full_scan(self): self._periodic_callid = self._clock.callLater(self._periodic_full_scan_duration, self._full_scan) self._log("FULL SCAN") @@ -769,7 +772,6 @@ class Downloader(QueueMixin, WriteFileMixin): @defer.inlineCallbacks def start_downloading(self): self._log("start_downloading") - self._turn_delay = self._poll_interval files = self._db.get_all_relpaths() self._log("all files %s" % files) @@ -941,13 +943,15 @@ class Downloader(QueueMixin, WriteFileMixin): d.addCallback(_filter_batch_to_deque) return d + + def _scan_delay(self): + return self._poll_interval + @defer.inlineCallbacks - def _when_queue_is_empty(self): - # XXX can we amalgamate all the "scan" stuff and just call it - # directly from QueueMixin? + def _perform_scan(self): x = None try: - x = yield self._scan(None) + x = yield self._scan_remote_collective() self._status_reporter( True, 'Magic folder is working', 'Last scan: %s' % self.nice_current_time(), @@ -961,9 +965,6 @@ class Downloader(QueueMixin, WriteFileMixin): ) defer.returnValue(x) - def _scan(self, ign): - return self._scan_remote_collective() - def _process(self, item): # Downloader self._log("_process(%r)" % (item,)) diff --git a/src/allmydata/test/cli/test_magic_folder.py b/src/allmydata/test/cli/test_magic_folder.py index 7d6ea0d21..23973bf34 100644 --- a/src/allmydata/test/cli/test_magic_folder.py +++ b/src/allmydata/test/cli/test_magic_folder.py @@ -122,8 +122,8 @@ class MagicFolderCLITestMixin(CLITestMixin, GridTestMixin, NonASCIIPathMixin): d = defer.succeed(None) def _clean(ign): d = self.magicfolder.finish() - self.magicfolder.uploader._clock.advance(self.magicfolder.uploader._turn_delay + 1) - self.magicfolder.downloader._clock.advance(self.magicfolder.downloader._turn_delay + 1) + self.magicfolder.uploader._clock.advance(self.magicfolder.uploader._pending_delay + 1) + self.magicfolder.downloader._clock.advance(self.magicfolder.downloader._poll_interval + 1) return d d.addCallback(_clean) @@ -139,10 +139,10 @@ class MagicFolderCLITestMixin(CLITestMixin, GridTestMixin, NonASCIIPathMixin): local_path_u=local_magic_dir, dbfile=dbfile, umask=0o077, - pending_delay=0.2, clock=clock, + uploader_delay=0.2, + downloader_delay=0, ) - magicfolder.downloader._turn_delay = 0 magicfolder.setServiceParent(self.get_client(client_num)) magicfolder.ready() diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index d45c5cb18..42cd8d63d 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -322,14 +322,14 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test class MockMagicFolder(service.MultiService): name = 'magic-folder' - def __init__(self, client, upload_dircap, collective_dircap, local_dir, dbfile, umask, inotify=None, - pending_delay=1.0): + def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, inotify=None, + uploader_delay=1.0, clock=None, downloader_delay=3): service.MultiService.__init__(self) self.client = client self._umask = umask self.upload_dircap = upload_dircap self.collective_dircap = collective_dircap - self.local_dir = local_dir + self.local_dir = local_path_u self.dbfile = dbfile self.inotify = inotify @@ -376,8 +376,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test class Boom(Exception): pass - def BoomMagicFolder(client, upload_dircap, collective_dircap, local_dir, dbfile, - inotify=None, pending_delay=1.0): + def BoomMagicFolder(client, upload_dircap, collective_dircap, local_path_u, dbfile, + umask, inotify=None, uploader_delay=1.0, clock=None, downloader_delay=3): raise Boom() self.patch(allmydata.frontends.magic_folder, 'MagicFolder', BoomMagicFolder) diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index a4178aa83..e584bf0d9 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -111,13 +111,13 @@ def iterate_downloader(magic): # can do either of these: #d = magic.downloader._process_deque() d = magic.downloader.set_hook('iteration') - magic.downloader._clock.advance(magic.downloader._turn_delay + 1) + magic.downloader._clock.advance(magic.downloader._poll_interval + 1) return d def iterate_uploader(magic): d = magic.uploader.set_hook('iteration') - magic.uploader._clock.advance(magic.uploader._turn_delay + 1) + magic.uploader._clock.advance(magic.uploader._pending_delay + 1) return d @defer.inlineCallbacks @@ -308,7 +308,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea self.alice_fileops = FileOperationsHelper(self.alice_magicfolder.uploader, self.inject_inotify) d0 = self.alice_magicfolder.uploader.set_hook('iteration') d1 = self.alice_magicfolder.downloader.set_hook('iteration') - self.alice_clock.advance(self.alice_magicfolder.uploader._turn_delay + 1) + self.alice_clock.advance(self.alice_magicfolder.uploader._pending_delay + 1) d0.addCallback(lambda ign: d1) d0.addCallback(lambda ign: result) return d0 @@ -332,7 +332,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea self.bob_fileops = FileOperationsHelper(self.bob_magicfolder.uploader, self.inject_inotify) d0 = self.bob_magicfolder.uploader.set_hook('iteration') d1 = self.bob_magicfolder.downloader.set_hook('iteration') - self.bob_clock.advance(self.alice_magicfolder.uploader._turn_delay + 1) + self.bob_clock.advance(self.alice_magicfolder.uploader._pending_delay + 1) d0.addCallback(lambda ign: d1) d0.addCallback(lambda ign: result) return d0 @@ -346,8 +346,8 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea d1 = self.bob_magicfolder.finish() for mf in [self.alice_magicfolder, self.bob_magicfolder]: - for loader in [mf.uploader, mf.downloader]: - loader._clock.advance(loader._turn_delay + 1) + mf.uploader._clock.advance(mf.uploader._pending_delay + 1) + mf.downloader._clock.advance(mf.downloader._poll_interval + 1) yield d0 yield d1 @@ -541,7 +541,7 @@ class MagicFolderAliceBobTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, Rea # now, we ONLY want to do the scan, not a full iteration of # the process loop. So we do just the scan part "by hand" in # Bob's downloader - yield self.bob_magicfolder.downloader._when_queue_is_empty() + yield self.bob_magicfolder.downloader._perform_scan() # while we're delving into internals, I guess we might as well # confirm that we did queue up an item to download self.assertEqual(1, len(self.bob_magicfolder.downloader._deque)) From 0636d44cc1a7ee24fb959a66e62bf5c7522a7649 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 24 Oct 2016 16:44:38 -0600 Subject: [PATCH 3/3] add poll-interval config for magic-folder --- src/allmydata/client.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index f2fc101d8..e03e9ca47 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -88,6 +88,7 @@ def _valid_config_sections(): "download.umask", "enabled", "local.directory", + "poll_interval", ), }) return cfg @@ -565,6 +566,11 @@ class Client(node.Node, pollmixin.PollMixin): db_filename = os.path.join(self.basedir, "private", "magicfolderdb.sqlite") local_dir_config = self.get_config("magic_folder", "local.directory").decode("utf-8") + try: + poll_interval = int(self.get_config("magic_folder", "poll_interval", 3)) + except ValueError: + raise ValueError("[magic_folder] poll_interval must be an int") + s = magic_folder.MagicFolder( client=self, upload_dircap=self.get_private_config("magic_folder_dircap"), @@ -572,6 +578,7 @@ class Client(node.Node, pollmixin.PollMixin): local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), dbfile=abspath_expanduser_unicode(db_filename), umask=self.get_config("magic_folder", "download.umask", 0077), + downloader_delay=poll_interval, ) self._magic_folder = s s.setServiceParent(self)