Allow map[string]string to support ':' in its keys and values

[sethcleveland]

This is useful for urls for either keys or values. As of now, envconfig fails with -- invalid map item at https://github.com/kelseyhightower/envconfig/blob/master/envconfig.go#L274.

[sethcleveland]

In my experience, I've seen this handled with doubling the character or escaping it.

[teepark]

Yeah I'm on the fence about this.

On the one hand there's nowhere else that we support escaping of any kind (so you also can't put commas into maps or slices for instance) and that's by design: it's more simple to implement and less to explain. Also there is a big workaround in the custom decoder support -- you can type alias map[string]string and implement your own escape-aware deserialization.

On the other hand the limited character sets are a pretty significant caveat.

[sethcleveland]

Agreed. From a practical standpoint the env value gets hard to understand with the colons etc... I find using the env as the map gets more understandable.

ex.

MAP_FIELD_K1=V1
MAP_FIELD_K2=V2
MAP_FIELD_k3=V3

type Config struct {
 Map map[string]string `envconfig:"MAP_FIELD"`
}

Then envconfig could build this map -- 

map[string]string = {
  "K1": V1,
  "K2": V2,
  "k3": V3,
}

[teepark]

I agree that's a lot clearer, but I'd suggest going one small step further and nail down precisely the map keys you're expecting with a struct type. Then it's covered by the nested struct support we already have.

The implementation change to support arbitrary key/vals under a common prefix like this is a bigger deal than it might seem. Today envconfig never iterates over the whole environment, it just identifies the specific keys it's looking for and asks for them by name.

[sethcleveland]

I agree with calling for values by name. From a programmatic perspective, iterating through the environment wouldn't be ideal. For now, we've implemented the map configuration lookup using a keys env variable with a subsequent call to load each map value from the env.

My concerns more along the operational side of things -- adding/removing entries from the map. This now requires two spots to assert -- keys and value. In my opinion, this works fine for small maps. However, problematic for larger maps. And easy to miss when new engineers come on board.

From a pragmatic perspective, I doubt the environment's gonna be large. Probably only a few hundred entries, if that for a 12-factor app. As a user of this envconfig, I'd like to see more flexibility with loading maps. Such that, when defining the map config variable, the engineer can decide to use the current behavior or the possibly slower one that iterates through the environment for values.

[teepark]

Hrmm. Maybe an alternative approach would just be to serialize your map to JSON in a single env var, and then load it with a custom decoder. That solves your "two places to update" problem at least, while not really being any worse of an environment variable abuse.

But really if your app configuration calls for a key/value store with a dynamic set of keys (especially a large set of keys), you've probably long since crossed the complexity threshold of a config file being more appropriate.

I worry that providing convenient support for complex cases like dynamic maps will encourage use of the environment in places for which it's really poorly suited. If you ask me, envconfig is already pretty far down that road today.

[teepark]

Thanks @sethcleveland for the thoughts. In the interest of clarity and simplicity and because this paradigm can be achieved with a custom decoder implementation, I'm going to close this here.

If you publish a package with a map[string]string alias that implements Decode in this fashion, I'll gladly drop a link in envconfig's README.