vert-x3 / vertx-config

Vert.x Configuration Service
Apache License 2.0
54 stars 64 forks source link

Preventing 'EnvVariablesConfigStore' from attempting to convert values. #16

Closed bmcustodio closed 7 years ago

bmcustodio commented 7 years ago

The EnvVariablesConfigStore attempts a number of conversions on the values of envvar values before making them available to the config.getXXX methods. For instance, if I define

CLIENT_ID="162738491282"

as one of my environment variables it will end up in config() as a Double instead of a String. This happens because EnvVariablesConfigStore uses JsonObjectHelper#put on the set of envvars. However, sometimes all you need are the "raw" string values. Also, if you are later querying these with methods like config().getDouble() and config().getString(), I see little to no point in this behaviour—config().getString("CLIENT_ID") will throw in this use case.

Could we get one of the following?

  1. Make EnvVariablesConfigStore not use JsonObjectHelper#put and get raw values in config().
  2. Introduce a RawEnvVariablesConfigStore in order not to break any existing code out there and still get raw values in config()?

In any of the cases, would you consider a PR?

cescoffier commented 7 years ago

I may be wrong, but can't you get the number from JSON as a String:

Double.toString(config.getDouble("key", 2.0));
cescoffier commented 7 years ago

That's said, we can add an option to the Env and Sys config stores to not parse the content (read all entries as String). Something like `'json-conversion': false'

bmcustodio commented 7 years ago

@cescoffier that wouldn't work—you may end up with something like "4.31543975932E11". Even if it wasn't the case, that would only work as long as the value stored in the envvar doesn't overflow (I'm thinking of big numerical strings).

cescoffier commented 7 years ago

@brunomcustodio if the string representation is not a valid Java double, it will be added as a String to the json object.

bmcustodio commented 7 years ago

@cescoffier yes, I know, but I was talking specifically about your snippet :-)

bmcustodio commented 7 years ago

IMHO we can continue discussing this on #17.