omegaconf icon indicating copy to clipboard operation
omegaconf copied to clipboard

Dereferencing a resolver that returns "???"

Open Jasha10 opened this issue 4 years ago • 7 comments

Describe the bug Normally, trying to access a missing literal "???" results in an error. When a custom resolver returns "???", the missing literal can be accessed without throwing an error.

To Reproduce

>>> from omegaconf import OmegaConf
>>> OmegaConf.register_new_resolver("baz", lambda: "???")
>>> cfg = OmegaConf.create({"bar": "${baz:}"})
>>> print(cfg)
{'bar': '${baz:}'}
>>> print(cfg.bar)
???

Expected behavior I would expect that accessing cfg.bar raises a MissingMandatoryValue or an InterpolationToMissingValueError instead of printing the string "???".

Additional context

  • OmegaConf version: on master branch
  • Python version: 3.9.5 I became aware of this while studying this issue: #759.

Jasha10 avatar Jun 28 '21 23:06 Jasha10

This is arguably an accidental feature. It enables the usage of the literal string ???. What is the use case for a resolver to return a MISSING value?

omry avatar Jun 29 '21 06:06 omry

The use case for me was to integrate env configuration so I created some yaml mapping like -

db:
  port: ${oc.env:DB_PORT, ???}
  host: ${oc.env:DB_HOST, ???}

I put missing as default so when I call merge if a certain env variable does not exist it will fall down to file configuration or default values.

This approach is not perfect but it works.

Ohad31415 avatar Jun 29 '21 06:06 Ohad31415

The behavior you are describing sounds like a bug: merge is not supposed to resolve interpolations (except interpolation to container nodes, that has a special treatment).

omry avatar Jun 30 '21 05:06 omry

@omry I need to use OmegaConf.resolve before merging, then each entry either evaluates to a value or to MISSING and then when I do the merge I get this priority of - take the env if existing, if not from file.

Ohad31415 avatar Jun 30 '21 06:06 Ohad31415

Can you provide a minimal snippet of what you are doing with an explanation of the desired behavior?

omry avatar Jun 30 '21 18:06 omry

Sure. I want to be able to read configuration from the following sources by priority from first to last -

  • defaults
  • file
  • env
  • cmd

For defaults, file and cmd I have the appropriate OmegaConf methods to load them and than merge them. There is no omegaconf function to load from env so I simply make a yaml mapping, something like this -

# env-mapping.yaml
db:
  user: ${oc.env:DB_USER, ???}
  password: ${oc.env:DB_PASSWORD, ???}

Then I can do

env_cfg = OmegaConf.load('env-mapping.yaml')
env_cfg = OmegaConf.resolve(env_cfg)

and add it to the merging phase. I need the ??? default so I don't end up having the resolve raising an exception.

I do think it would be nicer to have something like OmegaConf.from_env.

Ohad31415 avatar Jun 30 '21 18:06 Ohad31415

Just to further document the issue, here is the behavior before and after calling OmegaConf.resolve:

from omegaconf import OmegaConf, MissingMandatoryValue
from pytest import raises

cfg_defaults = OmegaConf.create({"foo": "default"})
cfg = OmegaConf.create({"foo": "${oc.env:VAR_NOT_SET, ???}"})

assert cfg == {"foo": "???"}
assert cfg.foo == "???"
assert not OmegaConf.is_missing(cfg, "foo")
assert OmegaConf.merge(cfg_defaults, cfg) == {"foo": "???"}

OmegaConf.resolve(cfg)

assert cfg == {"foo": "???"}
with raises(MissingMandatoryValue):
    cfg.foo
assert OmegaConf.is_missing(cfg, "foo")
assert OmegaConf.merge(cfg_defaults, cfg) == {"foo": "default"}

Jasha10 avatar Jul 06 '21 20:07 Jasha10