Merge pull request #19 from affenull2345/type-fixes

Fix type errors to keep pyright checks green

setup.cfg

@@ -45,7 +45,7 @@ tests_require =
 
 lint_requires =
   importlib-metadata>=3.10.1
-  importlib-resources>=5.4.0
+  importlib-resources==5.4.0
   pre-commit>=2.0.1
   black>=22.1.0
   pyright>=0.0.13

src/cobbler_tftp/cli.py

@@ -2,6 +2,9 @@
 Cobbler-tftp will be managable as a command-line service.
 """
 
+from pathlib import Path
+from typing import List, Optional
+
 import click
 import yaml
 
@@ -17,14 +20,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"])
 
 
@@ -45,17 +40,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."
@@ -68,25 +63,31 @@ 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: Optional[bool],
+    enable_automigration: Optional[bool],
+    config: Optional[str],
+    settings: List[str],
+):
     """
     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
-        )
+    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
+    # )
+    if config is None:
+        config_path = None
     else:
-        click.echo("Cobbler-tftp will be running as a daemon in the background.")
+        config_path = Path(config)
+    application_settings = SettingsFactory.build_settings(
+        settings_factory, config_path, daemon, enable_automigration, settings
+    )
+    print(application_settings)
 
 
 @cli.command()
@@ -103,7 +104,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()

src/cobbler_tftp/settings/__init__.py

@@ -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.
@@ -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(
@@ -165,15 +169,15 @@ def load_config_file(self, config_path: Union[Path, None]) -> SettingsDict:
                 config_file_content = (
                     files("cobbler_tftp.settings.data")
                     .joinpath("settings.yml")
-                    .read_text(encoding="UTF-8")
+                    .read_text(encoding="UTF-8")  # type: ignore
                 )
                 self._settings_dict = yaml.safe_load(config_file_content)
             except yaml.YAMLError:
                 print(f"Error: No valid configuration file found at {config_path}!")
         elif config_path and Path.exists(config_path):
             try:
                 config_file_content = (
-                    files(config_import_path).joinpath(config_file).read_text("utf-8")
+                    files(config_import_path).joinpath(config_file).read_text("utf-8")  # type: ignore
                 )
                 self._settings_dict = yaml.safe_load(config_file_content)
             except yaml.YAMLError:
@@ -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.
@@ -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)
 

tests/unittests/application_settings/test_settings.py

@@ -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)
@@ -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
@@ -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(
@@ -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()