From 30d03b9406369388696a3522cb3f1d8c1cad5015 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Sat, 31 Jan 2026 10:07:23 -0600 Subject: [PATCH] refactor: move CLI defaults from duct_main.py to cli.py Move DUCT_OUTPUT_PREFIX and EXECUTION_SUMMARY_FORMAT out of duct_main.py and into cli.py as DEFAULT_OUTPUT_PREFIX and DEFAULT_SUMMARY_FORMAT. These are CLI defaults, not execution logic. Break the circular import (cli -> ls -> duct_main -> cli) by adding --output-prefix / -p to con-duct ls, passing the value through args so ls.py no longer imports from duct_main. Also fix a latent bug: DEFAULT_OUTPUT_PREFIX is now a plain string constant, with os.getenv() evaluated at parser creation time (after .env files are loaded) rather than at import time. Closes #386 Co-Authored-By: Claude Opus 4.5 --- src/con_duct/cli.py | 29 ++++++++++++++++++++++++++--- src/con_duct/duct_main.py | 19 ------------------- src/con_duct/ls.py | 9 +++++++-- test/duct_main/test_aggregation.py | 18 +++++++++--------- test/duct_main/test_report.py | 10 +++++----- test/test_ls.py | 30 ++++++++++++++++++++++++++++++ test/utils.py | 6 +++--- 7 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/con_duct/cli.py b/src/con_duct/cli.py index 29584ca5..a1220a8c 100644 --- a/src/con_duct/cli.py +++ b/src/con_duct/cli.py @@ -8,12 +8,28 @@ from typing import List, Optional from con_duct import __version__ from con_duct._models import Outputs, RecordTypes, SessionMode -from con_duct.duct_main import DUCT_OUTPUT_PREFIX, EXECUTION_SUMMARY_FORMAT from con_duct.duct_main import execute as duct_execute from con_duct.ls import LS_FIELD_CHOICES, ls from con_duct.plot import matplotlib_plot from con_duct.pprint_json import pprint_json +DEFAULT_OUTPUT_PREFIX = ".duct/logs/{datetime_filesafe}-{pid}_" +DEFAULT_SUMMARY_FORMAT = ( + "Summary:\n" + "Exit Code: {exit_code!E}\n" + "Command: {command}\n" + "Log files location: {logs_prefix}\n" + "Wall Clock Time: {wall_clock_time:.3f} sec\n" + "Memory Peak Usage (RSS): {peak_rss!S}\n" + "Memory Average Usage (RSS): {average_rss!S}\n" + "Virtual Memory Peak Usage (VSZ): {peak_vsz!S}\n" + "Virtual Memory Average Usage (VSZ): {average_vsz!S}\n" + "Memory Peak Percentage: {peak_pmem:.2f!N}%\n" + "Memory Average Percentage: {average_pmem:.2f!N}%\n" + "CPU Peak Usage: {peak_pcpu:.2f!N}%\n" + "Average CPU Usage: {average_pcpu:.2f!N}%\n" +) + # Default .env file search paths (in precedence order) DEFAULT_CONFIG_PATHS_LIST = ( "/etc/duct/.env", @@ -264,7 +280,7 @@ def _create_run_parser() -> argparse.ArgumentParser: "-p", "--output-prefix", type=str, - default=DUCT_OUTPUT_PREFIX, + default=os.getenv("DUCT_OUTPUT_PREFIX", DEFAULT_OUTPUT_PREFIX), help="File string format to be used as a prefix for the files -- the captured " "stdout and stderr and the resource usage logs. The understood variables are " "{datetime}, {datetime_filesafe}, and {pid}. " @@ -274,7 +290,7 @@ def _create_run_parser() -> argparse.ArgumentParser: parser.add_argument( "--summary-format", type=str, - default=os.getenv("DUCT_SUMMARY_FORMAT", EXECUTION_SUMMARY_FORMAT), + default=os.getenv("DUCT_SUMMARY_FORMAT", DEFAULT_SUMMARY_FORMAT), help="Output template to use when printing the summary following execution. " "Accepts custom conversion flags: " "!S: Converts filesizes to human readable units, green if measured, red if None. " @@ -456,6 +472,13 @@ def _create_ls_parser() -> argparse.ArgumentParser: action="store_true", help="List entries in reverse order (most recent first).", ) + parser.add_argument( + "-p", + "--output-prefix", + default=os.getenv("DUCT_OUTPUT_PREFIX", DEFAULT_OUTPUT_PREFIX), + help="Output prefix pattern used to glob for log files when no paths are given. " + "Defaults to DUCT_OUTPUT_PREFIX so ls searches where logs are written.", + ) return parser diff --git a/src/con_duct/duct_main.py b/src/con_duct/duct_main.py index 385f42fb..0e879fae 100755 --- a/src/con_duct/duct_main.py +++ b/src/con_duct/duct_main.py @@ -13,25 +13,6 @@ lgr = logging.getLogger("con-duct") -DUCT_OUTPUT_PREFIX = os.getenv( - "DUCT_OUTPUT_PREFIX", ".duct/logs/{datetime_filesafe}-{pid}_" -) -EXECUTION_SUMMARY_FORMAT = ( - "Summary:\n" - "Exit Code: {exit_code!E}\n" - "Command: {command}\n" - "Log files location: {logs_prefix}\n" - "Wall Clock Time: {wall_clock_time:.3f} sec\n" - "Memory Peak Usage (RSS): {peak_rss!S}\n" - "Memory Average Usage (RSS): {average_rss!S}\n" - "Virtual Memory Peak Usage (VSZ): {peak_vsz!S}\n" - "Virtual Memory Average Usage (VSZ): {average_vsz!S}\n" - "Memory Peak Percentage: {peak_pmem:.2f!N}%\n" - "Memory Average Percentage: {average_pmem:.2f!N}%\n" - "CPU Peak Usage: {peak_pcpu:.2f!N}%\n" - "Average CPU Usage: {average_pcpu:.2f!N}%\n" -) - def execute( command: str, diff --git a/src/con_duct/ls.py b/src/con_duct/ls.py index 37b21e8c..c11f037c 100644 --- a/src/con_duct/ls.py +++ b/src/con_duct/ls.py @@ -9,7 +9,6 @@ from con_duct._constants import __schema_version__ from con_duct._formatter import SummaryFormatter from con_duct._utils import parse_version -from con_duct.duct_main import DUCT_OUTPUT_PREFIX from con_duct.json_utils import is_info_file try: @@ -220,7 +219,13 @@ def pyout_ls(run_data_list: List[OrderedDict[str, Any]], enable_colors: bool) -> def ls(args: argparse.Namespace) -> int: if not args.paths: - pattern = f"{DUCT_OUTPUT_PREFIX[:DUCT_OUTPUT_PREFIX.index('{')]}*" + # The default search prefix contains {datetime_filesafe}, {pid}, etc. + # Strip from the first '{' onward to get a static glob prefix. + try: + prefix = args.output_prefix[: args.output_prefix.index("{")] + except ValueError: + prefix = args.output_prefix + pattern = f"{prefix}*" args.paths = [p for p in glob.glob(pattern)] if args.format == "auto": diff --git a/test/duct_main/test_aggregation.py b/test/duct_main/test_aggregation.py index 5b9857c9..a2fc8759 100644 --- a/test/duct_main/test_aggregation.py +++ b/test/duct_main/test_aggregation.py @@ -6,7 +6,7 @@ import pytest from con_duct._models import ProcessStats, Sample from con_duct._tracker import Report -from con_duct.duct_main import EXECUTION_SUMMARY_FORMAT +from con_duct.cli import DEFAULT_SUMMARY_FORMAT stat0 = ProcessStats( pcpu=0.0, @@ -70,7 +70,7 @@ def test_aggregation_num_samples_increment(mock_log_paths: mock.MagicMock) -> No mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -98,7 +98,7 @@ def test_aggregation_single_sample_sanity(mock_log_paths: mock.MagicMock) -> Non mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -135,7 +135,7 @@ def test_aggregation_single_stat_multiple_samples_sanity( mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -184,7 +184,7 @@ def test_aggregation_averages(mock_log_paths: mock.MagicMock) -> None: mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -256,7 +256,7 @@ def test_aggregation_current_ave_diverges_from_total_ave( mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -323,7 +323,7 @@ def test_aggregation_many_samples( mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -364,7 +364,7 @@ def test_aggregation_sample_no_pids(mock_log_paths: mock.MagicMock) -> None: mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) # When there are no pids, finalization should be triggered because the exe is finished, # so a Sample with no PIDs should never be passed to update_from_sample. @@ -379,7 +379,7 @@ def test_aggregation_no_false_peak(mock_log_paths: mock.MagicMock) -> None: mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) sample1.add_pid(1, deepcopy(stat100)) sample1.add_pid(2, deepcopy(stat0)) diff --git a/test/duct_main/test_report.py b/test/duct_main/test_report.py index 52377725..cb9e1ec2 100644 --- a/test/duct_main/test_report.py +++ b/test/duct_main/test_report.py @@ -8,7 +8,7 @@ import pytest from con_duct._models import Averages, ProcessStats, Sample from con_duct._tracker import Report -from con_duct.duct_main import EXECUTION_SUMMARY_FORMAT +from con_duct.cli import DEFAULT_SUMMARY_FORMAT stat0 = ProcessStats( pcpu=0.0, @@ -219,7 +219,7 @@ def test_system_info_sanity(mock_log_paths: mock.MagicMock) -> None: mock_log_paths.prefix = "mock_prefix" cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) report.get_system_info() assert report.system_info is not None @@ -242,7 +242,7 @@ def test_gpu_parsing_green( ).encode("utf-8") cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) report.get_system_info() assert report.gpus is not None @@ -271,7 +271,7 @@ def test_gpu_call_error( mock_sp.side_effect = subprocess.CalledProcessError(1, "errrr") cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) report.get_system_info() assert report.gpus is None @@ -294,7 +294,7 @@ def test_gpu_parse_error( ).encode("utf-8") cwd = os.getcwd() report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False + "_cmd", [], mock_log_paths, DEFAULT_SUMMARY_FORMAT, cwd, clobber=False ) report.get_system_info() assert report.gpus is None diff --git a/test/test_ls.py b/test/test_ls.py index 7d7dd232..adfa715f 100644 --- a/test/test_ls.py +++ b/test/test_ls.py @@ -198,6 +198,11 @@ def setUp(self) -> None: "execution_summary": {}, "prefix": "default_file1", }, + "custom/prefix_abc123_info.json": { + "schema_version": MINIMUM_SCHEMA_VERSION, + "execution_summary": {}, + "prefix": "custom_prefix_file", + }, } for filename, content in self.files.items(): full_path = os.path.join(self.temp_dir.name, filename) @@ -223,6 +228,7 @@ def _run_ls( format=fmt, func=ls, reverse=False, + output_prefix=".duct/logs/{datetime_filesafe}-{pid}_", ) buf = StringIO() with contextlib.redirect_stdout(buf): @@ -255,6 +261,7 @@ def test_ls_with_filter(self) -> None: format="summaries", func=ls, reverse=False, + output_prefix=".duct/logs/{datetime_filesafe}-{pid}_", ) result = self._run_ls(paths, "summaries", args) @@ -286,6 +293,28 @@ def test_ls_no_pos_args(self) -> None: assert "file3" not in result assert "not_matching.json" not in result + def test_ls_custom_output_prefix(self) -> None: + """Test --output-prefix finds files matching the custom prefix.""" + args = argparse.Namespace( + paths=[], + colors=False, + fields=["prefix", "schema_version"], + eval_filter=None, + format="summaries", + func=ls, + reverse=False, + output_prefix="custom/prefix_{id}_", + ) + buf = StringIO() + with contextlib.redirect_stdout(buf): + exit_code = ls(args) + assert exit_code == 0 + result = buf.getvalue().strip() + + assert "Prefix:" in result + assert "custom/prefix_abc123_" in result + assert "default_logpath" not in result + def test_ls_multiple_paths(self) -> None: """Basic sanity test to ensure ls() runs without crashing.""" files_1_and_2 = ["file1_info.json", "file2_info.json"] @@ -383,6 +412,7 @@ def test_ls_reverse(self) -> None: format="json", func=ls, reverse=True, + output_prefix=".duct/logs/{datetime_filesafe}-{pid}_", ) result_reversed = self._run_ls(paths, "json", args) parsed_reversed = json.loads(result_reversed) diff --git a/test/utils.py b/test/utils.py index 2cc04f83..4c747c95 100644 --- a/test/utils.py +++ b/test/utils.py @@ -15,14 +15,14 @@ def run_duct_command(cli_args: list[str], **kwargs: Any) -> int: Exit code from the executed command """ from con_duct._models import Outputs, RecordTypes, SessionMode - from con_duct.duct_main import DUCT_OUTPUT_PREFIX, EXECUTION_SUMMARY_FORMAT + from con_duct.cli import DEFAULT_OUTPUT_PREFIX, DEFAULT_SUMMARY_FORMAT from con_duct.duct_main import execute as duct_execute command = cli_args[0] command_args = cli_args[1:] if len(cli_args) > 1 else [] defaults = { - "output_prefix": DUCT_OUTPUT_PREFIX, + "output_prefix": DEFAULT_OUTPUT_PREFIX, "sample_interval": 1.0, "report_interval": 60.0, "fail_time": 3.0, @@ -30,7 +30,7 @@ def run_duct_command(cli_args: list[str], **kwargs: Any) -> int: "capture_outputs": Outputs.ALL, "outputs": Outputs.ALL, "record_types": RecordTypes.ALL, - "summary_format": EXECUTION_SUMMARY_FORMAT, + "summary_format": DEFAULT_SUMMARY_FORMAT, "colors": False, "mode": SessionMode.NEW_SESSION, "message": "",