Skip to content

Commit

Permalink
Use consistent types for CLI configuration
Browse files Browse the repository at this point in the history
Add enable/disable options for configurable booleans and pass all of them
to build_settings and load_cli_options and fix the type errors in these
functions.
  • Loading branch information
affenull2345 committed Jan 8, 2024
1 parent 605a974 commit b591ad6
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 44 deletions.
49 changes: 19 additions & 30 deletions src/cobbler_tftp/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@
except importlib_metadata.PackageNotFoundError:
__version__ = "unknown (not installed)"

with open(
"src/cobbler_tftp/settings/data/settings.yml", "r", encoding="utf-8"
) as stream:
try:
SETTINGS = yaml.safe_load(stream)
except yaml.YAMLError as exc:
print(exc)

_context_settings = dict(help_option_names=["-h", "--help"])


Expand All @@ -45,17 +37,17 @@ def cli(ctx):

@cli.command()
@click.option(
"--no-daemon",
"-dd",
"--daemon/--no-daemon",
"-d/-dd",
is_flag=True,
default=not SETTINGS["is_daemon"], # type: ignore
help="Stop cobbler-tftp from running as daemon.",
default=None,
help="Force cobbler-tftp to run as daemon or not.",
)
@click.option(
"--enable-automigration",
"--enable-automigration/--disable-automigration",
is_flag=True,
default=SETTINGS["auto_migrate_settings"], # type: ignore
help="Enable auto migration of settings.",
default=None,
help="Enable or disable auto migration of settings.",
)
@click.option(
"--config", "-c", type=click.Path(), help="Set location of configuration file."
Expand All @@ -68,25 +60,22 @@ def cli(ctx):
<PARENT_YAML_KEY>.<CHILD_YAML_KEY>.<...>.<KEY_NAME>=<VALUE>.\n
Your settings must use single quotes. If a single quote appears within a value it must be escaped.""",
)
def start(no_daemon: bool, enable_automigration, config, settings):
def start(daemon, enable_automigration, config, settings):
"""
Start the cobbler-tftp server.
"""
click.echo(cli.__doc__)
click.echo("Initializing Cobbler-tftp server...")
if no_daemon:
click.echo("'--no-daemon' flag set. Server running in the foreground...")
settings_factory: SettingsFactory = SettingsFactory()
# settings_file = SettingsFactory.load_config_file(settings_factory, config)
# environment_variables = SettingsFactory.load_env_variables(settings_factory)
# cli_arguments = SettingsFactory.load_cli_options(
# settings_factory, daemon, enable_automigration, settings
# )
application_settings = SettingsFactory.build_settings(
settings_factory, config, settings
)
else:
click.echo("Cobbler-tftp will be running as a daemon in the background.")
settings_factory: SettingsFactory = SettingsFactory()
# settings_file = SettingsFactory.load_config_file(settings_factory, config)
# environment_variables = SettingsFactory.load_env_variables(settings_factory)
# cli_arguments = SettingsFactory.load_cli_options(
# settings_factory, daemon, enable_automigration, settings
# )
application_settings = SettingsFactory.build_settings(
settings_factory, config, daemon, enable_automigration, settings
)
print(application_settings)


@cli.command()
Expand All @@ -103,7 +92,7 @@ def print_default_config():
Print the default application parameters.
"""
settings_factory: SettingsFactory = SettingsFactory()
click.echo(settings_factory.build_settings(None, []))
click.echo(settings_factory.build_settings(None))


@cli.command()
Expand Down
19 changes: 9 additions & 10 deletions src/cobbler_tftp/settings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ def __init__(self) -> None:
self._settings_dict: SettingsDict = {}

def build_settings(
self, config_path: Optional[Path], cli_arguments: List[str]
self,
config_path: Optional[Path],
daemon: Optional[bool] = None,
enable_automigration: Optional[bool] = None,
cli_arguments: List[str] = [],
) -> Settings:
"""
Build new Settings object using parameters from all sources.
Expand All @@ -110,7 +114,7 @@ def build_settings(
self.load_env_variables()

# Load CLI options
self.load_cli_options(cli_arguments)
self.load_cli_options(daemon, enable_automigration, cli_arguments)

if not migrations.validate(self._settings_dict):
raise ValueError(
Expand Down Expand Up @@ -219,7 +223,7 @@ def load_cli_options(
self,
daemon: Optional[bool] = None,
enable_automigration: Optional[bool] = None,
settings: Optional[Dict[str, Union[str, Path]]] = None,
settings: List[str] = [],
) -> SettingsDict:
"""
Get parameters and flags from CLI.
Expand All @@ -234,16 +238,11 @@ def load_cli_options(
:return _settings_dict: Settings dictionary.
"""

if not daemon and not enable_automigration and not settings:
return self._settings_dict

if daemon:
if daemon is not None:
self._settings_dict["is_daemon"] = daemon
if enable_automigration:
if enable_automigration is not None:
self._settings_dict["auto_migrate_settings"] = enable_automigration

if settings is None:
raise ValueError
for setting in settings:
option_list = setting.split("=", 1)

Expand Down
8 changes: 4 additions & 4 deletions tests/unittests/application_settings/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def test_build_settings_with_default_config_file(
config file
"""
# Call the build_settings method with None as the config_path argument
settings = settings_factory.build_settings(None, None)
settings = settings_factory.build_settings(None)

# Assert that the expected values are set in the Settings object
assert isinstance(settings, Settings)
Expand All @@ -42,7 +42,7 @@ def test_build_settings_with_valid_config_file(
settings_factory: SettingsFactory, mocker
):
valid_file_path = Path("tests/test_data/valid_config.yml")
settings = settings_factory.build_settings(valid_file_path, None)
settings = settings_factory.build_settings(valid_file_path)

assert isinstance(settings, Settings)
assert settings.auto_migrate_settings is True
Expand All @@ -60,7 +60,7 @@ def test_build_settings_with_invalid_config_file(
path = Path("tests/test_data/invalid_config.yml")

with pytest.raises(ValueError) as exc:
settings_factory.build_settings(path, None)
settings_factory.build_settings(path)

assert """Validation Error: Configuration Parameters could not be validated!\n
This may be due to an invalid configuration file or path.""" in str(
Expand All @@ -83,7 +83,7 @@ def test_build_settings_with_missing_config_file(
expected_message = f"Warning: No configuration file found at {path}! Using default configuration file...\n"

# Act
settings = settings_factory.build_settings(path, None)
settings = settings_factory.build_settings(path)

# Assert
captured_message = capsys.readouterr()
Expand Down

0 comments on commit b591ad6

Please sign in to comment.