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

Allow Environment.from_string() to take a filename for the loaded template #2013

Conversation

bartfeenstra
Copy link
Contributor

I currently use Environment.from_string() to dynamically load template files, and I would like to be able to set the correct file name on them so the generated tracebacks are much more helpful in identifying the source file (rather than the tracebacks saying the file was <template>). The reason I am not using a Loader is twofold: none of these files ever need to be rendered twice, and Loader is blocking (i.e. not async).

Please look at the PR as an example of the approach itself. If you think this is something that would be approved, I'll finish up the tests and documentation.

To do

  • Add tests that demonstrate the correct behavior of the change. Tests
    should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.

@davidism
Copy link
Member

davidism commented Aug 6, 2024

As explained in #1316, we don't plan to add this.

@davidism davidism closed this Aug 6, 2024
@bartfeenstra
Copy link
Contributor Author

Thanks for clarifying this. I do think this is a needless restriction, especially considering #1304 has not been worked on (I get it, we're all busy, this is not a criticism). Switching to loaders, while possible, would add some unneeded blocking file I/O to my application, just to get a better hint in the traceback.

@davidism
Copy link
Member

davidism commented Aug 6, 2024

There's another option offered in the issue I linked, which does allow setting the filename.

I don't know your exact circumstance, but I wouldn't expect loading the template to be a significant length of blocked time. You could also use a thread executor if it was an issue: https://docs.python.org/3/library/asyncio-task.html#running-in-threads

has not been worked on (I get it, we're all busy, this is not a criticism)

Thanks for understanding. I'm always happy to review a good design or PR! I don't personally use asyncio, so waiting for me to do something about it is going to take a lot longer, compared to someone who needs and understands asyncio contributing to the project.

@bartfeenstra
Copy link
Contributor Author

I admit I completely missed that one suggestion earlier, but I'm happy to report that I successfully implemented it in bartfeenstra/betty#1861.

Thanks for your time and work on Jinja! 😊

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants