From 427da7caec85a2d9ef92268ce089be1e58a25095 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 28 Aug 2024 22:59:53 +0000 Subject: [PATCH 1/7] temporary fix to add manage-sources-list-d to configuration options for landscape-client --- landscape/client/configuration.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index a546fc86..809db88c 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -190,6 +190,12 @@ def make_parser(self): """ parser = super().make_parser() + parser.add_option( + "--manage-sources-list-d", + default=True, + help="Repository profiles manage the files in" + "’etc/apt/sources.list.d'.", + ) parser.add_option( "--import", dest="import_from", From 7035ccb2540a4c632ffba0196d3422cb90379bfd Mon Sep 17 00:00:00 2001 From: HJK-X <34551787+HJK-X@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:10:16 -0700 Subject: [PATCH 2/7] argparse transferred fully, may need additional tests --- landscape-client.conf | 2 +- landscape/client/broker/config.py | 31 ++++++------ landscape/client/configuration.py | 33 +++++++------ landscape/client/deployment.py | 40 ++++++++-------- landscape/client/manager/config.py | 10 ++-- landscape/client/monitor/config.py | 2 +- landscape/client/package/reporter.py | 6 +-- landscape/client/tests/test_deployment.py | 36 +++++++------- landscape/client/watchdog.py | 9 ++-- landscape/lib/cli.py | 2 +- landscape/lib/config.py | 58 +++++++++++++++-------- landscape/lib/fetch.py | 13 ++--- landscape/lib/logging.py | 6 +-- landscape/lib/tests/test_config.py | 26 +++++----- landscape/sysinfo/deployment.py | 7 +-- 15 files changed, 150 insertions(+), 131 deletions(-) diff --git a/landscape-client.conf b/landscape-client.conf index af4faaf9..71bfa85a 100644 --- a/landscape-client.conf +++ b/landscape-client.conf @@ -2,7 +2,7 @@ bus = session computer_title = John's PC account_name = onward -registration_key = +registration_key = url = http://localhost:8080/message-system data_path = /tmp/landscape/ log_dir = /tmp/landscape/ diff --git a/landscape/client/broker/config.py b/landscape/client/broker/config.py index c050c22f..0709a7ef 100644 --- a/landscape/client/broker/config.py +++ b/landscape/client/broker/config.py @@ -1,4 +1,5 @@ """Configuration class for the broker.""" + import os from landscape.client.deployment import Configuration @@ -22,7 +23,7 @@ def exchange_store_path(self): def make_parser(self): """Parser factory for broker-specific options. - @return: An L{OptionParser} preset for all the options + @return: An L{ArgumentParser} preset for all the options from L{Configuration.make_parser} plus: - C{account_name} - C{registration_key} @@ -35,66 +36,66 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "-a", "--account-name", metavar="NAME", help="The account this computer belongs to.", ) - parser.add_option( + parser.add_argument( "-p", "--registration-key", metavar="KEY", help="The account-wide key used for registering clients.", ) - parser.add_option( + parser.add_argument( "-t", "--computer-title", metavar="TITLE", help="The title of this computer", ) - parser.add_option( + parser.add_argument( "--exchange-interval", default=15 * 60, - type="int", + type=int, metavar="INTERVAL", help="The number of seconds between server exchanges.", ) - parser.add_option( + parser.add_argument( "--urgent-exchange-interval", default=1 * 60, - type="int", + type=int, metavar="INTERVAL", help="The number of seconds between urgent server exchanges.", ) - parser.add_option( + parser.add_argument( "--ping-interval", default=30, - type="int", + type=int, metavar="INTERVAL", help="The number of seconds between pings.", ) - parser.add_option( + parser.add_argument( "--http-proxy", metavar="URL", help="The URL of the HTTP proxy, if one is needed.", ) - parser.add_option( + parser.add_argument( "--https-proxy", metavar="URL", help="The URL of the HTTPS proxy, if one is needed.", ) - parser.add_option( + parser.add_argument( "--access-group", default="", help="Suggested access group for this computer.", ) - parser.add_option( + parser.add_argument( "--tags", help="Comma separated list of tag names to be sent " "to the server.", ) - parser.add_option( + parser.add_argument( "--hostagent-uid", help="Only set this value if this computer is a WSL instance " "managed by Landscape, in which case set it to be the uid that " diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 809db88c..2a0a94a7 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -3,6 +3,7 @@ This module, and specifically L{LandscapeSetupScript}, implements the support for the C{landscape-config} script. """ + import getpass import io import logging @@ -190,13 +191,14 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "--manage-sources-list-d", + nargs="?", default=True, help="Repository profiles manage the files in" "’etc/apt/sources.list.d'.", ) - parser.add_option( + parser.add_argument( "--import", dest="import_from", metavar="FILENAME_OR_URL", @@ -205,68 +207,68 @@ def make_parser(self): "passed in the command line, with precedence " "being given to real command line options.", ) - parser.add_option( + parser.add_argument( "--script-users", metavar="USERS", help="A comma-separated list of users to allow " "scripts to run. To allow scripts to be run " "by any user, enter: ALL", ) - parser.add_option( + parser.add_argument( "--include-manager-plugins", metavar="PLUGINS", default="", help="A comma-separated list of manager plugins " "to enable in addition to the defaults.", ) - parser.add_option( + parser.add_argument( "-n", "--no-start", action="store_true", help="Don't start the client automatically.", ) - parser.add_option( + parser.add_argument( "--ok-no-register", action="store_true", help="Return exit code 0 instead of 2 if the client " "can't be registered.", ) - parser.add_option( + parser.add_argument( "--silent", action="store_true", default=False, help="Run without manual interaction.", ) - parser.add_option( + parser.add_argument( "--disable", action="store_true", default=False, help="Stop running clients and disable start at boot.", ) - parser.add_option( + parser.add_argument( "--init", action="store_true", default=False, help="Set up the client directories structure and exit.", ) - parser.add_option( + parser.add_argument( "--is-registered", action="store_true", help="Exit with code 0 (success) if client is " "registered else returns {}. Displays " "registration info.".format(EXIT_NOT_REGISTERED), ) - parser.add_option( + parser.add_argument( "--skip-registration", action="store_true", help="Don't send a new registration request", ) - parser.add_option( + parser.add_argument( "--force-registration", action="store_true", help="Force sending a new registration request", ) - parser.add_option( + parser.add_argument( "--register-if-needed", action="store_true", help=( @@ -831,10 +833,7 @@ def main(args, print=print): sys.exit(1) init_app_logging( - config.log_dir, - config.log_level, - "landscape-config", - config.quiet + config.log_dir, config.log_level, "landscape-config", config.quiet ) if config.skip_registration and config.force_registration: diff --git a/landscape/client/deployment.py b/landscape/client/deployment.py index 913f9666..fa53c63d 100644 --- a/landscape/client/deployment.py +++ b/landscape/client/deployment.py @@ -6,7 +6,7 @@ from datetime import datetime from datetime import timezone from logging import debug -from optparse import SUPPRESS_HELP +from argparse import SUPPRESS from typing import Sequence from twisted.logger import globalLogBeginner @@ -70,7 +70,7 @@ def __init__(self): def make_parser(self): """Parser factory for supported options. - @return: An OptionParser preset with options that all + @return: An ArgumentParser preset with options that all programs commonly accept. These include - config - data_path @@ -92,7 +92,7 @@ class Configuration(BaseConfiguration): def make_parser(self): """Parser factory for supported options. - @return: An L{OptionParser} preset for all options + @return: An L{ArgumentParser} preset for all options from L{BaseConfiguration.make_parser} plus: - C{quiet} (C{False}) - C{log_dir} (C{"/var/log/landscape"}) @@ -105,57 +105,57 @@ def make_parser(self): """ parser = super().make_parser() logging.add_cli_options(parser, logdir="/var/log/landscape") - parser.add_option( + parser.add_argument( "-u", "--url", default=self.DEFAULT_URL, help="The server URL to connect to.", ) - parser.add_option( + parser.add_argument( "--ping-url", help="The URL to perform lightweight exchange initiation with.", default="http://landscape.canonical.com/ping", ) - parser.add_option( + parser.add_argument( "-k", "--ssl-public-key", help="The public SSL key to verify the server. " "Only used if the given URL is https.", ) - parser.add_option( + parser.add_argument( "--ignore-sigint", action="store_true", default=False, help="Ignore interrupt signals.", ) - parser.add_option( + parser.add_argument( "--ignore-sigusr1", action="store_true", default=False, help="Ignore SIGUSR1 signal to " "rotate logs.", ) - parser.add_option( + parser.add_argument( "--package-monitor-interval", default=30 * 60, - type="int", + type=int, help="The interval between package monitor runs " "(default: 1800).", ) - parser.add_option( + parser.add_argument( "--apt-update-interval", default=6 * 60 * 60, - type="int", + type=int, help="The interval between apt update runs (default: 21600).", ) - parser.add_option( + parser.add_argument( "--flush-interval", default=5 * 60, - type="int", + type=int, metavar="INTERVAL", help="The number of seconds between flushes to disk " "for persistent data.", ) - parser.add_option( + parser.add_argument( "--stagger-launch", metavar="STAGGER_RATIO", dest="stagger_launch", @@ -164,20 +164,20 @@ def make_parser(self): help="Ratio, between 0 and 1, by which to stagger " "various tasks of landscape.", ) - parser.add_option( + parser.add_argument( "--snap-monitor-interval", default=30 * 60, - type="int", + type=int, help="The interval between snap monitor runs (default 1800).", ) # Hidden options, used for load-testing to run in-process clones - parser.add_option("--clones", default=0, type=int, help=SUPPRESS_HELP) - parser.add_option( + parser.add_argument("--clones", default=0, type=int, help=SUPPRESS) + parser.add_argument( "--start-clones-over", default=25 * 60, type=int, - help=SUPPRESS_HELP, + help=SUPPRESS, ) return parser diff --git a/landscape/client/manager/config.py b/landscape/client/manager/config.py index 40fabce3..6b23484b 100644 --- a/landscape/client/manager/config.py +++ b/landscape/client/manager/config.py @@ -30,30 +30,30 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "--manager-plugins", metavar="PLUGIN_LIST", help="Comma-delimited list of manager plugins to " "use. ALL means use all plugins.", default="ALL", ) - parser.add_option( + parser.add_argument( "--include-manager-plugins", metavar="PLUGIN_LIST", help="Comma-delimited list of manager plugins to " "enable, in addition to the defaults.", ) - parser.add_option( + parser.add_argument( "--script-users", metavar="USERS", help="Comma-delimited list of usernames that scripts" " may be run as. Default is to allow all " "users.", ) - parser.add_option( + parser.add_argument( "--script-output-limit", metavar="SCRIPT_OUTPUT_LIMIT", - type="int", + type=int, default=512, help="Maximum allowed output size that scripts" " can send. " diff --git a/landscape/client/monitor/config.py b/landscape/client/monitor/config.py index f0113eaa..63d278ac 100644 --- a/landscape/client/monitor/config.py +++ b/landscape/client/monitor/config.py @@ -34,7 +34,7 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "--monitor-plugins", metavar="PLUGIN_LIST", help="Comma-delimited list of monitor plugins to " diff --git a/landscape/client/package/reporter.py b/landscape/client/package/reporter.py index 38a4eed8..fb215291 100644 --- a/landscape/client/package/reporter.py +++ b/landscape/client/package/reporter.py @@ -53,18 +53,18 @@ def make_parser(self): reporter-specific options. """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "--force-apt-update", default=False, action="store_true", help="Force running apt-update.", ) - parser.add_option( + parser.add_argument( "--http-proxy", metavar="URL", help="The URL of the HTTP proxy, if one is needed.", ) - parser.add_option( + parser.add_argument( "--https-proxy", metavar="URL", help="The URL of the HTTPS proxy, if one is needed.", diff --git a/landscape/client/tests/test_deployment.py b/landscape/client/tests/test_deployment.py index 4e4fbea6..eca56a12 100644 --- a/landscape/client/tests/test_deployment.py +++ b/landscape/client/tests/test_deployment.py @@ -19,7 +19,7 @@ class BabbleConfiguration(BaseConfiguration): def make_parser(self): parser = super().make_parser() - parser.add_option("--whatever", metavar="STUFF") + parser.add_argument("--whatever", metavar="STUFF") return parser @@ -130,7 +130,7 @@ def test_different_config_file_section(self): def test_config_file_default(self): """Ensure parse_args sets appropriate config file default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.config, "/etc/landscape/client.conf") # The default filename isn't actually used. @@ -139,7 +139,7 @@ def test_config_file_default(self): def test_data_directory_default(self): """Ensure parse_args sets appropriate data_path default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.data_path, "/var/lib/landscape/client/") @@ -159,27 +159,27 @@ def test_log_file_option(self): """Ensure options.log_dir option can be read by parse_args.""" options = self.parser.parse_args( ["--log-dir", "/var/log/my-awesome-log"], - )[0] + ) self.assertEqual(options.log_dir, "/var/log/my-awesome-log") def test_log_level_default(self): """Ensure options.log_level default is set within parse_args.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.log_level, "info") def test_log_level_option(self): """Ensure options.log_level option can be read by parse_args.""" - options = self.parser.parse_args(["--log-level", "debug"])[0] + options = self.parser.parse_args(["--log-level", "debug"]) self.assertEqual(options.log_level, "debug") def test_quiet_option(self): """Ensure options.quiet option can be read by parse_args.""" - options = self.parser.parse_args(["--quiet"])[0] + options = self.parser.parse_args(["--quiet"]) self.assertEqual(options.quiet, True) def test_quiet_default(self): """Ensure options.quiet default is set within parse_args.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.quiet, False) # other options @@ -188,24 +188,24 @@ def test_url_option(self): """Ensure options.url option can be read by parse_args.""" options = self.parser.parse_args( ["--url", "http://mylandscape/message-system"], - )[0] + ) self.assertEqual(options.url, "http://mylandscape/message-system") def test_url_default(self): """Ensure parse_args sets appropriate url default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.url, self.config.DEFAULT_URL) def test_ping_url_option(self): """Ensure options.ping_url option can be read by parse_args.""" options = self.parser.parse_args( ["--ping-url", "http://mylandscape/ping"], - )[0] + ) self.assertEqual(options.ping_url, "http://mylandscape/ping") def test_ping_url_default(self): """Ensure parse_args sets appropriate ping_url default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual( options.ping_url, "http://landscape.canonical.com/ping", @@ -215,34 +215,34 @@ def test_ssl_public_key_option(self): """Ensure options.ssl_public_key option can be read by parse_args.""" options = self.parser.parse_args( ["--ssl-public-key", "/tmp/somekeyfile.ssl"], - )[0] + ) self.assertEqual(options.ssl_public_key, "/tmp/somekeyfile.ssl") def test_ssl_public_key_default(self): """Ensure parse_args sets appropriate ssl_public_key default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.ssl_public_key, None) def test_ignore_sigint_option(self): """Ensure options.ignore_sigint option can be read by parse_args.""" - options = self.parser.parse_args(["--ignore-sigint"])[0] + options = self.parser.parse_args(["--ignore-sigint"]) self.assertEqual(options.ignore_sigint, True) def test_ignore_sigint_default(self): """Ensure options.ignore_sigint default is set within parse_args.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(options.ignore_sigint, False) # hidden options def test_clones_default(self): """By default, no clones are started.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertEqual(0, options.clones) def test_clones_option(self): """It's possible to specify additional clones to be started.""" - options = self.parser.parse_args(["--clones", "3"])[0] + options = self.parser.parse_args(["--clones", "3"]) self.assertEqual(3, options.clones) # properties diff --git a/landscape/client/watchdog.py b/landscape/client/watchdog.py index ed2762a2..1af36e12 100644 --- a/landscape/client/watchdog.py +++ b/landscape/client/watchdog.py @@ -4,6 +4,7 @@ The main C{landscape-client} program uses this watchdog. """ + import errno import os import pwd @@ -508,17 +509,17 @@ def _notify_rotate_logs(self): class WatchDogConfiguration(Configuration): def make_parser(self): parser = super().make_parser() - parser.add_option( + parser.add_argument( "--daemon", action="store_true", help="Fork and run in the background.", ) - parser.add_option( + parser.add_argument( "--pid-file", - type="str", + type=str, help="The file to write the PID to.", ) - parser.add_option( + parser.add_argument( "--monitor-only", action="store_true", help="Don't enable management features. This is " diff --git a/landscape/lib/cli.py b/landscape/lib/cli.py index 18b2bec4..43843e84 100644 --- a/landscape/lib/cli.py +++ b/landscape/lib/cli.py @@ -7,7 +7,7 @@ def add_cli_options(parser, cfgfile=None, datadir=None): datadirhelp = "The directory in which to store data files." if datadir: datadirhelp += f" (default: {datadir!r})" - parser.add_option( + parser.add_argument( "-d", "--data-path", metavar="PATH", diff --git a/landscape/lib/config.py b/landscape/lib/config.py index 8f93a81c..a5a25f60 100644 --- a/landscape/lib/config.py +++ b/landscape/lib/config.py @@ -1,7 +1,7 @@ import os.path import sys from logging import getLogger -from optparse import OptionParser +from argparse import ArgumentParser, SUPPRESS from typing import Optional from typing import Sequence @@ -20,7 +20,7 @@ def add_cli_options(parser, filename=None): ) if filename is not None: cfgfilehelp += f" (default: {filename!r})" - parser.add_option( + parser.add_argument( "-c", "--config", metavar="FILE", @@ -29,15 +29,14 @@ def add_cli_options(parser, filename=None): ) -class ConfigSpecOptionParser(OptionParser): - +class ConfigSpecOptionParser(ArgumentParser): def __init__(self, unsaved_options=None): - OptionParser.__init__(self, unsaved_options) + ArgumentParser.__init__(self, unsaved_options) - def add_option(self, *args, **kwargs): - option = OptionParser.add_option(self, *args, **kwargs) + def add_argument(self, *args, **kwargs): + option = ArgumentParser.add_argument(self, *args, **kwargs) print(dir(option)) - print(option.get_opt_string()) + print(option.option_strings) return option @@ -70,10 +69,13 @@ def __init__(self): self._config_filename = None self._config_file_options = {} self._parser = self.make_parser() - self._command_line_defaults = self._parser.defaults.copy() - # We don't want them mixed with explicitly given options, - # otherwise we can't define the precedence properly. - self._parser.defaults.clear() + + # We don't want config files to save any default values + defaults = {} + for action in self._parser._actions: + defaults[action.dest] = action.default + action.default = SUPPRESS + self._command_line_defaults = defaults def __getattr__(self, name): """Find and return the value of the given configuration parameter. @@ -99,14 +101,21 @@ def __getattr__(self, name): value = options[name] break else: - if self._parser.has_option("--" + name.replace("_", "-")): + if name in [action.dest for action in self._parser._actions]: value = None else: raise AttributeError(name) if isinstance(value, StringType): - option = self._parser.get_option("--" + name.replace("_", "-")) - if option is not None: - value = option.convert_value(None, value) + option = None + for action in self._parser._actions: + if name == action.dest: + option = action + break + if option is not None and option.type is not None: + if option.nargs is None or option.nargs == 1: + return option.type(value) + else: + return tuple([option.type(v) for v in value]) return value def clone(self): @@ -169,7 +178,6 @@ def load(self, args, accept_nonexistent_default_config=False): config_filename, os.R_OK, ): - self.load_configuration_file(config_filename) break @@ -203,7 +211,7 @@ def _load_external_options(self): def load_command_line(self, args): """Load configuration data from the given command line.""" self._command_line_args = args - values = self._parser.parse_args(args)[0] + values = self._parser.parse_intermixed_args(args) self._command_line_options = vars(values) def load_configuration_file(self, filename): @@ -284,8 +292,8 @@ def write(self): value == self._command_line_defaults.get(name) and name not in self._config_file_options and name not in self._command_line_options + or name == "positional" ): - # We don't want to write this value to the config file # as it is default value and as not present in the # config file @@ -300,12 +308,20 @@ def write(self): def make_parser(self, cfgfile=None, datadir=None): """Parser factory for supported options. - @return: An OptionParser preset with options that all + @return: An ArgumentParser preset with options that all programs commonly accept. These include - config - data_path """ - parser = OptionParser(version=self.version) + parser = ArgumentParser() + # workaround to ignore extra arguments + # similar to OptionParser.parse_args([]) + parser.add_argument( + "positional", nargs="*", default=SUPPRESS, help=SUPPRESS + ) + version = self.version + if version: + parser.add_argument("--version", action="version", version=version) cli.add_cli_options(parser, cfgfile, datadir) return parser diff --git a/landscape/lib/fetch.py b/landscape/lib/fetch.py index da675df8..ba9542db 100644 --- a/landscape/lib/fetch.py +++ b/landscape/lib/fetch.py @@ -1,7 +1,7 @@ import io import os import sys -from optparse import OptionParser +from argparse import ArgumentParser from twisted.internet.defer import DeferredList from twisted.internet.threads import deferToThread @@ -212,11 +212,12 @@ def log_error(failure, url): def test(args): - parser = OptionParser() - parser.add_option("--post", action="store_true") - parser.add_option("--data", default="") - parser.add_option("--cainfo") - options, (url,) = parser.parse_args(args) + parser = ArgumentParser() + parser.add_argument("--post", action="store_true") + parser.add_argument("--data", default="") + parser.add_argument("--cainfo") + options = parser.parse_args(args) + url = options.positional print( fetch( url, diff --git a/landscape/lib/logging.py b/landscape/lib/logging.py index e3d79841..f13d3923 100644 --- a/landscape/lib/logging.py +++ b/landscape/lib/logging.py @@ -11,7 +11,7 @@ class LoggingAttributeError(Exception): def add_cli_options(parser, level="info", logdir=None): """Add common logging-related CLI options to the given arg parser.""" - parser.add_option( + parser.add_argument( "-q", "--quiet", default=False, @@ -22,7 +22,7 @@ def add_cli_options(parser, level="info", logdir=None): logdirhelp = "The directory in which to write log files" if logdir: logdirhelp += f" (default: {logdir!r})." - parser.add_option( + parser.add_argument( "-l", "--log-dir", metavar="FILE", @@ -30,7 +30,7 @@ def add_cli_options(parser, level="info", logdir=None): help=logdirhelp, ) - parser.add_option( + parser.add_argument( "--log-level", default=level, help="One of debug, info, warning, error or critical.", diff --git a/landscape/lib/tests/test_config.py b/landscape/lib/tests/test_config.py index 98270204..525afcef 100644 --- a/landscape/lib/tests/test_config.py +++ b/landscape/lib/tests/test_config.py @@ -2,7 +2,7 @@ import os.path import sys import unittest -from optparse import OptionParser +from argparse import ArgumentParser from textwrap import dedent from unittest import mock @@ -21,7 +21,7 @@ class BabbleConfiguration(BaseConfiguration): def make_parser(self): parser = super().make_parser() - parser.add_option("--whatever", metavar="STUFF") + parser.add_argument("--whatever", metavar="STUFF") return parser @@ -34,7 +34,7 @@ def make_parser(self): parser = super().make_parser() for name, value in defaults.items(): name = name.replace("_", "-") - parser.add_option("--" + name, default=value) + parser.add_argument("--" + name, default=value) return parser return MyConfiguration @@ -189,9 +189,9 @@ def test_get_config_object_with_alternative_config(self): def test_command_line_option_without_default(self): class MyConfiguration(BaseConfiguration): def make_parser(self): - parser = OptionParser() + parser = ArgumentParser() # Keep the dash in the option name to ensure it works. - parser.add_option("--foo-bar") + parser.add_argument("--foo-bar") return parser self.assertEqual(MyConfiguration().foo_bar, None) @@ -225,19 +225,19 @@ def test_command_line_with_unsaved_options(self): # --config def test_config_option(self): - options = self.parser.parse_args(["--config", "hello.cfg"])[0] + options = self.parser.parse_args(["--config", "hello.cfg"]) self.assertEqual(options.config, "hello.cfg") def test_config_file_default(self): """Ensure parse_args sets appropriate config file default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertIs(options.config, None) parser = BaseConfiguration().make_parser( cfgfile="spam.conf", datadir="/tmp/spam/data", ) - options = parser.parse_args([])[0] + options = parser.parse_args([]) self.assertEqual(options.config, "spam.conf") def test_load_config_from_option(self): @@ -264,19 +264,19 @@ def test_data_directory_option(self): """Ensure options.data_path option can be read by parse_args.""" options = self.parser.parse_args( ["--data-path", "/opt/hoojy/var/run"], - )[0] + ) self.assertEqual(options.data_path, "/opt/hoojy/var/run") def test_data_directory_default(self): """Ensure parse_args sets appropriate data_path default.""" - options = self.parser.parse_args([])[0] + options = self.parser.parse_args([]) self.assertIs(options.data_path, None) parser = BaseConfiguration().make_parser( cfgfile="spam.conf", datadir="/tmp/spam/data", ) - options = parser.parse_args([])[0] + options = parser.parse_args([]) self.assertEqual(options.data_path, "/tmp/spam/data") # loading @@ -301,7 +301,7 @@ def test_load_typed_option_from_file(self): class MyConfiguration(self.config_class): def make_parser(self): parser = super().make_parser() - parser.add_option("--year", default=1, type="int") + parser.add_argument("--year", default=1, type=int) return parser filename = self.makeFile("[my-config]\nyear = 2008\n") @@ -318,7 +318,7 @@ def test_load_typed_option_from_command_line(self): class MyConfiguration(self.config_class): def make_parser(self): parser = super().make_parser() - parser.add_option("--year", default=1, type="int") + parser.add_argument("--year", default=1, type=int) return parser self.write_config_file() diff --git a/landscape/sysinfo/deployment.py b/landscape/sysinfo/deployment.py index e743d92a..ea3ddceb 100644 --- a/landscape/sysinfo/deployment.py +++ b/landscape/sysinfo/deployment.py @@ -1,4 +1,5 @@ """Deployment code for the sysinfo tool.""" + import os import sys from logging import Formatter @@ -52,14 +53,14 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_option( + parser.add_argument( "--sysinfo-plugins", metavar="PLUGIN_LIST", help="Comma-delimited list of sysinfo plugins to " "use. Default is to use all plugins.", ) - parser.add_option( + parser.add_argument( "--exclude-sysinfo-plugins", metavar="PLUGIN_LIST", help="Comma-delimited list of sysinfo plugins to " @@ -67,7 +68,7 @@ def make_parser(self): "plugins to include.", ) - parser.add_option( + parser.add_argument( "--width", type=int, default=80, From fe3b6ff89313a55c7c2233de620f8f6aabe72f9e Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 30 Aug 2024 18:00:48 +0000 Subject: [PATCH 3/7] Added tests for optparse to argparse transfer --- landscape/client/configuration.py | 4 +-- landscape/lib/tests/test_config.py | 55 +++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 3ef49e75..3e7a7477 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -195,8 +195,8 @@ def make_parser(self): "--manage-sources-list-d", nargs="?", default=True, - help="Repository profiles manage the files in" - "’etc/apt/sources.list.d'.", + help="Repository profiles manage the files in " + "’etc/apt/sources.list.d'. (default: true)", ) parser.add_argument( "--import", diff --git a/landscape/lib/tests/test_config.py b/landscape/lib/tests/test_config.py index 525afcef..86dc1575 100644 --- a/landscape/lib/tests/test_config.py +++ b/landscape/lib/tests/test_config.py @@ -2,7 +2,7 @@ import os.path import sys import unittest -from argparse import ArgumentParser +from argparse import ArgumentParser, SUPPRESS from textwrap import dedent from unittest import mock @@ -159,6 +159,59 @@ def test_get_config_filename_precedence(self): self.config.config = explicit_filename self.assertEqual(self.config.get_config_filename(), explicit_filename) + # argparse CLI parsing + + def test_default_positional_argument(self): + self.reset_config(cfg_class(foo_bar=None)) + self.config_class.config = None + self.write_config_file() + + self.config.load(["--foo-bar", "ooga"]) + self.assertEqual(self.config.foo_bar, "ooga") + self.assertEqual(self.config.positional, SUPPRESS) + + def test_single_positional_argument(self): + self.reset_config(cfg_class(foo_bar=None)) + self.config_class.config = None + self.write_config_file() + + self.config.load(["bar", "--foo-bar", "ooga"]) + self.assertEqual(self.config.foo_bar, "ooga") + self.assertEqual(self.config.positional, ["bar"]) + + def test_intermixed_positional_arguments(self): + self.reset_config(cfg_class(foo_bar=None, extra_bar=None)) + self.config_class.config = None + self.write_config_file() + + self.config.load( + [ + "bar", + "--foo-bar", + "ooga", + "b1", + "b2", + "--extra-bar", + "foo", + "b3", + ] + ) + self.assertEqual(self.config.foo_bar, "ooga") + self.assertEqual(self.config.extra_bar, "foo") + self.assertEqual(self.config.positional, ["bar", "b1", "b2", "b3"]) + + def test_positional_arguments_unsaved(self): + self.reset_config(cfg_class()) + self.config_class.config = None + self.write_config_file() + + self.config.load(["foo", "bar"]) + self.assertEqual(self.config.positional, ["foo", "bar"]) + self.config.write() + + self.config.load([]) + self.assertEqual(self.config.positional, SUPPRESS) + # ConfigObj def test_get_config_object(self): From 2a6399a5c8752f3491c405370ac19cb6ffc62496 Mon Sep 17 00:00:00 2001 From: HJK-X <34551787+HJK-X@users.noreply.github.com> Date: Fri, 30 Aug 2024 11:49:27 -0700 Subject: [PATCH 4/7] moved command order --- landscape/client/configuration.py | 14 +++++++------- man/landscape-config.1 | 5 +++++ man/landscape-config.txt | 2 ++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 3e7a7477..7869dafc 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -191,13 +191,6 @@ def make_parser(self): """ parser = super().make_parser() - parser.add_argument( - "--manage-sources-list-d", - nargs="?", - default=True, - help="Repository profiles manage the files in " - "’etc/apt/sources.list.d'. (default: true)", - ) parser.add_argument( "--import", dest="import_from", @@ -221,6 +214,13 @@ def make_parser(self): help="A comma-separated list of manager plugins " "to enable in addition to the defaults.", ) + parser.add_argument( + "--manage-sources-list-d", + nargs="?", + default=True, + help="Repository profiles manage the files in " + "’etc/apt/sources.list.d'. (default: true)", + ) parser.add_argument( "-n", "--no-start", diff --git a/man/landscape-config.1 b/man/landscape-config.1 index 5a4fb910..fda99197 100644 --- a/man/landscape-config.1 +++ b/man/landscape-config.1 @@ -171,6 +171,11 @@ A comma-separated list of manager plugins to load explicitly. .TP .B +\fB--manage-sources-list-d\fP [MANAGE_SOURCES_LIST_D] +Repository profiles manage the files in +’etc/apt/sources.list.d'. (default: true) +.TP +.B \fB-n\fP, \fB--no-start\fP Don't start the client automatically. .TP diff --git a/man/landscape-config.txt b/man/landscape-config.txt index ada79eb8..335ed2e7 100644 --- a/man/landscape-config.txt +++ b/man/landscape-config.txt @@ -72,6 +72,8 @@ OPTIONS enter: ALL. --include-manager-plugins=PLUGINS A comma-separated list of manager plugins to load explicitly. + --manage-sources-list-d [MANAGE_SOURCES_LIST_D] Repository profiles manage + the files in ’etc/apt/sources.list.d'. (default: true) -n, --no-start Don't start the client automatically. --ok-no-register Return exit code 0 instead of 2 if the client can't be registered. From 7b788b65016244d53ab47de7cfdb7c8df3743d55 Mon Sep 17 00:00:00 2001 From: HJK-X <34551787+HJK-X@users.noreply.github.com> Date: Fri, 30 Aug 2024 13:44:01 -0700 Subject: [PATCH 5/7] Added tests to ensure optional argument options are valid --- landscape/client/configuration.py | 3 +- landscape/lib/config.py | 2 +- landscape/lib/tests/test_config.py | 80 ++++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 7869dafc..907ee644 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -217,7 +217,8 @@ def make_parser(self): parser.add_argument( "--manage-sources-list-d", nargs="?", - default=True, + default="true", + const="true", help="Repository profiles manage the files in " "’etc/apt/sources.list.d'. (default: true)", ) diff --git a/landscape/lib/config.py b/landscape/lib/config.py index a5a25f60..a96b2d97 100644 --- a/landscape/lib/config.py +++ b/landscape/lib/config.py @@ -112,7 +112,7 @@ def __getattr__(self, name): option = action break if option is not None and option.type is not None: - if option.nargs is None or option.nargs == 1: + if option.nargs in [None, 1, "?"]: return option.type(value) else: return tuple([option.type(v) for v in value]) diff --git a/landscape/lib/tests/test_config.py b/landscape/lib/tests/test_config.py index 86dc1575..4eccb201 100644 --- a/landscape/lib/tests/test_config.py +++ b/landscape/lib/tests/test_config.py @@ -25,6 +25,19 @@ def make_parser(self): return parser +class OptionalArgConfiguration(BaseConfiguration): + config_section = "my-config" + + def make_parser(self): + parser = super().make_parser() + parser.add_argument("--test") + parser.add_argument( + "--foo-bar", nargs="?", default="foofoo", const="foofoo" + ) + + return parser + + def cfg_class(section=None, **defaults): class MyConfiguration(BaseConfiguration): config_section = section or "my-config" @@ -161,7 +174,7 @@ def test_get_config_filename_precedence(self): # argparse CLI parsing - def test_default_positional_argument(self): + def test_parse_default_positional_argument(self): self.reset_config(cfg_class(foo_bar=None)) self.config_class.config = None self.write_config_file() @@ -170,7 +183,7 @@ def test_default_positional_argument(self): self.assertEqual(self.config.foo_bar, "ooga") self.assertEqual(self.config.positional, SUPPRESS) - def test_single_positional_argument(self): + def test_parse_single_positional_argument(self): self.reset_config(cfg_class(foo_bar=None)) self.config_class.config = None self.write_config_file() @@ -179,7 +192,7 @@ def test_single_positional_argument(self): self.assertEqual(self.config.foo_bar, "ooga") self.assertEqual(self.config.positional, ["bar"]) - def test_intermixed_positional_arguments(self): + def test_parse_intermixed_positional_arguments(self): self.reset_config(cfg_class(foo_bar=None, extra_bar=None)) self.config_class.config = None self.write_config_file() @@ -200,7 +213,7 @@ def test_intermixed_positional_arguments(self): self.assertEqual(self.config.extra_bar, "foo") self.assertEqual(self.config.positional, ["bar", "b1", "b2", "b3"]) - def test_positional_arguments_unsaved(self): + def test_parse_positional_arguments_unsaved(self): self.reset_config(cfg_class()) self.config_class.config = None self.write_config_file() @@ -212,6 +225,65 @@ def test_positional_arguments_unsaved(self): self.config.load([]) self.assertEqual(self.config.positional, SUPPRESS) + def test_parse_without_option_and_optional_argument(self): + self.reset_config(OptionalArgConfiguration) + self.config_filename = self.makeFile("") + os.unlink(self.config_filename) + self.config.default_config_filenames[:] = [self.config_filename] + self.config_class.config = None + self.config.write() + + self.config.load(["--test", "ooga"]) + self.assertEqual(self.config.test, "ooga") + self.assertEqual(self.config.foo_bar, "foofoo") + self.config.write() + + self.config.load([]) + self.assertEqual(self.config.foo_bar, "foofoo") + + data = read_text_file(self.config_filename) + self.assertConfigEqual(data, "[my-config]\ntest = ooga") + + def test_parse_with_option_including_optional_argument(self): + self.reset_config(OptionalArgConfiguration) + self.config_filename = self.makeFile("") + os.unlink(self.config_filename) + self.config.default_config_filenames[:] = [self.config_filename] + self.config_class.config = None + self.config.write() + + self.config.load(["--foo-bar", "barbar", "--test", "ooga"]) + self.assertEqual(self.config.foo_bar, "barbar") + self.config.write() + + self.config.load([]) + self.assertEqual(self.config.foo_bar, "barbar") + + data = read_text_file(self.config_filename) + self.assertConfigEqual( + data, "[my-config]\nfoo_bar = barbar\ntest = ooga" + ) + + def test_parse_with_option_excluding_optional_argument(self): + self.reset_config(OptionalArgConfiguration) + self.config_filename = self.makeFile("") + os.unlink(self.config_filename) + self.config.default_config_filenames[:] = [self.config_filename] + self.config_class.config = None + self.config.write() + + self.config.load(["--foo-bar", "--test", "ooga"]) + self.assertEqual(self.config.foo_bar, "foofoo") + self.config.write() + + self.config.load([]) + self.assertEqual(self.config.foo_bar, "foofoo") + + data = read_text_file(self.config_filename) + self.assertConfigEqual( + data, "[my-config]\nfoo_bar = foofoo\ntest = ooga" + ) + # ConfigObj def test_get_config_object(self): From bb654b5b286bb63792f9214a3250bbfe2198a7c2 Mon Sep 17 00:00:00 2001 From: HJK-X Date: Thu, 5 Sep 2024 15:47:57 -0700 Subject: [PATCH 6/7] docstrings for tests --- landscape-client.conf | 2 +- landscape/lib/tests/test_config.py | 34 +++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/landscape-client.conf b/landscape-client.conf index 71bfa85a..af4faaf9 100644 --- a/landscape-client.conf +++ b/landscape-client.conf @@ -2,7 +2,7 @@ bus = session computer_title = John's PC account_name = onward -registration_key = +registration_key = url = http://localhost:8080/message-system data_path = /tmp/landscape/ log_dir = /tmp/landscape/ diff --git a/landscape/lib/tests/test_config.py b/landscape/lib/tests/test_config.py index 4eccb201..2e037ade 100644 --- a/landscape/lib/tests/test_config.py +++ b/landscape/lib/tests/test_config.py @@ -175,6 +175,11 @@ def test_get_config_filename_precedence(self): # argparse CLI parsing def test_parse_default_positional_argument(self): + """ + Ensure the option for any positional arguments defaults to + argparse.SUPPRESS when config.load is called with no positional + arguments. + """ self.reset_config(cfg_class(foo_bar=None)) self.config_class.config = None self.write_config_file() @@ -184,6 +189,10 @@ def test_parse_default_positional_argument(self): self.assertEqual(self.config.positional, SUPPRESS) def test_parse_single_positional_argument(self): + """ + Ensure a positional argument is correctly parsed when config.load is + called and collected into the positional option. + """ self.reset_config(cfg_class(foo_bar=None)) self.config_class.config = None self.write_config_file() @@ -193,6 +202,10 @@ def test_parse_single_positional_argument(self): self.assertEqual(self.config.positional, ["bar"]) def test_parse_intermixed_positional_arguments(self): + """ + Ensure positional arguments that are intermixed with optional arguments + are parsed and collected into the positional option. + """ self.reset_config(cfg_class(foo_bar=None, extra_bar=None)) self.config_class.config = None self.write_config_file() @@ -202,18 +215,19 @@ def test_parse_intermixed_positional_arguments(self): "bar", "--foo-bar", "ooga", + "b0", "b1", - "b2", "--extra-bar", "foo", - "b3", + "b2", ] ) self.assertEqual(self.config.foo_bar, "ooga") self.assertEqual(self.config.extra_bar, "foo") - self.assertEqual(self.config.positional, ["bar", "b1", "b2", "b3"]) + self.assertEqual(self.config.positional, ["bar", "b0", "b2", "b3"]) def test_parse_positional_arguments_unsaved(self): + """Ensure positional arguments are not saved to the config file.""" self.reset_config(cfg_class()) self.config_class.config = None self.write_config_file() @@ -226,6 +240,10 @@ def test_parse_positional_arguments_unsaved(self): self.assertEqual(self.config.positional, SUPPRESS) def test_parse_without_option_and_optional_argument(self): + """ + For an option with an optional argument, ensure that calling + config.load without the option will not save this value. + """ self.reset_config(OptionalArgConfiguration) self.config_filename = self.makeFile("") os.unlink(self.config_filename) @@ -245,6 +263,11 @@ def test_parse_without_option_and_optional_argument(self): self.assertConfigEqual(data, "[my-config]\ntest = ooga") def test_parse_with_option_including_optional_argument(self): + """ + For an option with an optional argument, ensure that calling + config.load with the option and setting the argument will result in the + option having the set value. + """ self.reset_config(OptionalArgConfiguration) self.config_filename = self.makeFile("") os.unlink(self.config_filename) @@ -265,6 +288,11 @@ def test_parse_with_option_including_optional_argument(self): ) def test_parse_with_option_excluding_optional_argument(self): + """ + For an option with an optional argument, ensure that calling + config.load with the option and without the argument will result in the + option having the default value + """ self.reset_config(OptionalArgConfiguration) self.config_filename = self.makeFile("") os.unlink(self.config_filename) From f27c776a7243858e13b18573212b400956074d6f Mon Sep 17 00:00:00 2001 From: HJK-X Date: Thu, 5 Sep 2024 23:00:17 +0000 Subject: [PATCH 7/7] accidentally edited this --- landscape/lib/tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/landscape/lib/tests/test_config.py b/landscape/lib/tests/test_config.py index 2e037ade..d662791b 100644 --- a/landscape/lib/tests/test_config.py +++ b/landscape/lib/tests/test_config.py @@ -224,7 +224,7 @@ def test_parse_intermixed_positional_arguments(self): ) self.assertEqual(self.config.foo_bar, "ooga") self.assertEqual(self.config.extra_bar, "foo") - self.assertEqual(self.config.positional, ["bar", "b0", "b2", "b3"]) + self.assertEqual(self.config.positional, ["bar", "b0", "b1", "b2"]) def test_parse_positional_arguments_unsaved(self): """Ensure positional arguments are not saved to the config file."""