From e197f156a72e33c63e7068e012fe3bb0ce397e81 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 May 2015 05:47:09 +0000 Subject: [PATCH 1/5] choose_plugin can return None --- letsencrypt/client/display/ops.py | 6 +++++- letsencrypt/client/tests/display/ops_test.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py index 5a77e0ffb..dc6992c8c 100644 --- a/letsencrypt/client/display/ops.py +++ b/letsencrypt/client/display/ops.py @@ -71,7 +71,11 @@ def pick_plugin(config, default, plugins, question, ifaces): if len(prepared) > 1: logging.debug("Multiple candidate plugins: %s", prepared) - return choose_plugin(prepared.values(), question).init() + plugin_ep = choose_plugin(prepared.values(), question) + if plugin_ep is None: + return None + else: + return plugin_ep.init() elif len(prepared) == 1: plugin_ep = prepared.values()[0] logging.debug("Single candidate plugin: %s", plugin_ep) diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py index 151358f8a..7c5c1f74f 100644 --- a/letsencrypt/client/tests/display/ops_test.py +++ b/letsencrypt/client/tests/display/ops_test.py @@ -102,6 +102,17 @@ class PickPluginTest(unittest.TestCase): mock_choose.assert_called_once_with( [plugin_ep, plugin_ep], self.question) + def test_choose_plugin_none(self): + self.reg.ifaces().verify().available.return_value = { + "bar": None, + "baz": None, + } + + with mock.patch("letsencrypt.client.display" + ".ops.choose_plugin") as mock_choose: + mock_choose.return_value = None + self.assertTrue(self._call() is None) + class ConveniencePickPluginTest(unittest.TestCase): """Tests for letsencrypt.client.display.ops.pick_*.""" From 75a7b7605b82945a8701f34df414da98411afd52 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 May 2015 07:25:36 +0000 Subject: [PATCH 2/5] PluginsRegistry.find_init --- letsencrypt/client/plugins/disco.py | 26 ++++++++++++++++++++++++ letsencrypt/client/plugins/disco_test.py | 6 ++++++ 2 files changed, 32 insertions(+) diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py index f4a9faecf..fd636d59a 100644 --- a/letsencrypt/client/plugins/disco.py +++ b/letsencrypt/client/plugins/disco.py @@ -16,6 +16,9 @@ class PluginEntryPoint(object): PREFIX_FREE_DISTRIBUTIONS = ["letsencrypt"] """Distributions for which prefix will be omitted.""" + # this object is mutable, don't allow it to be hashed! + __hash__ = None + def __init__(self, entry_point): self.name = self.entry_point_to_plugin_name(entry_point) self.plugin_cls = entry_point.load() @@ -188,6 +191,29 @@ class PluginsRegistry(collections.Mapping): return self.filter(lambda p_ep: p_ep.available) # succefully prepared + misconfigured + def find_init(self, plugin): + """Find an initialized plugin. + + This is particularly useful for finding a name for the plugin + (although `.IPluginFactory.__call__` takes ``name`` as one of + the arguments, ``IPlugin.name`` is not part of the interface):: + + # plugin is an instance providing IPlugin, initialized + # somewhere else in the code + plugin_registry.find_init(plugin).name + + Returns ``None`` if ``plugin`` is not found in the registry. + + """ + # use list instead of set beacse PluginEntryPoint is not hashable + candidates = [plugin_ep for plugin_ep in self.plugins.itervalues() + if plugin_ep.initialized and plugin_ep.init() is plugin] + assert len(candidates) <= 1 + if candidates: + return candidates[0] + else: + return None + def __repr__(self): return "{0}({1!r})".format( self.__class__.__name__, set(self.plugins.itervalues())) diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py index 945f72d6b..ca3bc42d4 100644 --- a/letsencrypt/client/plugins/disco_test.py +++ b/letsencrypt/client/plugins/disco_test.py @@ -216,6 +216,12 @@ class PluginsRegistryTest(unittest.TestCase): self.plugin_ep.available = False self.assertEqual({}, self.reg.available().plugins) + def test_find_init(self): + self.assertTrue(self.reg.find_init(mock.Mock()) is None) + self.plugin_ep.initalized = True + self.assertTrue( + self.reg.find_init(self.plugin_ep.init()) is self.plugin_ep) + def test_repr(self): self.plugin_ep.__repr__ = lambda _: "PluginEntryPoint#mock" self.assertEqual("PluginsRegistry(set([PluginEntryPoint#mock]))", From e415a63d1fa6d969f9deaadd087284eac5510af6 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 May 2015 07:29:51 +0000 Subject: [PATCH 3/5] PluginsRegistry.plugins -> PluginsRegistry._plugins --- letsencrypt/client/plugins/disco.py | 22 +++++++++++----------- letsencrypt/client/plugins/disco_test.py | 15 +++++++++------ 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py index fd636d59a..ce6e23172 100644 --- a/letsencrypt/client/plugins/disco.py +++ b/letsencrypt/client/plugins/disco.py @@ -135,7 +135,7 @@ class PluginsRegistry(collections.Mapping): """Plugins registry.""" def __init__(self, plugins): - self.plugins = plugins + self._plugins = plugins @classmethod def find_all(cls): @@ -155,23 +155,23 @@ class PluginsRegistry(collections.Mapping): return cls(plugins) def __getitem__(self, name): - return self.plugins[name] + return self._plugins[name] def __iter__(self): - return iter(self.plugins) + return iter(self._plugins) def __len__(self): - return len(self.plugins) + return len(self._plugins) def init(self, config): """Initialize all plugins in the registry.""" return [plugin_ep.init(config) for plugin_ep - in self.plugins.itervalues()] + in self._plugins.itervalues()] def filter(self, pred): """Filter plugins based on predicate.""" return type(self)(dict((name, plugin_ep) for name, plugin_ep - in self.plugins.iteritems() if pred(plugin_ep))) + in self._plugins.iteritems() if pred(plugin_ep))) def ifaces(self, *ifaces_groups): """Filter plugins based on interfaces.""" @@ -184,7 +184,7 @@ class PluginsRegistry(collections.Mapping): def prepare(self): """Prepare all plugins in the registry.""" - return [plugin_ep.prepare() for plugin_ep in self.plugins.itervalues()] + return [plugin_ep.prepare() for plugin_ep in self._plugins.itervalues()] def available(self): """Filter plugins based on availability.""" @@ -206,7 +206,7 @@ class PluginsRegistry(collections.Mapping): """ # use list instead of set beacse PluginEntryPoint is not hashable - candidates = [plugin_ep for plugin_ep in self.plugins.itervalues() + candidates = [plugin_ep for plugin_ep in self._plugins.itervalues() if plugin_ep.initialized and plugin_ep.init() is plugin] assert len(candidates) <= 1 if candidates: @@ -216,9 +216,9 @@ class PluginsRegistry(collections.Mapping): def __repr__(self): return "{0}({1!r})".format( - self.__class__.__name__, set(self.plugins.itervalues())) + self.__class__.__name__, set(self._plugins.itervalues())) def __str__(self): - if not self.plugins: + if not self._plugins: return "No plugins" - return "\n\n".join(str(p_ep) for p_ep in self.plugins.itervalues()) + return "\n\n".join(str(p_ep) for p_ep in self._plugins.itervalues()) diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py index ca3bc42d4..f5ea9e6ee 100644 --- a/letsencrypt/client/plugins/disco_test.py +++ b/letsencrypt/client/plugins/disco_test.py @@ -194,16 +194,18 @@ class PluginsRegistryTest(unittest.TestCase): def test_ifaces(self): self.plugin_ep.ifaces.return_value = True - self.assertEqual(self.plugins, self.reg.ifaces().plugins) + # pylint: disable=protected-access + self.assertEqual(self.plugins, self.reg.ifaces()._plugins) self.plugin_ep.ifaces.return_value = False - self.assertEqual({}, self.reg.ifaces().plugins) + self.assertEqual({}, self.reg.ifaces()._plugins) def test_verify(self): self.plugin_ep.verify.return_value = True + # pylint: disable=protected-access self.assertEqual( - self.plugins, self.reg.verify(mock.MagicMock()).plugins) + self.plugins, self.reg.verify(mock.MagicMock())._plugins) self.plugin_ep.verify.return_value = False - self.assertEqual({}, self.reg.verify(mock.MagicMock()).plugins) + self.assertEqual({}, self.reg.verify(mock.MagicMock())._plugins) def test_prepare(self): self.plugin_ep.prepare.return_value = "baz" @@ -212,9 +214,10 @@ class PluginsRegistryTest(unittest.TestCase): def test_available(self): self.plugin_ep.available = True - self.assertEqual(self.plugins, self.reg.available().plugins) + # pylint: disable=protected-access + self.assertEqual(self.plugins, self.reg.available()._plugins) self.plugin_ep.available = False - self.assertEqual({}, self.reg.available().plugins) + self.assertEqual({}, self.reg.available()._plugins) def test_find_init(self): self.assertTrue(self.reg.find_init(mock.Mock()) is None) From 21411266087a6db13501241e0c34f0a5240a395f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 May 2015 07:36:52 +0000 Subject: [PATCH 4/5] clean up disco logging --- letsencrypt/client/plugins/disco.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py index ce6e23172..50e0bce50 100644 --- a/letsencrypt/client/plugins/disco.py +++ b/letsencrypt/client/plugins/disco.py @@ -80,7 +80,7 @@ class PluginEntryPoint(object): def prepared(self): """Has the plugin been prepared already?""" if not self.initialized: - logging.debug(".prepared called on uninitialized %s", self) + logging.debug(".prepared called on uninitialized %r", self) return self._prepared is not None def prepare(self): @@ -90,10 +90,10 @@ class PluginEntryPoint(object): try: self._initialized.prepare() except errors.LetsEncryptMisconfigurationError as error: - logging.debug("Misconfigured %s: %s", self, error) + logging.debug("Misconfigured %r: %s", self, error) self._prepared = error except errors.LetsEncryptNoInstallationError as error: - logging.debug("No installation (%s): %s", self, error) + logging.debug("No installation (%r): %s", self, error) self._prepared = error else: self._prepared = True @@ -150,8 +150,8 @@ class PluginsRegistry(collections.Mapping): if interfaces.IPluginFactory.providedBy(plugin_ep.plugin_cls): plugins[plugin_ep.name] = plugin_ep else: # pragma: no cover - logging.warning("Plugin entry point %s does not provide " - "IPluginFactory, skipping", plugin_ep) + logging.warning( + "%r does not provide IPluginFactory, skipping", plugin_ep) return cls(plugins) def __getitem__(self, name): From 42e7ec4bd2432397927f8d9661455f6681dfc5ac Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 May 2015 19:50:58 +0000 Subject: [PATCH 5/5] CLI: use_curses -> text_mode Previously, the --help output seemed to be broken: -t, --text Use the text output instead of the curses UI. (default: True) --- letsencrypt/client/cli.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py index 762e9a850..a83c0f767 100644 --- a/letsencrypt/client/cli.py +++ b/letsencrypt/client/cli.py @@ -236,7 +236,7 @@ def create_parser(plugins): help="Turn off confirmation screens, currently used for --revoke") add("-e", "--agree-tos", dest="tos", action="store_true", help="Skip the end user license agreement screen.") - add("-t", "--text", dest="use_curses", action="store_false", + add("-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") subparsers = parser.add_subparsers(metavar="SUBCOMMAND") @@ -339,10 +339,10 @@ def main(args=sys.argv[1:]): config = configuration.NamespaceConfig(args) # Displayer - if args.use_curses: - displayer = display_util.NcursesDisplay() - else: + if args.text_mode: displayer = display_util.FileDisplay(sys.stdout) + else: + displayer = display_util.NcursesDisplay() zope.component.provideUtility(displayer) # Logging @@ -350,7 +350,7 @@ def main(args=sys.argv[1:]): logger = logging.getLogger() logger.setLevel(level) logging.debug("Logging level set at %d", level) - if args.use_curses: + if not args.text_mode: logger.addHandler(log.DialogHandler()) logging.debug("Discovered plugins: %r", plugins)