From 3c871cadbf897bd1687b281d9d61913b0f514f40 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 3 Feb 2016 13:59:06 -0800 Subject: [PATCH 1/3] Be less spammy about donation messages --- letsencrypt/cli.py | 25 +++++++++++++++---------- letsencrypt/tests/cli_test.py | 7 ++++--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4c30e2ca8..597fb3731 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -406,14 +406,18 @@ def _report_new_cert(cert_path, fullchain_path): reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) -def _suggest_donation_if_appropriate(config): +def _suggest_donation_if_appropriate(config, action): """Potentially suggest a donation to support Let's Encrypt.""" - if not config.staging and not config.verb == "renew": # --dry-run implies --staging - reporter_util = zope.component.getUtility(interfaces.IReporter) - msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" - "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" - "Donating to EFF: https://eff.org/donate-le\n\n") - reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) + if config.staging or config.verb == "renew": + # --dry-run implies --staging + return + if action not in ["renew", "newcert"]: + return + reporter_util = zope.component.getUtility(interfaces.IReporter) + msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" + "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" + "Donating to EFF: https://eff.org/donate-le\n\n") + reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) def _report_successful_dry_run(): @@ -666,7 +670,7 @@ def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals else: display_ops.success_renewal(domains, action) - _suggest_donation_if_appropriate(config) + _suggest_donation_if_appropriate(config, action) def obtain_cert(config, plugins, lineage=None): @@ -686,6 +690,7 @@ def obtain_cert(config, plugins, lineage=None): # TODO: Handle errors from _init_le_client? le_client = _init_le_client(config, authenticator, installer) + action = "newcert" # This is a special case; cert and chain are simply saved if config.csr is not None: assert lineage is None, "Did not expect a CSR with a RenewableCert" @@ -700,11 +705,11 @@ def obtain_cert(config, plugins, lineage=None): _report_new_cert(cert_path, cert_fullchain) else: domains = _find_domains(config, installer) - _auth_from_domains(le_client, config, domains, lineage) + _, action = _auth_from_domains(le_client, config, domains, lineage) if config.dry_run: _report_successful_dry_run() - _suggest_donation_if_appropriate(config) + _suggest_donation_if_appropriate(config, action) def install(config, plugins): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f083018b3..c414d9054 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -233,7 +233,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably") with mock.patch("letsencrypt.cli._init_le_client") as mock_init: - with mock.patch("letsencrypt.cli._auth_from_domains"): + with mock.patch("letsencrypt.cli._auth_from_domains") as mock_afd: + mock_afd.return_value = (mock.MagicMock(), mock.MagicMock()) self._call(["certonly", "--manual", "-d", "foo.bar"]) unused_config, auth, unused_installer = mock_init.call_args[0] self.assertTrue(isinstance(auth, manual.Authenticator)) @@ -598,8 +599,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) self.assertFalse(mock_client.obtain_certificate.called) self.assertFalse(mock_client.obtain_and_enroll_certificate.called) - self.assertTrue( - 'donate' in mock_get_utility().add_message.call_args[0][0]) + self.assertEqual(mock_get_utility().add_message.call_count, 0) + #self.assertTrue('donate' not in mock_get_utility().add_message.call_args[0][0]) def _test_certonly_csr_common(self, extra_args=None): certr = 'certr' From 7281df234fd93c864ed854fc65400c0395292657 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 7 Feb 2016 18:20:05 -0800 Subject: [PATCH 2/3] Fix lint / merge error. --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 78e908f98..2e20bb288 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -715,7 +715,7 @@ def obtain_cert(config, plugins, lineage=None): # from happening. installer.restart() print("reloaded") - _suggest_donation_if_appropriate(config) + _suggest_donation_if_appropriate(config, action) def install(config, plugins): From 96d89cf44d4eebab6c3e7e785e440e3abe3875dc Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 9 Feb 2016 18:32:59 -0800 Subject: [PATCH 3/3] Disable too-many-locals for now --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 040ffa618..8cb4c0879 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -680,7 +680,7 @@ def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals def obtain_cert(config, plugins, lineage=None): """Implements "certonly": authenticate & obtain cert, but do not install it.""" - + # pylint: disable=too-many-locals try: # installers are used in auth mode to determine domain names installer, authenticator = choose_configurator_plugins(config, plugins, "certonly")