CLI vs env vars

[fredbi]

Tracing here a thread in PR, for follow-up:

while we're in initConfig, could the viper.AutomaticEnvironment call get moved next to where viper parses the config file if present and before values are read out of viper into the config = newConfig() object? there's something lurking about initConfig that bothers me. encoding that the generated config file must be at the same location as the config file read by initConfig is probably part of it, but i don't think that's the entire story.. .. that datamonFlags.context.Descriptor.Name is set either by env variable or flag could be part of it, although i suspect it's a combination of things.

@ransomw1c here is a more complete explanation about what is wrong with env-driven config here, why I did not change it at this time (because this requires some extra changes and some agreement) and a proposal to go. Please advise.

have considered the overall golang arg parse situation at more length over the weekend. as far as environment variables are concerned, being able to encode a direct env var to config correspondence rather than this string munging mapping (via prefix or anything else) would be more in accord with my prior expectations from, e.g., Click. a more drastic opinion is that all golang programs might as well be parameterized by either a simple ARGBEGIN/ARGEND or optarg sort of for-switch construction or a config file, either standard unix .conf style or .yaml, plus validation routines.

[fredbi]

    // 1. Defaults: none at the moment (defaults are set together with flags)

    // 2. Override via environment variables
    // NOTE:
    //   Since there is no viper env prefix set, we use raw viper key names from env vars, i.e. $CONTEXT will set
    //   the context config key, something that is hard-wired below as DATAMON_CONTEXT...
    //
    //   Setting a prefix like "DATAMON" would be impractical, since DATAMON_CONFIG (the location of the local config file)
    //   would conflict with the Config confif key (the bucket of the remote config).
    //
    // TODO: IMHO we should rename the 'config' key as 'remoteConfig', remove the DATAMON_CONTEXT code below and use
    // viper.SetEnvPrefix("DATAMON").
    //

[fredbi]

Agree that this is drifting away from the original intent of this PR. Let's have this one merged and move on forward.

My suggestion was merely about exploiting the capabilities of spf13/viper regarding env vars.

Just a word to say that viper is quite popular in goland, notwithstanding the qualities of your python references. My comment (now removed and traced as a new github issue) as intended to provide you with some understanding of the viper-way of doing it.

Not wanting to play the cargo cult developer here, not wanting either to waste to much time on this.

My point with this comment, was that (i) atm, it just doesn't work as a regular user would expect (principle of no surprise broken) and (ii) Since we've adopted the cobra/viper/pflags triptych, I'd favor ways in line with what the lib is suggesting us

[ransomw1c]

i'd like to make specific mention of the way the GOOGLE_APPLICATION_CREDENTIALS (or whatever it is) variable gets used: it's not just read by cobra. it's read by the google-cloud-go library. note in cmd/ where the golang binary sets the variable if its not set while the otherwise standard non-standard of spf13's cobra and viper are reading in and assembling the config from defaults.

this data path around the golang binary (insomuch as the OS globals are used rather, even, than a global in a golang package) is troubling enough in its own right that i don't think we can make any universal decisions about env vars without first figuring out how to pass the token usually obtained via gcloud+browser-based oauth dance (or stored in a Secret on k8s) to google-cloud-go without using an environment variable.

or

we could at least ensure that datamon only writes to GOOGLE_APPLICATION_CREDENTIALS, just choosing a different name for the READ op constituted by viper/cobra. at least then we have some tighter constraint on this register machine (to be greatly exaggerated) the current google-cloud-go usage is asking the operating system to function as.