Skip to content

Commit a5d223d

Browse files
wgreenbergwgreenberg
authored
Replace (most) global state in cli/__init__.py (#9678)
* Rewrite helpful_test to appease the linter * Use public interface to access argparse sources dict * HelpfulParser builds ArgumentSources dict, stores it in NamespaceConfig After arguments/config files/user prompted input have been parsed, we build a mapping of Namespace options to an ArgumentSource value. These generally come from argparse's builtin "source_to_settings" dict, but we also add a source value representing dynamic values set at runtime. This dict is then passed to NamespaceConfig, which can then be queried directly or via the "set_by_user" method, which replaces the global "set_by_cli" and "option_was_set" functions. * Use NamespaceConfig.set_by_user instead of set_by_cli/option_was_set This involves passing the NamespaceConfig around to more functions than before, removes the need for most of the global state shenanigans needed by set_by_cli and friends. * Set runtime config values on the NamespaceConfig object This'll correctly mark them as being "runtime" values in the ArgumentSources dict * Bump oldest configargparse version We need a version that has get_source_to_settings_dict() * Add more cli unit tests, use ArgumentSource.DEFAULT by default One of the tests revealed that ConfigArgParse's source dict excludes arguments it considers unimportant/irrelevant. We now mark all arguments as having a DEFAULT source by default, and update them otherwise. * Mark more argument sources as RUNTIME * Removes some redundant helpful_test.py, moves one to cli_test.py We were already testing most of these cases in cli_test.py, only with a more complete HelpfulArgumentParser setup. And since the hsts/no-hsts test was manually performing the kind of argument adding that cli already does out of the box, I figured the cli tests were a more natural place for it. * appease the linter * Various fixups from review * Add windows compatability fix * Add test ensuring relevant_values behaves properly * Build sources dict in a more predictable manner The dict is now built in a defined order: first defaults, then config files, then env vars, then command line args. This way we eliminate the possibility of undefined behavior if configargparse puts an arg's entry in multiple source dicts. * remove superfluous update to sources dict * remove duplicate constant defines, resolve circular import situation
1 parent b5661e8 commit a5d223d

File tree

22 files changed

+494
-554
lines changed

22 files changed

+494
-554
lines changed

‎certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def _prepare_configurator(self) -> None:
5858
getattr(entrypoint.ENTRYPOINT.OS_DEFAULTS, k))
5959

6060
self._configurator = entrypoint.ENTRYPOINT(
61-
config=configuration.NamespaceConfig(self.le_config),
61+
config=configuration.NamespaceConfig(self.le_config, {}),
6262
name="apache")
6363
self._configurator.prepare()
6464

‎certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def _prepare_configurator(self) -> None:
4444
for k in constants.CLI_DEFAULTS:
4545
setattr(self.le_config, "nginx_" + k, constants.os_constant(k))
4646

47-
conf = configuration.NamespaceConfig(self.le_config)
47+
conf = configuration.NamespaceConfig(self.le_config, {})
4848
self._configurator = configurator.NginxConfigurator(config=conf, name="nginx")
4949
self._configurator.prepare()
5050

‎certbot/certbot/_internal/cli/__init__.py‎

Lines changed: 8 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from typing import Type
1111

1212
import certbot
13+
from certbot.configuration import NamespaceConfig
1314
from certbot._internal import constants
1415
from certbot._internal.cli.cli_constants import ARGPARSE_PARAMS_TO_REMOVE
1516
from certbot._internal.cli.cli_constants import cli_command
@@ -20,7 +21,6 @@
2021
from certbot._internal.cli.cli_constants import SHORT_USAGE
2122
from certbot._internal.cli.cli_constants import VAR_MODIFIERS
2223
from certbot._internal.cli.cli_constants import ZERO_ARG_ACTIONS
23-
from certbot._internal.cli.cli_utils import _Default
2424
from certbot._internal.cli.cli_utils import _DeployHookAction
2525
from certbot._internal.cli.cli_utils import _DomainsAction
2626
from certbot._internal.cli.cli_utils import _EncodeReasonAction
@@ -54,19 +54,19 @@
5454
helpful_parser: Optional[HelpfulArgumentParser] = None
5555

5656

57-
def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[str],
58-
detect_defaults: bool = False) -> argparse.Namespace:
57+
def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[str]
58+
) -> NamespaceConfig:
5959
"""Returns parsed command line arguments.
6060
6161
:param .PluginsRegistry plugins: available plugins
6262
:param list args: command line arguments with the program name removed
6363
6464
:returns: parsed command line arguments
65-
:rtype: argparse.Namespace
65+
:rtype: configuration.NamespaceConfig
6666
6767
"""
6868

69-
helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
69+
helpful = HelpfulArgumentParser(args, plugins)
7070
_add_all_groups(helpful)
7171

7272
# --help is automatically provided by argparse
@@ -471,95 +471,16 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[st
471471
# parser (--help should display plugin-specific options last)
472472
_plugins_parsing(helpful, plugins)
473473

474-
if not detect_defaults:
475-
global helpful_parser # pylint: disable=global-statement
476-
helpful_parser = helpful
474+
global helpful_parser # pylint: disable=global-statement
475+
helpful_parser = helpful
477476
return helpful.parse_args()
478477

479478

480-
def set_by_cli(var: str) -> bool:
481-
"""
482-
Return True if a particular config variable has been set by the user
483-
(CLI or config file) including if the user explicitly set it to the
484-
default. Returns False if the variable was assigned a default value.
485-
"""
486-
# We should probably never actually hit this code. But if we do,
487-
# a deprecated option has logically never been set by the CLI.
488-
if var in DEPRECATED_OPTIONS:
489-
return False
490-
491-
detector = set_by_cli.detector # type: ignore
492-
if detector is None and helpful_parser is not None:
493-
# Setup on first run: `detector` is a weird version of config in which
494-
# the default value of every attribute is wrangled to be boolean-false
495-
plugins = plugins_disco.PluginsRegistry.find_all()
496-
# reconstructed_args == sys.argv[1:], or whatever was passed to main()
497-
reconstructed_args = helpful_parser.args + [helpful_parser.verb]
498-
499-
detector = set_by_cli.detector = prepare_and_parse_args( # type: ignore
500-
plugins, reconstructed_args, detect_defaults=True)
501-
# propagate plugin requests: eg --standalone modifies config.authenticator
502-
detector.authenticator, detector.installer = (
503-
plugin_selection.cli_plugin_requests(detector))
504-
505-
if not isinstance(getattr(detector, var), _Default):
506-
logger.debug("Var %s=%s (set by user).", var, getattr(detector, var))
507-
return True
508-
509-
for modifier in VAR_MODIFIERS.get(var, []):
510-
if set_by_cli(modifier):
511-
logger.debug("Var %s=%s (set by user).",
512-
var, VAR_MODIFIERS.get(var, []))
513-
return True
514-
515-
return False
516-
517-
518-
# static housekeeping var
519-
# functions attributed are not supported by mypy
520-
# https://github.com/python/mypy/issues/2087
521-
set_by_cli.detector = None # type: ignore
522-
523-
524-
def has_default_value(option: str, value: Any) -> bool:
525-
"""Does option have the default value?
526-
527-
If the default value of option is not known, False is returned.
528-
529-
:param str option: configuration variable being considered
530-
:param value: value of the configuration variable named option
531-
532-
:returns: True if option has the default value, otherwise, False
533-
:rtype: bool
534-
535-
"""
536-
if helpful_parser is not None:
537-
return (option in helpful_parser.defaults and
538-
helpful_parser.defaults[option] == value)
539-
return False
540-
541-
542-
def option_was_set(option: str, value: Any) -> bool:
543-
"""Was option set by the user or does it differ from the default?
544-
545-
:param str option: configuration variable being considered
546-
:param value: value of the configuration variable named option
547-
548-
:returns: True if the option was set, otherwise, False
549-
:rtype: bool
550-
551-
"""
552-
# If an option is deprecated, it was effectively not set by the user.
553-
if option in DEPRECATED_OPTIONS:
554-
return False
555-
return set_by_cli(option) or not has_default_value(option, value)
556-
557-
558479
def argparse_type(variable: Any) -> Type:
559480
"""Return our argparse type function for a config variable (default: str)"""
560481
# pylint: disable=protected-access
561482
if helpful_parser is not None:
562-
for action in helpful_parser.parser._actions:
483+
for action in helpful_parser.actions:
563484
if action.type is not None and action.dest == variable:
564485
return action.type
565486
return str

‎certbot/certbot/_internal/cli/cli_utils.py‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,6 @@
2222
from certbot._internal.cli import helpful
2323

2424

25-
class _Default:
26-
"""A class to use as a default to detect if a value is set by a user"""
27-
28-
def __bool__(self) -> bool:
29-
return False
30-
31-
def __eq__(self, other: Any) -> bool:
32-
return isinstance(other, _Default)
33-
34-
def __hash__(self) -> int:
35-
return id(_Default)
36-
37-
def __nonzero__(self) -> bool:
38-
return self.__bool__()
39-
40-
4125
def read_file(filename: str, mode: str = "rb") -> Tuple[str, Any]:
4226
"""Returns the given file's contents.
4327

0 commit comments

Comments
 (0)