From e32b664b2bd87644f21ab79148b857f5772f653e Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 14 Dec 2016 19:49:47 -0700 Subject: [PATCH 1/3] Adjust default poll_interval Also adds a --poll-interval option to both 'magic-folder join' and 'magic-folder create' so that the integration tests can pass something "very short". --- integration/conftest.py | 2 ++ src/allmydata/frontends/magic_folder.py | 4 ++-- src/allmydata/scripts/magic_folder_cli.py | 25 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/integration/conftest.py b/integration/conftest.py index bc0cbc67f..c1c31bc7a 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -337,6 +337,7 @@ def alice_invite(reactor, alice, temp_dir, request): [ sys.executable, '-m', 'allmydata.scripts.runner', 'magic-folder', 'create', + '--poll-interval', '2', '--basedir', node_dir, 'magik:', 'alice', join(temp_dir, 'magic-alice'), ] @@ -380,6 +381,7 @@ def magic_folder(reactor, alice_invite, alice, bob, temp_dir, request): [ sys.executable, '-m', 'allmydata.scripts.runner', 'magic-folder', 'join', + '--poll-interval', '2', '--basedir', bob_dir, alice_invite, join(temp_dir, 'magic-bob'), diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index 1a784020a..ae9063665 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, - uploader_delay=1.0, clock=None, downloader_delay=3): + uploader_delay=1.0, clock=None, downloader_delay=60): precondition_abspath(local_path_u) service.MultiService.__init__(self) @@ -752,7 +752,7 @@ class Downloader(QueueMixin, WriteFileMixin): def __init__(self, client, local_path_u, db, collective_dirnode, upload_readonly_dircap, clock, is_upload_pending, umask, - status_reporter, poll_interval=3): + status_reporter, poll_interval=60): QueueMixin.__init__(self, client, local_path_u, db, 'downloader', clock) if not IDirectoryNode.providedBy(collective_dirnode): diff --git a/src/allmydata/scripts/magic_folder_cli.py b/src/allmydata/scripts/magic_folder_cli.py index 2fb363ac7..f20f9f984 100644 --- a/src/allmydata/scripts/magic_folder_cli.py +++ b/src/allmydata/scripts/magic_folder_cli.py @@ -30,6 +30,10 @@ class CreateOptions(BasedirOptions): nickname = None local_dir = None synopsis = "MAGIC_ALIAS: [NICKNAME LOCAL_DIR]" + optParameters = [ + ("poll-interval", "p", "60", "How often to ask for updates"), + ] + def parseArgs(self, alias, nickname=None, local_dir=None): BasedirOptions.parseArgs(self) alias = argv_to_unicode(alias) @@ -37,6 +41,13 @@ class CreateOptions(BasedirOptions): raise usage.UsageError("An alias must end with a ':' character.") self.alias = alias[:-1] self.nickname = None if nickname is None else argv_to_unicode(nickname) + try: + if int(self['poll-interval']) <= 0: + raise ValueError("should be positive") + except ValueError: + raise usage.UsageError( + "--poll-interval must be a positive integer" + ) # Expand the path relative to the current directory of the CLI command, not the node. self.local_dir = None if local_dir is None else argv_to_abspath(local_dir, long_path=False) @@ -81,6 +92,8 @@ def create(options): return rc invite_code = invite_options.stdout.getvalue().strip() join_options = _delegate_options(options, JoinOptions()) + if 'poll-interval' in options: + join_options['poll-interval'] = options['poll-interval'] join_options.local_dir = options.local_dir join_options.invite_code = invite_code rc = join(join_options) @@ -147,9 +160,20 @@ class JoinOptions(BasedirOptions): synopsis = "INVITE_CODE LOCAL_DIR" dmd_write_cap = "" magic_readonly_cap = "" + optParameters = [ + ("poll-interval", "p", "60", "How often to ask for updates"), + ] + def parseArgs(self, invite_code, local_dir): BasedirOptions.parseArgs(self) + try: + if int(self['poll-interval']) <= 0: + raise ValueError("should be positive") + except ValueError: + raise usage.UsageError( + "--poll-interval must be a positive integer" + ) # Expand the path relative to the current directory of the CLI command, not the node. self.local_dir = None if local_dir is None else argv_to_abspath(local_dir, long_path=False) self.invite_code = to_str(argv_to_unicode(invite_code)) @@ -175,6 +199,7 @@ def join(options): config = configutil.get_config(os.path.join(options["node-directory"], u"tahoe.cfg")) configutil.set_config(config, "magic_folder", "enabled", "True") configutil.set_config(config, "magic_folder", "local.directory", options.local_dir.encode('utf-8')) + configutil.set_config(config, "magic_folder", "poll_interval", options.get("poll-interval", "60")) configutil.write_config(os.path.join(options["node-directory"], u"tahoe.cfg"), config) return 0 From 9303d20ed7e95893bcfc6e7247cd4d08dfe0449e Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 14 Dec 2016 21:20:37 -0800 Subject: [PATCH 2/3] test_magic_folder: improve coverage --- src/allmydata/test/cli/test_magic_folder.py | 41 +++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/allmydata/test/cli/test_magic_folder.py b/src/allmydata/test/cli/test_magic_folder.py index 23973bf34..9ed9d39c6 100644 --- a/src/allmydata/test/cli/test_magic_folder.py +++ b/src/allmydata/test/cli/test_magic_folder.py @@ -10,6 +10,7 @@ from allmydata.util.assertutil import precondition from allmydata.util import fileutil from allmydata.scripts.common import get_aliases from ..no_network import GridTestMixin +from ..common_util import parse_cli from .common import CLITestMixin from allmydata.test.common_util import NonASCIIPathMixin from allmydata.scripts import magic_folder_cli @@ -382,3 +383,43 @@ class CreateMagicFolder(MagicFolderCLITestMixin, unittest.TestCase): d.addCallback(check_success) return d + +class CreateErrors(unittest.TestCase): + def test_poll_interval(self): + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "create", "--poll-interval=frog", + "alias:") + self.assertEqual(str(e), "--poll-interval must be a positive integer") + + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "create", "--poll-interval=-4", + "alias:") + self.assertEqual(str(e), "--poll-interval must be a positive integer") + + def test_alias(self): + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "create", "no-colon") + self.assertEqual(str(e), "An alias must end with a ':' character.") + + def test_nickname(self): + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "create", "alias:", "nickname") + self.assertEqual(str(e), "If NICKNAME is specified then LOCAL_DIR must also be specified.") + +class InviteErrors(unittest.TestCase): + def test_alias(self): + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "invite", "no-colon") + self.assertEqual(str(e), "An alias must end with a ':' character.") + +class JoinErrors(unittest.TestCase): + def test_poll_interval(self): + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "join", "--poll-interval=frog", + "code", "localdir") + self.assertEqual(str(e), "--poll-interval must be a positive integer") + + e = self.assertRaises(usage.UsageError, parse_cli, + "magic-folder", "join", "--poll-interval=-2", + "code", "localdir") + self.assertEqual(str(e), "--poll-interval must be a positive integer") From 56519945017fee2f25794fde152cb9343a32506a Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 14 Dec 2016 21:20:45 -0800 Subject: [PATCH 3/3] magic_folder_cli: remove unnecessary conditional (this increases our branch coverage by one) --- src/allmydata/scripts/magic_folder_cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/allmydata/scripts/magic_folder_cli.py b/src/allmydata/scripts/magic_folder_cli.py index f20f9f984..9df49dd14 100644 --- a/src/allmydata/scripts/magic_folder_cli.py +++ b/src/allmydata/scripts/magic_folder_cli.py @@ -92,8 +92,7 @@ def create(options): return rc invite_code = invite_options.stdout.getvalue().strip() join_options = _delegate_options(options, JoinOptions()) - if 'poll-interval' in options: - join_options['poll-interval'] = options['poll-interval'] + join_options['poll-interval'] = options['poll-interval'] join_options.local_dir = options.local_dir join_options.invite_code = invite_code rc = join(join_options)