Refactor migrate_config (#202)

unioslo · Sep 9, 2024 · 0266490 · 0266490
1 parent 2294842
commit 0266490
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 24 deletions.
diff --git a/zabbix_cli/commands/cli.py b/zabbix_cli/commands/cli.py
@@ -240,13 +240,13 @@ def init(
 def migrate_config(
     ctx: typer.Context,
     source: Optional[Path] = typer.Option(
-        None, "--source", "-s", help="Location of the config file."
+        None, "--source", "-s", help="Location of the config file to migrate."
     ),
     destination: Optional[Path] = typer.Option(
         None,
         "--destination",
         "-d",
-        help="Location of the new config file. Uses the default config path if not specified.",
+        help="Path of the new config file to create. Uses the default config path if not specified.",
     ),
     overwrite: bool = typer.Option(
         False, "--overwrite", help="Overwrite destination config file if it exists."
@@ -267,13 +267,18 @@ def migrate_config(
 
     if source:
         conf = Config.from_file(source)
-        if not conf.app.is_legacy:
-            exit_err(f"Config file {source} is not a legacy .conf file.")
     else:
+        if not app.state.is_config_loaded:
+            # this should never happen!
+            exit_err(
+                "Application was unable to load a config. Use [option]--source[/] to specify one."
+            )
         conf = app.state.config
-        if not conf.app.is_legacy:
-            p = f"'{conf.config_path}' " if conf.config_path else ""
-            exit_err(f"Loaded config {p}is not a legacy .conf config file.")
+
+    if not conf.app.is_legacy:
+        exit_err(
+            "Unable to detect legacy config file. Use [option]--source[/] to specify one."
+        )
 
     if not destination:
         destination = DEFAULT_CONFIG_FILE
@@ -289,9 +294,5 @@ def migrate_config(
     # By default, we move users over to the new format.
     conf.app.legacy_json_format = legacy_json
 
-    try:
-        conf.dump_to_file(destination)
-    except Exception as e:
-        exit_err(f"Unable to create config file {destination}: {e}", exception=e)
-    else:
-        success(f"Config migrated to {destination}")
+    conf.dump_to_file(destination)
+    success(f"Config migrated to {destination}")
diff --git a/zabbix_cli/config/model.py b/zabbix_cli/config/model.py
@@ -337,7 +337,7 @@ def from_toml_file(cls, filename: Path) -> Config:
     @classmethod
     def from_conf_file(cls, filename: Path) -> Config:
         """Load configuration from a legacy .conf file."""
-        logging.warning("Using legacy config file (%s)", filename)
+        logging.info("Using legacy config file (%s)", filename)
         conf = load_config_conf(filename)
         # Use legacy JSON format if we load from a legacy .conf file
         # and mark the loaded config as stemming from a legacy config file
@@ -359,11 +359,12 @@ def _assign_legacy_options(self) -> Self:
         """Ensures that options that have moved from one section to another are copied
         to the new section. I.e. `app.username` -> `api.username`.
         """
-        # Only override if `api.username` is not set
+        # Only override if `api.username` is not set (and not using legacy config file)
         if self.app.username:
-            logging.warning(
-                "Config option `app.username` is deprecated and will be removed. Use `api.username` instead."
-            )
+            if not self.app.is_legacy:
+                logging.warning(
+                    "Config option `app.username` is deprecated and will be removed. Use `api.username` instead."
+                )
             self.api.username = self.app.username
         if not self.api.username:
             raise ConfigError("No username specified in the configuration file.")
@@ -373,13 +374,16 @@ def as_toml(self, secrets: bool = False) -> str:
         """Dump the configuration to a TOML string."""
         import tomli_w
 
-        return tomli_w.dumps(
-            self.model_dump(
-                mode="json",
-                exclude_none=True,  # we shouldn't have any, but just in case
-                context={"secrets": secrets},
+        try:
+            return tomli_w.dumps(
+                self.model_dump(
+                    mode="json",
+                    exclude_none=True,  # we shouldn't have any, but just in case
+                    context={"secrets": secrets},
+                )
             )
-        )
+        except Exception as e:
+            raise ConfigError(f"Failed to serialize configuration to TOML: {e}") from e
 
     def dump_to_file(self, filename: Path) -> None:
         """Dump the configuration to a TOML file."""