diff --git a/scrapegraphai/telemetry/telemetry.py b/scrapegraphai/telemetry/telemetry.py index 07fa6e080..fba586b33 100644 --- a/scrapegraphai/telemetry/telemetry.py +++ b/scrapegraphai/telemetry/telemetry.py @@ -36,6 +36,19 @@ def _load_config(config_location: str) -> configparser.ConfigParser: return config +def _parse_bool(value: str) -> bool: + """Parse a boolean from a string using configparser's accepted spellings. + + Accepts the same values as the config file does, so + ``SCRAPEGRAPHAI_TELEMETRY_ENABLED=false`` and ``telemetry_enabled = false`` + behave identically. Raises ValueError on anything unrecognised. + """ + try: + return configparser.ConfigParser.BOOLEAN_STATES[value.strip().lower()] + except KeyError: + raise ValueError(f"invalid boolean value: {value!r}") + + def _check_config_and_environ_for_telemetry_flag(default_value: bool, config_obj): telemetry_enabled = default_value if "telemetry_enabled" in config_obj["DEFAULT"]: @@ -44,13 +57,18 @@ def _check_config_and_environ_for_telemetry_flag(default_value: bool, config_obj except Exception: pass - if os.environ.get("SCRAPEGRAPHAI_TELEMETRY_ENABLED") is not None: + env_value = os.environ.get("SCRAPEGRAPHAI_TELEMETRY_ENABLED") + if env_value is not None: try: - telemetry_enabled = config_obj.getboolean( - "DEFAULT", "telemetry_enabled" + telemetry_enabled = _parse_bool(env_value) + except ValueError: + logger.warning( + "SCRAPEGRAPHAI_TELEMETRY_ENABLED is set to %r, which is not a " + "recognised boolean. Telemetry is left at %s. Use one of: " + "true/false, yes/no, on/off, 1/0.", + env_value, + telemetry_enabled, ) - except Exception: - pass return telemetry_enabled diff --git a/tests/test_telemetry_flag.py b/tests/test_telemetry_flag.py new file mode 100644 index 000000000..c3743c1f4 --- /dev/null +++ b/tests/test_telemetry_flag.py @@ -0,0 +1,70 @@ +"""Tests for the telemetry opt-out flag. + +These cover the environment variable path, which previously read its value from +the config file instead of from the variable, so `SCRAPEGRAPHAI_TELEMETRY_ENABLED=false` +left telemetry enabled. +""" + +import configparser + +import pytest + +from scrapegraphai.telemetry.telemetry import ( + _check_config_and_environ_for_telemetry_flag, + _parse_bool, +) + + +def _config(**defaults): + cfg = configparser.ConfigParser() + cfg["DEFAULT"] = {k: str(v) for k, v in defaults.items()} + return cfg + + +class TestParseBool: + @pytest.mark.parametrize("value", ["false", "False", "FALSE", "no", "off", "0", " false "]) + def test_falsey_spellings(self, value): + assert _parse_bool(value) is False + + @pytest.mark.parametrize("value", ["true", "True", "yes", "on", "1"]) + def test_truthy_spellings(self, value): + assert _parse_bool(value) is True + + def test_rejects_nonsense(self): + with pytest.raises(ValueError): + _parse_bool("maybe") + + +class TestTelemetryFlag: + def test_defaults_to_the_given_default(self): + assert _check_config_and_environ_for_telemetry_flag(True, _config()) is True + + def test_config_file_can_disable(self): + cfg = _config(telemetry_enabled="False") + assert _check_config_and_environ_for_telemetry_flag(True, cfg) is False + + def test_env_var_disables_with_no_config_key(self, monkeypatch): + """The regression. Previously returned True, because the value was read + out of the config file rather than out of the environment variable.""" + monkeypatch.setenv("SCRAPEGRAPHAI_TELEMETRY_ENABLED", "false") + assert _check_config_and_environ_for_telemetry_flag(True, _config()) is False + + def test_env_var_overrides_the_config_file(self, monkeypatch): + monkeypatch.setenv("SCRAPEGRAPHAI_TELEMETRY_ENABLED", "false") + cfg = _config(telemetry_enabled="True") + assert _check_config_and_environ_for_telemetry_flag(True, cfg) is False + + def test_env_var_can_also_enable(self, monkeypatch): + monkeypatch.setenv("SCRAPEGRAPHAI_TELEMETRY_ENABLED", "true") + cfg = _config(telemetry_enabled="False") + assert _check_config_and_environ_for_telemetry_flag(True, cfg) is True + + def test_unparseable_env_var_leaves_the_flag_alone(self, monkeypatch): + monkeypatch.setenv("SCRAPEGRAPHAI_TELEMETRY_ENABLED", "banana") + cfg = _config(telemetry_enabled="False") + assert _check_config_and_environ_for_telemetry_flag(True, cfg) is False + + def test_unset_env_var_leaves_the_config_in_charge(self, monkeypatch): + monkeypatch.delenv("SCRAPEGRAPHAI_TELEMETRY_ENABLED", raising=False) + cfg = _config(telemetry_enabled="False") + assert _check_config_and_environ_for_telemetry_flag(True, cfg) is False