From f57ea15865b5a5bc64fab8bedb1de6aa81a7ef7f Mon Sep 17 00:00:00 2001 From: klemen1999 Date: Thu, 16 Jul 2026 14:37:17 +0200 Subject: [PATCH 1/2] Added Enums to telemetry for better code quality --- modelconverter/__main__.py | 28 +++--- modelconverter/utils/telemetry.py | 138 ++++++++++++++++++++--------- tests/test_utils/test_telemetry.py | 37 +++++--- 3 files changed, 140 insertions(+), 63 deletions(-) diff --git a/modelconverter/__main__.py b/modelconverter/__main__.py index c4b51cc5..58b062da 100644 --- a/modelconverter/__main__.py +++ b/modelconverter/__main__.py @@ -47,6 +47,11 @@ CONFIGURED_EVENT, CONVERSION_RUN_ID_ENV_VAR, RESULT_EVENT, + ArchiveOutputMode, + CommandResult, + ConversionPhase, + FailureReason, + TelemetryFlowStep, build_command_properties, build_conversion_result_properties, build_conversion_summary, @@ -148,7 +153,7 @@ def handle_signal(signum: int, frame: Any) -> None: output_artifact_count: int | None = None uploaded_output = False uploaded_intermediate_outputs = False - phase = "configuration" + phase = ConversionPhase.CONFIGURATION caught_exc: BaseException | None = None try: @@ -212,14 +217,14 @@ def handle_signal(signum: int, frame: Any) -> None: conversion_summary = build_flow_properties( conversion_run_id, - "configuration_resolved", + TelemetryFlowStep.CONFIGURATION_RESOLVED, build_conversion_summary( cfg, target=target, config_source=detect_config_source( original_path, opts, archive_cfg ), - archive_output_mode=to, + archive_output_mode=ArchiveOutputMode(to), archive_preprocess=archive_preprocess, main_stage_provided=main_stage_provided, ), @@ -232,7 +237,7 @@ def handle_signal(signum: int, frame: Any) -> None: ) conversion_start = time.monotonic() - phase = "conversion" + phase = ConversionPhase.CONVERSION out_models = exporter.run() if not isinstance(out_models, list): out_models = [out_models] @@ -280,7 +285,7 @@ def handle_signal(signum: int, frame: Any) -> None: put_file_plugin = _cfg.put_file_plugin if upload_url is not None: - phase = "upload_output" + phase = ConversionPhase.UPLOAD_OUTPUT for model_path in out_models: logger.info(f"Uploading {model_path} to {upload_url}") upload_to_remote( @@ -291,7 +296,7 @@ def handle_signal(signum: int, frame: Any) -> None: uploaded_output = True if intermediate_url is not None: - phase = "upload_intermediate" + phase = ConversionPhase.UPLOAD_INTERMEDIATE exporters = ( exporter.exporters.values() if isinstance(exporter, MultiStageExporter) @@ -337,7 +342,7 @@ def handle_signal(signum: int, frame: Any) -> None: RESULT_EVENT, build_flow_properties( conversion_run_id, - "result_recorded", + TelemetryFlowStep.RESULT_RECORDED, { **( { @@ -350,12 +355,13 @@ def handle_signal(signum: int, frame: Any) -> None: ), **build_conversion_result_properties( result=( - "success" + CommandResult.SUCCESS if caught_exc is None else ( - "interrupted" - if failure_reason == "user_interrupt" - else "failed" + CommandResult.INTERRUPTED + if failure_reason + is FailureReason.USER_INTERRUPT + else CommandResult.FAILED ) ), failure_reason=failure_reason, diff --git a/modelconverter/utils/telemetry.py b/modelconverter/utils/telemetry.py index 4456257b..3088bb4e 100644 --- a/modelconverter/utils/telemetry.py +++ b/modelconverter/utils/telemetry.py @@ -1,6 +1,7 @@ import os import resource import sys +from enum import Enum from pathlib import Path from typing import Any from uuid import uuid4 @@ -45,6 +46,54 @@ ) +class TelemetryFlowStep(str, Enum): + CONFIGURATION_RESOLVED = "configuration_resolved" + RESULT_RECORDED = "result_recorded" + + +class CommandName(str, Enum): + CONVERT = "convert" + + +class CommandResult(str, Enum): + SUCCESS = "success" + INTERRUPTED = "interrupted" + FAILED = "failed" + + +class FailureReason(str, Enum): + USER_INTERRUPT = "user_interrupt" + RUNTIME_ERROR = "runtime_error" + CONFIG_ERROR = "config_error" + UPLOAD_ERROR = "upload_error" + CONVERSION_ERROR = "conversion_error" + + +class ConversionPhase(str, Enum): + CONFIGURATION = "configuration" + CONVERSION = "conversion" + UPLOAD_OUTPUT = "upload_output" + UPLOAD_INTERMEDIATE = "upload_intermediate" + + +class ConfigSource(str, Enum): + DIRECT_MODEL_INPUT = "direct_model_input" + YAML_CONFIG = "yaml_config" + NN_ARCHIVE = "nn_archive" + ARCHIVE_DIRECTORY = "archive_directory" + + +class ArchiveOutputMode(str, Enum): + NATIVE = "native" + NN_ARCHIVE = "nn_archive" + + +class CalibrationSource(str, Enum): + IMAGE_DIRECTORY = "image_directory" + RANDOM = "random" + REMOTE_LINK = "remote_link" + + def get_conversion_run_id() -> str: conversion_run_id = os.environ.get(CONVERSION_RUN_ID_ENV_VAR) if conversion_run_id: @@ -93,14 +142,14 @@ def build_command_properties( custom_image_provided: bool, memory_limit_set: bool, cpu_limit_set: bool, - result: str, + result: CommandResult, duration_ms: int, - failure_reason: str | None = None, + failure_reason: FailureReason | None = None, ) -> dict[str, Any]: return _drop_none( { "conversion_run_id": conversion_run_id, - "command_name": "convert", + "command_name": CommandName.CONVERT.value, "target": target.value, "runs_in_docker": runs_in_docker, "dev_image": dev_image, @@ -109,8 +158,8 @@ def build_command_properties( "custom_image_provided": custom_image_provided, "memory_limit_set": memory_limit_set, "cpu_limit_set": cpu_limit_set, - "result": result, - "failure_reason": failure_reason, + "result": result.value, + "failure_reason": failure_reason.value if failure_reason else None, "duration_ms": duration_ms, } ) @@ -120,8 +169,8 @@ def build_conversion_summary( cfg: Config, *, target: Target, - config_source: str, - archive_output_mode: str, + config_source: ConfigSource, + archive_output_mode: ArchiveOutputMode, archive_preprocess: bool, main_stage_provided: bool, ) -> dict[str, Any]: @@ -145,11 +194,11 @@ def build_conversion_summary( return _drop_none( { "target": target.value, - "config_source": config_source, + "config_source": config_source.value, "stage_count_bucket": bucket_count(len(stages)), "is_multistage": len(stages) > 1, "main_stage_provided": main_stage_provided, - "archive_output_mode": archive_output_mode, + "archive_output_mode": archive_output_mode.value, "archive_preprocess": archive_preprocess, "input_model_format": ( input_format_values[0] if input_format_values else None @@ -191,30 +240,32 @@ def build_conversion_summary( def build_flow_properties( - conversion_run_id: str, flow_step: str, properties: dict[str, Any] + conversion_run_id: str, + flow_step: TelemetryFlowStep, + properties: dict[str, Any], ) -> dict[str, Any]: return { "flow_name": FLOW_NAME, "conversion_run_id": conversion_run_id, - "flow_step": flow_step, + "flow_step": flow_step.value, **properties, } def build_conversion_result_properties( *, - result: str, + result: CommandResult, duration_ms: int, uploaded_output: bool, uploaded_intermediate_outputs: bool, - failure_reason: str | None = None, + failure_reason: FailureReason | None = None, output_artifact_count: int | None = None, peak_ram_bytes: int | None = None, ) -> dict[str, Any]: return _drop_none( { - "result": result, - "failure_reason": failure_reason, + "result": result.value, + "failure_reason": failure_reason.value if failure_reason else None, "duration_ms": duration_ms, "output_artifact_count_bucket": ( bucket_count(output_artifact_count) @@ -232,23 +283,25 @@ def build_conversion_result_properties( ) -def command_result_from_exception(exc: BaseException | None) -> str: +def command_result_from_exception( + exc: BaseException | None, +) -> CommandResult: if exc is None: - return "success" + return CommandResult.SUCCESS code = getattr(exc, "code", None) if isinstance(exc, SystemExit) and code in {None, 0}: - return "success" + return CommandResult.SUCCESS if isinstance(exc, (KeyboardInterrupt, SystemExit)) and code in { None, 130, }: - return "interrupted" - return "failed" + return CommandResult.INTERRUPTED + return CommandResult.FAILED def command_failure_reason_from_exception( exc: BaseException | None, -) -> str | None: +) -> FailureReason | None: if exc is None: return None code = getattr(exc, "code", None) @@ -258,24 +311,27 @@ def command_failure_reason_from_exception( None, 130, }: - return "user_interrupt" - return "runtime_error" + return FailureReason.USER_INTERRUPT + return FailureReason.RUNTIME_ERROR def runtime_failure_reason_from_exception( - exc: BaseException | None, *, phase: str -) -> str | None: + exc: BaseException | None, *, phase: ConversionPhase +) -> FailureReason | None: if exc is None: return None if isinstance(exc, (KeyboardInterrupt, SystemExit)) and getattr( exc, "code", None ) in {None, 130}: - return "user_interrupt" - if phase == "configuration": - return "config_error" - if phase.startswith("upload"): - return "upload_error" - return "conversion_error" + return FailureReason.USER_INTERRUPT + if phase is ConversionPhase.CONFIGURATION: + return FailureReason.CONFIG_ERROR + if phase in { + ConversionPhase.UPLOAD_OUTPUT, + ConversionPhase.UPLOAD_INTERMEDIATE, + }: + return FailureReason.UPLOAD_ERROR + return FailureReason.CONVERSION_ERROR def resolve_target_tool_version( @@ -290,16 +346,16 @@ def detect_config_source( path: str | None, opts: list[str], archive_cfg: NNArchiveConfig | None, -) -> str: +) -> ConfigSource: if archive_cfg is not None: if path and is_nn_archive(path): - return "nn_archive" - return "archive_directory" + return ConfigSource.NN_ARCHIVE + return ConfigSource.ARCHIVE_DIRECTORY if path and _looks_like_model_input(path): - return "direct_model_input" + return ConfigSource.DIRECT_MODEL_INPUT if "input_model" in opts[::2]: - return "direct_model_input" - return "yaml_config" + return ConfigSource.DIRECT_MODEL_INPUT + return ConfigSource.YAML_CONFIG def _looks_like_model_input(path: str) -> bool: @@ -405,13 +461,13 @@ def _target_configuration( return None -def _calibration_source(calibration: Any) -> str | None: +def _calibration_source(calibration: Any) -> CalibrationSource | None: if isinstance(calibration, ImageCalibrationConfig): - return "image_directory" + return CalibrationSource.IMAGE_DIRECTORY if isinstance(calibration, RandomCalibrationConfig): - return "random" + return CalibrationSource.RANDOM if isinstance(calibration, LinkCalibrationConfig): - return "remote_link" + return CalibrationSource.REMOTE_LINK return None diff --git a/tests/test_utils/test_telemetry.py b/tests/test_utils/test_telemetry.py index f8f0b451..4c10c6be 100644 --- a/tests/test_utils/test_telemetry.py +++ b/tests/test_utils/test_telemetry.py @@ -4,6 +4,7 @@ import pytest import yaml +from luxonis_ml.nn_archive.config import Config as NNArchiveConfig from onnx import checker, helper from onnx.onnx_pb import TensorProto @@ -11,6 +12,10 @@ from modelconverter.utils.docker_utils import generate_compose_config from modelconverter.utils.telemetry import ( MODELCONVERTER_TELEMETRY_DEFAULTS, + ArchiveOutputMode, + CommandResult, + ConfigSource, + FailureReason, build_conversion_result_properties, build_conversion_summary, command_failure_reason_from_exception, @@ -82,8 +87,8 @@ def test_build_conversion_summary_rvc4(tmp_path: Path) -> None: summary = build_conversion_summary( cfg, target=Target.RVC4, - config_source="direct_model_input", - archive_output_mode="nn_archive", + config_source=ConfigSource.DIRECT_MODEL_INPUT, + archive_output_mode=ArchiveOutputMode.NN_ARCHIVE, archive_preprocess=False, main_stage_provided=True, ) @@ -122,28 +127,36 @@ def test_build_conversion_summary_rvc4(tmp_path: Path) -> None: def test_detect_config_source(tmp_path: Path) -> None: model_path = tmp_path / "dummy_model.onnx" archive_path = tmp_path / "archive.tar.xz" + archive_cfg = NNArchiveConfig.model_construct() _create_dummy_onnx(model_path) _create_dummy_archive(archive_path, model_path) - assert detect_config_source("model.onnx", [], None) == "direct_model_input" - assert detect_config_source("config.yaml", [], None) == "yaml_config" + assert ( + detect_config_source("model.onnx", [], None) + is ConfigSource.DIRECT_MODEL_INPUT + ) + assert ( + detect_config_source("config.yaml", [], None) + is ConfigSource.YAML_CONFIG + ) assert ( detect_config_source(None, ["input_model", "model.onnx"], None) - == "direct_model_input" + is ConfigSource.DIRECT_MODEL_INPUT ) assert ( - detect_config_source(str(archive_path), [], object()) == "nn_archive" + detect_config_source(str(archive_path), [], archive_cfg) + is ConfigSource.NN_ARCHIVE ) assert ( - detect_config_source("archive_dir", [], object()) - == "archive_directory" + detect_config_source("archive_dir", [], archive_cfg) + is ConfigSource.ARCHIVE_DIRECTORY ) def test_build_conversion_result_properties(): properties = build_conversion_result_properties( - result="failed", - failure_reason="upload_error", + result=CommandResult.FAILED, + failure_reason=FailureReason.UPLOAD_ERROR, duration_ms=1234, output_artifact_count=3, uploaded_output=True, @@ -163,7 +176,9 @@ def test_build_conversion_result_properties(): def test_command_result_from_exception_treats_system_exit_zero_as_success(): - assert command_result_from_exception(SystemExit(0)) == "success" + assert ( + command_result_from_exception(SystemExit(0)) is CommandResult.SUCCESS + ) assert command_failure_reason_from_exception(SystemExit(0)) is None From 3dc5b7917487c004162cf6b13d3cd3e257af9321 Mon Sep 17 00:00:00 2001 From: klemen1999 Date: Thu, 16 Jul 2026 19:07:54 +0200 Subject: [PATCH 2/2] Release v0.5.7 --- modelconverter/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modelconverter/__init__.py b/modelconverter/__init__.py index 20bae98c..b0d90909 100644 --- a/modelconverter/__init__.py +++ b/modelconverter/__init__.py @@ -4,7 +4,7 @@ from luxonis_ml.utils import PUT_FILE_REGISTRY from pydantic_extra_types.semantic_version import SemanticVersion -__version__: Final[str] = "0.5.6" +__version__: Final[str] = "0.5.7" __semver__: Final[SemanticVersion] = SemanticVersion.parse(__version__)