fix: 🐛 read SCRAPEGRAPHAI_TELEMETRY_ENABLED from the environment, not the config file - #1141
Open
HKlabworks wants to merge 1 commit into
Open
Conversation
… the config file
`_check_config_and_environ_for_telemetry_flag` checked that the environment
variable existed and then read its value out of the config file:
if os.environ.get("SCRAPEGRAPHAI_TELEMETRY_ENABLED") is not None:
try:
telemetry_enabled = config_obj.getboolean("DEFAULT", "telemetry_enabled")
except Exception:
pass
With no `telemetry_enabled` key in `~/.scrapegraphai.conf`, `getboolean` raises,
the bare `except` swallows it, and the flag keeps its default of `True`. So the
opt-out documented in the README leaves telemetry on for anyone who has not also
written a config file.
Now parses the variable's own value, reusing `configparser`'s BOOLEAN_STATES so the
environment variable and the config file accept the same spellings (true/false,
yes/no, on/off, 1/0). An unparseable value logs a warning and leaves the flag alone
rather than failing silently.
Adds tests/test_telemetry_flag.py covering the config path, the environment path,
precedence between them, and an unparseable value.
Verified with SCRAPEGRAPHAI_TELEMETRY_ENABLED=false and no config key:
before: True after: False
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for the library. While reviewing the telemetry code before adopting it, I found that the documented opt-out does not take effect on its own.
The bug
_check_config_and_environ_for_telemetry_flagchecks that the environment variable exists, then reads its value out of the config file:If
~/.scrapegraphai.confhas notelemetry_enabledkey, which is the state after a fresh install,getbooleanraises, the bareexceptswallows it, andtelemetry_enabledkeeps its default ofTrue.So
SCRAPEGRAPHAI_TELEMETRY_ENABLED=false, which is what the README recommends, leaves telemetry enabled unless the user has also written a config file. The failure is silent: nothing logs, and the flag reads as if the setting were honoured.Reproduced on 2.2.2 with the variable set to
falseand no config key:The fix
Parse the variable's own value, reusing
configparser.ConfigParser.BOOLEAN_STATESso the environment variable and the config file accept the same spellings (true/false,yes/no,on/off,1/0). That keeps the two paths consistent rather than inventing a second set of rules.An unrecognised value now logs a warning and leaves the flag unchanged, instead of being swallowed.
Precedence is unchanged in spirit and now actually works: config file first, environment variable overrides it.
Tests
Adds
tests/test_telemetry_flag.py, 20 cases covering the config path, the environment path, precedence between them, accepted boolean spellings, and an unparseable value. The key one istest_env_var_disables_with_no_config_key, which is the regression.Note on scope
This only changes how the flag is read. It does not change the default, the payload, or the endpoint. Happy to adjust the approach if you would rather handle it differently.