diff --git a/codecarbon/cli/main.py b/codecarbon/cli/main.py index 93f627e5b..477be9904 100644 --- a/codecarbon/cli/main.py +++ b/codecarbon/cli/main.py @@ -358,6 +358,34 @@ def config(): ) +def _cli_provided(ctx, name: str) -> bool: + """ + Whether the option `name` was typed on the command line (or set through its + environment variable) rather than left at its Typer default. + + Options left at their default must not be forwarded to the tracker: doing so + would silently override the values coming from `.codecarbon.config` and the + `CODECARBON_*` environment variables, which the tracker reads itself. + """ + get_source = getattr(ctx, "get_parameter_source", None) + if get_source is None: + # `monitor` called directly from Python, not through Click: every value + # given is explicit. + return True + source = get_source(name) + return source is None or source.name in ("COMMANDLINE", "ENVIRONMENT") + + +def _external_config() -> dict: + """The configuration files and CODECARBON_* variables, as a plain dict.""" + from codecarbon.core.config import get_hierarchical_config + + try: + return dict(get_hierarchical_config()) + except Exception: + return {} + + @codecarbon.command( "monitor", short_help="Monitor your machine's carbon emissions.", @@ -393,15 +421,27 @@ def monitor( ): """Monitor your machine's carbon emissions.""" - # Shared tracker args so monitor and run_and_monitor behave the same + external_conf = _external_config() + + # Shared tracker args so monitor and run_and_monitor behave the same. + # Only the options actually given are forwarded: the others are left to the + # tracker, which resolves them from the configuration file and environment. tracker_args = { - "measure_power_secs": measure_power_secs, - "api_call_interval": api_call_interval, - "log_level": log_level, + name: value + for name, value in ( + ("measure_power_secs", measure_power_secs), + ("api_call_interval", api_call_interval), + ("log_level", log_level), + ) + if _cli_provided(ctx, name) } + if "log_level" not in tracker_args and "log_level" not in external_conf: + # Nothing configures it: keep the unattended monitor quiet. + tracker_args["log_level"] = log_level + # Set up the tracker arguments based on mode (offline vs online) and validate required args for each mode if offline: - if not country_iso_code: + if not country_iso_code and "country_iso_code" not in external_conf: print( "ERROR: Country ISO code is required for offline mode. Add it to your configuration or provide it via the command line: `--country-iso-code FRA`", file=sys.stderr, @@ -410,8 +450,8 @@ def monitor( tracker_args = { **tracker_args, - "country_iso_code": country_iso_code, - "region": region, + **({"country_iso_code": country_iso_code} if country_iso_code else {}), + **({"region": region} if region else {}), } else: experiment_id = get_existing_exp_id() diff --git a/codecarbon/cli/monitor.py b/codecarbon/cli/monitor.py index 41b3ca353..dbecf7855 100644 --- a/codecarbon/cli/monitor.py +++ b/codecarbon/cli/monitor.py @@ -3,6 +3,7 @@ import os import subprocess import sys +from typing import Optional import typer from rich import print @@ -12,9 +13,9 @@ def run_and_monitor( ctx: typer.Context, log_level: Annotated[ - str, + Optional[str], typer.Option(help="Log level (critical, error, warning, info, debug)"), - ] = "error", + ] = None, offline: bool = False, **tracker_args, ): @@ -51,7 +52,11 @@ def run_and_monitor( from codecarbon.emissions_tracker import EmissionsTracker, OfflineEmissionsTracker from codecarbon.external.logger import set_logger_level - set_logger_level(log_level) + # `log_level` is None when nothing set it: leave it to the tracker, which + # resolves it from the configuration file and the environment. + if log_level is not None: + set_logger_level(log_level) + tracker_args["log_level"] = log_level # Get the command from remaining args (strip nested subcommand / `--` leftovers) command = list(getattr(ctx, "args", None) or []) @@ -67,7 +72,6 @@ def run_and_monitor( tracker_cls = OfflineEmissionsTracker if offline else EmissionsTracker tracker = tracker_cls( - log_level=log_level, save_to_logger=False, tracking_mode="process", **tracker_args, diff --git a/tests/cli/test_cli_main.py b/tests/cli/test_cli_main.py index 8bb4d66f4..64efe4770 100644 --- a/tests/cli/test_cli_main.py +++ b/tests/cli/test_cli_main.py @@ -386,6 +386,94 @@ def stop(self): assert calls["kwargs"]["region"] == "IDF" +def _fake_offline_monitor(monkeypatch, tmp_path): + """Patch the offline tracker and run the monitor loop in `tmp_path`.""" + calls = {} + + class FakeOfflineTracker: + def __init__(self, **kwargs): + calls["kwargs"] = kwargs + self._another_instance_already_running = True + + def start(self): + pass + + def stop(self): + return None + + monkeypatch.setattr( + "codecarbon.emissions_tracker.OfflineEmissionsTracker", FakeOfflineTracker + ) + monkeypatch.setattr(cli_main.signal, "signal", lambda *args, **kwargs: None) + # Isolate from any config file of the user running the tests + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.chdir(tmp_path) + return calls + + +def test_monitor_does_not_override_config_with_cli_defaults(monkeypatch, tmp_path): + """Options left at their default must not shadow the config file.""" + calls = _fake_offline_monitor(monkeypatch, tmp_path) + (tmp_path / ".codecarbon.config").write_text( + "[codecarbon]\n" + "log_level = DEBUG\n" + "measure_power_secs = 30\n" + "api_call_interval = 10\n" + "country_iso_code = FRA\n" + ) + + runner = CliRunner() + result = runner.invoke(cli_main.codecarbon, ["monitor", "--offline"]) + + assert result.exit_code == 0 + # Nothing is forwarded: the tracker reads those values from the config itself + for name in ("log_level", "measure_power_secs", "api_call_interval"): + assert name not in calls["kwargs"] + + +def test_monitor_cli_options_win_over_config(monkeypatch, tmp_path): + calls = _fake_offline_monitor(monkeypatch, tmp_path) + (tmp_path / ".codecarbon.config").write_text( + "[codecarbon]\nlog_level = DEBUG\nmeasure_power_secs = 30\n" + "country_iso_code = FRA\n" + ) + + runner = CliRunner() + result = runner.invoke( + cli_main.codecarbon, + ["monitor", "--offline", "--log-level", "warning", "--measure-power-secs", "5"], + ) + + assert result.exit_code == 0 + assert calls["kwargs"]["log_level"] == "warning" + assert calls["kwargs"]["measure_power_secs"] == 5 + + +def test_monitor_stays_quiet_without_configured_log_level(monkeypatch, tmp_path): + calls = _fake_offline_monitor(monkeypatch, tmp_path) + + runner = CliRunner() + result = runner.invoke( + cli_main.codecarbon, ["monitor", "--offline", "--country-iso-code", "FRA"] + ) + + assert result.exit_code == 0 + assert calls["kwargs"]["log_level"] == "error" + + +def test_monitor_offline_accepts_country_iso_code_from_config(monkeypatch, tmp_path): + calls = _fake_offline_monitor(monkeypatch, tmp_path) + (tmp_path / ".codecarbon.config").write_text( + "[codecarbon]\ncountry_iso_code = FRA\n" + ) + + runner = CliRunner() + result = runner.invoke(cli_main.codecarbon, ["monitor", "--offline"]) + + assert result.exit_code == 0 + assert "country_iso_code" not in calls["kwargs"] + + def test_monitor_delegates_offline_flag_to_run_and_monitor(monkeypatch): captured = {} @@ -480,6 +568,16 @@ def fake_run_and_monitor(ctx, offline=False, **kwargs): assert captured["kwargs"]["log_level"] == "debug" +def test_external_config_returns_empty_dict_on_error(monkeypatch): + """A malformed config file must not crash `monitor`: fall back to `{}`.""" + + def raise_error(): + raise ValueError("malformed config file") + + monkeypatch.setattr("codecarbon.core.config.get_hierarchical_config", raise_error) + assert cli_main._external_config() == {} + + def test_monitor_online_requires_experiment_id_for_wrapped_command(monkeypatch): monkeypatch.setattr(cli_main, "get_existing_exp_id", lambda: None) diff --git a/tests/cli/test_monitor.py b/tests/cli/test_monitor.py index 0a9bda365..f3fa4852c 100644 --- a/tests/cli/test_monitor.py +++ b/tests/cli/test_monitor.py @@ -150,6 +150,54 @@ def wait(self): assert captured["kwargs"]["save_to_api"] is True +def _run_and_monitor_capturing(monkeypatch, **kwargs): + """Run `run_and_monitor` on a dummy command, capturing what it does.""" + captured = {"levels": []} + + class FakeCapturingTracker(FakeTracker): + def __init__(self, **tracker_kwargs): + captured["kwargs"] = tracker_kwargs + super().__init__() + + class FakePopen: + def __init__(self, command, text=True): + pass + + def wait(self): + return 0 + + _patch_trackers( + monkeypatch, online_cls=FakeCapturingTracker, offline_cls=FakeCapturingTracker + ) + monkeypatch.setattr(monitor_module.subprocess, "Popen", FakePopen) + monkeypatch.setattr(monitor_module, "print", lambda *args, **kwargs: None) + monkeypatch.setattr( + "codecarbon.external.logger.set_logger_level", + lambda level: captured["levels"].append(level), + ) + + with pytest.raises(typer.Exit) as exc_info: + monitor_module.run_and_monitor(SimpleNamespace(args=["echo", "hi"]), **kwargs) + + assert exc_info.value.exit_code == 0 + return captured + + +def test_run_and_monitor_leaves_log_level_to_the_config_by_default(monkeypatch): + """No log level given: the tracker resolves it from the config, not from us.""" + captured = _run_and_monitor_capturing(monkeypatch) + + assert captured["levels"] == [] + assert "log_level" not in captured["kwargs"] + + +def test_run_and_monitor_applies_given_log_level(monkeypatch): + captured = _run_and_monitor_capturing(monkeypatch, log_level="debug") + + assert captured["levels"] == ["debug"] + assert captured["kwargs"]["log_level"] == "debug" + + def test_run_and_monitor_handles_keyboard_interrupt(monkeypatch): process_info = {"terminated": 0, "killed": 0}