Skip to content

docs: AutomaticEnv probes every key, and loads nothing - #2167

Open
ump45nose wants to merge 1 commit into
spf13:masterfrom
ump45nose:automatic-env-godoc
Open

ump45nose wants to merge 1 commit into
spf13:masterfrom
ump45nose:automatic-env-godoc

Conversation

@ump45nose

Copy link
Copy Markdown

Overview:

Corrects the AutomaticEnv godoc comment (both the package-level and the *Viper method copy) so it describes what find() actually does.

What problem does it solve?:

The comment makes two claims that the code contradicts:

  1. "check if environment variables match any of the existing keys (config, default or flags)" — the probe is not limited to keys that already exist. find() builds the variable name from whatever key is being looked up, so any Get/GetString/IsSet call hits the environment:

    // Env override next
    if v.automaticEnvApplied {
        envKey := strings.Join(append(v.parents, lcaseKey), ".")
        // even if it hasn't been registered, if automaticEnv is used,
        // check any Get request
        if val, ok := v.getEnv(v.mergeWithEnvPrefix(envKey)); ok {

    (viper.go:1271-1278, and the inline comment says the opposite of the doc comment.)

  2. "they are loaded into Viper" — nothing is loaded. AutomaticEnv() only flips a flag (viper.go:1460-1462), and getEnv calls os.LookupEnv on each lookup (viper.go:416-424). v.env is written only by BindEnv (viper.go:1163-1165), which is why AllKeys() — and therefore AllSettings() and WriteConfig() — never sees a key that exists only in the environment (viper.go:1980-1997).

The new wording also states how the variable name is derived, since that is what a user actually needs: the key is upper-cased by mergeWithEnvPrefix, prefixed with the SetEnvPrefix value when set (viper.go:389-395), and passed through the SetEnvKeyReplacer value in getEnv when set.

No behaviour change, no API change — comment text only.

Special notes for a reviewer:

The comment described AutomaticEnv as matching env vars against existing keys and loading matches into Viper. find() does neither: it joins the requested key with the sub-name parents, upper-cases it through mergeWithEnvPrefix, applies the key replacer in getEnv and calls os.LookupEnv on every lookup (viper.go:1272-1282). Nothing is stored, so env-only keys stay out of AllKeys and AllSettings (viper.go:1970-1987).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant