Skip to content
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

add jinja2 templating support #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grase123
Copy link

@grase123 grase123 commented Feb 2, 2025

It would be nice that yaml and toml types of config files support Jinja templating as suggested in PR.

@silvanmelchior
Copy link
Collaborator

Hi there!

Thanks for your contribution!

It's an interesting idea. I would have solved this by providing a DataSource, which can then overwrite values of a FileSource. Not as flexible as full jinja templating, but I'm not so sure if we really need this?

If we would want to move forward with this, there would still be quite some work I think:

  • Jinja2 shouldn't be a fixed depenceny, but an optional one (a separate dependency group), and the package should also work w/o it as long as you don't use the templating feature. Meaning, parse_stream couldn't just use it, and the package cannot be imported per default
  • I'm not sure if it should be part of FileSource, maybe this should be a separate loader? Not sure about this one
  • There needs to be much more testing, also the case of not having jinja2 installed and the library still working
  • The documentation should be in the docs, as part of the other documentation, not a separate markdown at the repo root level
  • The lock-file was not updated properly / not committed

I'm not so sure about moving forward with this; maybe this use case is so "exotic" that it's better to write a custom data loader if someone really needs this. In what scenario was this helpful, and what would have been other options?

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

Successfully merging this pull request may close these issues.

2 participants