Skip to content

Commit

Permalink
oc.env converts none-string inputs to string instead of raising an ex…
Browse files Browse the repository at this point in the history
…ception
  • Loading branch information
omry committed Apr 2, 2021
1 parent 2c48e60 commit 02a0622
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 44 deletions.
18 changes: 10 additions & 8 deletions docs/notebook/Tutorial.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -774,8 +774,8 @@
"cell_type": "markdown",
"metadata": {},
"source": [
"You can specify a default value to use in case the environment variable is not defined.\n",
"This default value can be a string or ``null`` (representing Python ``None``). Passing a default with a different type will result in an error.\n",
"You can specify a default value to use in case the environment variable is not set.\n",
"In such a case, the default value is converted to a string using ``str(default)``, unless it is ``null`` (representing Python ``None``) - in which case ``None`` is returned. \n",
"\n",
"The following example falls back to default passwords when ``DB_PASSWORD`` is not defined:"
]
Expand All @@ -789,23 +789,25 @@
"name": "stdout",
"output_type": "stream",
"text": [
"'abc123'\n",
"'12345'\n"
"'password'\n",
"'12345'\n",
"None\n"
]
}
],
"source": [
"os.environ.pop('DB_PASSWORD', None) # ensure env variable does not exist\n",
"cfg = OmegaConf.create(\n",
" {\n",
" \"database\": {\n",
" \"password1\": \"${oc.env:DB_PASSWORD,abc123}\", # the string 'abc123'\n",
" \"password2\": \"${oc.env:DB_PASSWORD,'12345'}\", # the string '12345'\n",
" \"password1\": \"${oc.env:DB_PASSWORD,password}\",\n",
" \"password2\": \"${oc.env:DB_PASSWORD,12345}\",\n",
" \"password3\": \"${oc.env:DB_PASSWORD,null}\",\n",
" },\n",
" }\n",
")\n",
"print(repr(cfg.database.password1))\n",
"print(repr(cfg.database.password2))"
"print(repr(cfg.database.password2))\n",
"print(repr(cfg.database.password3))"
]
},
{
Expand Down
23 changes: 15 additions & 8 deletions docs/source/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -403,24 +403,31 @@ Input YAML file:
>>> conf.user.home
'/home/omry'

You can specify a default value to use in case the environment variable is not defined.
This default value can be a string or ``null`` (representing Python ``None``). Passing a default with a different type will result in an error.
You can specify a default value to use in case the environment variable is not set.
In such a case, the default value is converted to a string using ``str(default)``, unless it is ``null`` (representing Python ``None``) - in which case ``None`` is returned.

The following example falls back to default passwords when ``DB_PASSWORD`` is not defined:

.. doctest::

>>> cfg = OmegaConf.create(
... {
... "database": {
... "password1": "${oc.env:DB_PASSWORD,abc123}",
... "password2": "${oc.env:DB_PASSWORD,'12345'}",
... "password1": "${oc.env:DB_PASSWORD,password}",
... "password2": "${oc.env:DB_PASSWORD,12345}",
... "password3": "${oc.env:DB_PASSWORD,null}",
... },
... }
... )
>>> cfg.database.password1 # the string 'abc123'
'abc123'
>>> cfg.database.password2 # the string '12345'
'12345'
>>> # default is already a string
>>> show(cfg.database.password1)
type: str, value: 'password'
>>> # default is converted to a string automatically
>>> show(cfg.database.password2)
type: str, value: '12345'
>>> # unless it's None
>>> show(cfg.database.password3)
type: NoneType, value: None


Decoding strings with interpolations
Expand Down
23 changes: 10 additions & 13 deletions omegaconf/omegaconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def register_default_resolvers() -> None:
# DEPRECATED: remove in 2.2
def legacy_env(key: str, default: Optional[str] = None) -> Any:
warnings.warn(
"The `env` resolver is deprecated, see https://github.com/omry/omegaconf/issues/573",
"The `env` resolver is deprecated, see https://github.com/omry/omegaconf/issues/573"
)

try:
Expand All @@ -109,22 +109,19 @@ def legacy_env(key: str, default: Optional[str] = None) -> Any:
else:
raise ValidationError(f"Environment variable '{key}' not found")

def env(key: str, default: Optional[str] = _DEFAULT_MARKER_) -> Optional[str]:
if (
default is not _DEFAULT_MARKER_
and default is not None
and not isinstance(default, str)
):
raise TypeError(
f"The default value of the `oc.env` resolver must be a string or "
f"None, but `{default}` is of type {type(default).__name__}"
)

def env(key: str, default: Any = _DEFAULT_MARKER_) -> Optional[str]:
"""
:param key: Environment variable key
:param default: Optional default value to use in case the key environment variable is not set.
If default is not a string, it is converted with str(default).
None default is returned as is.
:return: The environment variable 'key', or a string representation of the default.
"""
try:
return os.environ[key]
except KeyError:
if default is not _DEFAULT_MARKER_:
return default
return str(default) if default is not None else None
else:
raise KeyError(f"Environment variable '{key}' not found")

Expand Down
22 changes: 7 additions & 15 deletions tests/interpolation/built_in_resolvers/test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,19 +162,11 @@ def test_env_default_none(monkeypatch: Any) -> None:
assert c.my_key is None


@mark.parametrize("has_var", [True, False])
def test_env_non_str_default(monkeypatch: Any, has_var: bool) -> None:
if has_var:
monkeypatch.setenv("MYKEY", "456")
else:
monkeypatch.delenv("MYKEY", raising=False)

def test_env_non_str_default(monkeypatch: Any) -> None:
c = OmegaConf.create({"my_key": "${oc.env:MYKEY, 123}"})
with raises(
InterpolationResolutionError,
match=re.escape(
"TypeError raised while resolving interpolation: The default value "
"of the `oc.env` resolver must be a string or None, but `123` is of type int"
),
):
c.my_key

monkeypatch.setenv("MYKEY", "456")
assert c.my_key == "456"

monkeypatch.delenv("MYKEY")
assert c.my_key == "123"

0 comments on commit 02a0622

Please sign in to comment.