Skip to content

Bug: Configuration.from_yaml() cannot read properly template strings in a yaml file #857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ITHwang opened this issue Feb 20, 2025 · 5 comments

Comments

@ITHwang
Copy link

ITHwang commented Feb 20, 2025

Hi, I'm using Configuration.from_yaml() to read template strings in a yaml file like below:

template_string:  |-
  Hi! My name is ${name}.
  I'm ${age} years old.
  I'm living in ${country}.

And When I read the yaml file, the variables string in the yaml file disappear.

class Container(DeclarativeContainer):
    config = Configuration()
    config.from_yaml(yaml_dir / "introduce.yaml")
    ...

container = Container()
print(container.config.template_string())
# Hi! My name is .\nI'm years old.\nI'm living in .

But when I use pyyaml directly, it doesn't have any problem.

class Container(DeclarativeContainer):
    config = Configuration()
    config.from_pydantic(PydanticConfig())

    # config.from_yaml(yaml_dir / "introduce.yaml")
    introduce_yaml_path = yaml_dir / "introduce.yaml"
    with open(introduce_yaml_path, "r") as f:
        loaded = yaml.safe_load(f)
        config.from_dict(loaded)

print(container.config.template_string())
# Hi! My name is ${name}.\nI'm ${age} years old.\nI'm living in ${country}.

Could you guys clear up what happens in from_yaml()?
Thanks!

@ZipFile
Copy link
Contributor

ZipFile commented Feb 22, 2025

Funnily enough, this once was a feature #467. I think it is possible to make this behavior optional.

@ITHwang
Copy link
Author

ITHwang commented Feb 24, 2025

Apparently, this is because _resolve_config_env_markers() in from_yaml() considers variable placeholders(${...}) in yaml file as environment variable placeholders and tries to replace them with environment variable or None.

How about add a parameter like enable_env_markers=True to from_yaml() so that we make calling _resolve_config_env_markers optional?

@ZipFile
Copy link
Contributor

ZipFile commented Feb 24, 2025

I've repurposed existing arg to receive envs_required=None. PR with this change (#861) is already merged, will be available in the next release.

@ITHwang
Copy link
Author

ITHwang commented Feb 24, 2025

That's cool! Thanks for your consideration :)

@ITHwang ITHwang closed this as completed Feb 24, 2025
@ZipFile
Copy link
Contributor

ZipFile commented Feb 25, 2025

Fixed in v4.46.0 (#861).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants