Skip to content

Fail fast on invalid Jinja2Template instantiation parameters - #2568

Merged
Kludex merged 2 commits into
Kludex:masterfrom
pdelagrave:fix_templates_env_dir
Apr 20, 2024
Merged

Kludex merged 2 commits into
Kludex:masterfrom
pdelagrave:fix_templates_env_dir

Conversation

@pdelagrave

@pdelagrave pdelagrave commented Apr 12, 2024

Copy link
Copy Markdown

Summary

Calling Jinja2Template() with both directory and env shouldn't be allowed. When both parameters were used, the passed env was silently ignored in favor of creating a new one with the provided directory and the deprecated env_options.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

Calling `Jinja2Template()` with both `directory` and `env` shouldn't be allowed. When both parameters were used, the passed `env` was silently ignored in favor of creating a new one with the provided `directory` and the deprecated `env_options`.
@pdelagrave
pdelagrave force-pushed the fix_templates_env_dir branch from 402545d to 05c1d10 Compare April 12, 2024 13:26

@Kludex Kludex left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reflects the overrides. 馃憤

I think we should probably remove this assertion at some point... The type checker is able to catch this anyway.

@Kludex
Kludex enabled auto-merge (squash) April 20, 2024 07:53
@Kludex
Kludex disabled auto-merge April 20, 2024 07:53
@Kludex
Kludex enabled auto-merge (squash) April 20, 2024 07:53
@Kludex
Kludex merged commit 9cf26ee into Kludex:master Apr 20, 2024
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