Add env_str resolver built-in resolver

[omry]

In some cases the automatic conversion of environment variable does not work well or is not what the user wants. A simple fix is to add another resolver named "env_str" that would return the env string as is.

[omry]

Blocked on #445. Depending on the final decision there this may not be needed.

[odelalleau]

Maybe we can discuss here wether or not we want env to parse and convert env variables?

My suggestion: keep them as strings, and add a new default resolver ("eval"? "parse"? "resolve"? "process"? "autoconvert"? ...) that does this step. This way, one could always use ${eval:${env:VAR}} if they want to convert the string.

[omry]

I like it.

With the possible support of consulting the annotations (#506), I think we will probably not need this in most cases anyway. The annotation is a more precise way to express the type you actually want as opposed to the provided type. (and automatically convert it in some cases, like from an int 4 to a string "4".

[odelalleau]

With the possible support of consulting the annotations (#506), I think we will probably not need this in most cases anyway. The annotation is a more precise way to express the type you actually want as opposed to the provided type. (and automatically convert it in some cases, like from an int 4 to a string "4".

Agreed. I'm going to adapt #445 accordingly, and add the new resolver in a different PR. Do you have a preferred name for that one?

[omry]

I was thinking to change the behavior to not parse in the existing env resolver. It's a breaking change but a small one. did you have anything else in mind?

Edit: you are talking about the parsing resolver. maybe parse or decode. (might have a stronger opinion might I see what it looks like).

Collisions are still unlikely but maybe we should use a common prefix for built in resolvers: oc.env, oc.decode etc (and deprecate the existing env one, telling people to use oc.env).

[odelalleau]

Collisions are still unlikely but maybe we should use a common prefix for built in resolvers: oc.env, oc.decode etc (and deprecate the existing env one, telling people to use oc.env).

Ok sounds good, will do that in another PR afterwards. . is not a valid character for resolver names right now though, do you prefer to add it or to use oc_env / oc_decode instead?

[omry]

I think it would be nice support some more formal separator for namespacing. I actually ran into this need recently too in Hydra, can't remember the details though.

[omry]

Since we are planning to break compatibility here, I think the best plan is to deprecate the current env resolver while keeping it as is in terms of behavior and switch people to use oc.env which can also have a new behavior.

[odelalleau]

Since we are planning to break compatibility here, I think the best plan is to deprecate the current env resolver while keeping it as is in terms of behavior and switch people to use oc.env which can also have a new behavior.

Makes sense. Should I do that in #445? (it would be more convenient if I want to close #230 with it)

[omry]

let's not add anything else to 445.

230 has nothing to do with it, it's more of a bugfix.

We can do it in a new PR closing this one (after editing this one to reflect what we are actually doing).