Skip to content

Bump jinja2 minimal version #3200

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

Merged
merged 1 commit into from
Jul 21, 2021
Merged

Bump jinja2 minimal version #3200

merged 1 commit into from
Jul 21, 2021

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Jul 21, 2021

This should ensure that features like is boolean are available in jinja2 used by molecule.

Related: ansible-community/molecule-vagrant#112

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
This should ensure that features like `is boolean` are available in
jinja2 used by molecule.

Related: ansible-community/molecule-vagrant#112
@ssbarnea ssbarnea requested a review from tadeboro as a code owner July 21, 2021 10:01
@ssbarnea ssbarnea requested a review from apatard July 21, 2021 10:11
@ssbarnea ssbarnea changed the title Bump jinja2 version required Bump jinja2 minimal version Jul 21, 2021
Copy link
Contributor

@apatard apatard left a comment

Choose a reason for hiding this comment

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

is boolean is a nice thing, so good to have it and we must avoid using jinja2 3.x for now imho

@ssbarnea
Copy link
Member Author

@apatard Why do you think we should avoid 3.x, in fact we use 3.x during testing as we did not cap its version. I did consider bumping the requirements to v3 but that version is quite new and I do not see any reason for doing that yet. Bumping should happen when we have reasons.

@apatard
Copy link
Contributor

apatard commented Jul 21, 2021

Nothing really serious. I'm only thinking of:

  • from semver point of view, it's a major update (2.X vs 3.X) so it theory, bad breakages may occur
  • some distro may not have 3.X. For instance next debian will have 2.11.3 iirc

@@ -73,7 +73,7 @@ install_requires =
cookiecutter >= 1.7.3 # dependency issues in older versions
dataclasses; python_version<"3.7"
enrich >= 1.2.5
Jinja2 >= 2.10.1
Jinja2 >= 2.11.3
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not cap the Jinja version here because Ansible users will expect things to work with 2.11 and 3.x. After all, Ansible works with those versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw, my remarks were about Jinja2 >= 2.11.3 vs Jinja2 >= 3.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, OK. I misunderstood. Whenever someone talks about dependencies and semver, the upper bound is what comes to my mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably an upper bound like <4.0 would make sense but on the other hand experience told me that upper constraints should be added only when you have knowledge of something already breaking you. Otherwise is like lottery, in half of cases you prevent a bug, in the other half you introduce a time-bomb conflict which is not real and you endup having to move the bound up and make a hotfix.

@ssbarnea ssbarnea merged commit d0cb7b7 into master Jul 21, 2021
@ssbarnea ssbarnea deleted the fix/jinja2 branch July 21, 2021 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants