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

bump jinja2 to v3 #634

Merged
merged 2 commits into from Feb 23, 2022
Merged

bump jinja2 to v3 #634

merged 2 commits into from Feb 23, 2022

Conversation

marxide
Copy link
Contributor

@marxide marxide commented Feb 23, 2022

Jinja2 depends on Markupsafe but the constraint is a minimum version with no upper bound. Markupsafe recently removed a deprecated function which causes the following error:

ImportError: cannot import name 'soft_unicode' from 'markupsafe'

The problem has been reported by a few Jinja2 users, e.g. pallets/jinja#1585. The Jinja2 dev team recommends upgrading Jinja2 - they refuse to patch the v2 releases with an upper bound to their Markupsafe constraint as it's considered "unsupported".

This PR bumps out Jinja2 requirement to Jinja2 = "^3.0.3". We only use Jinja2 for templating the run config files - I see no breaking changes and the tests passed locally.

v2 is unsupported and causes an error due to a transitive dependency issue
@marxide marxide self-assigned this Feb 23, 2022
@github-actions github-actions bot added this to In progress in Pipeline Backlog Feb 23, 2022
@marxide
Copy link
Contributor Author

marxide commented Feb 23, 2022

@ajstewart I think this is better than adding Markupsafe to our dependencies since we don't directly depend on Markupsafe, Jinja2 does.

If you agree, I suggest merging this into dev then rebasing #627 onto dev, and removing the Markupsafe pin in #627.

@marxide marxide added the dependencies Pull requests that update a dependency file label Feb 23, 2022
@ajstewart
Copy link
Contributor

@ajstewart I think this is better than adding Markupsafe to our dependencies since we don't directly depend on Markupsafe, Jinja2 does.

If you agree, I suggest merging this into dev then rebasing #627 onto dev, and removing the Markupsafe pin in #627.

Yep makes sense, I'm happy with that.

Copy link
Contributor

@ajstewart ajstewart left a comment

Choose a reason for hiding this comment

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

Might want to not squash this one so the commits are matched up in #632.

Pipeline Backlog automation moved this from In progress to Reviewer approved Feb 23, 2022
@marxide marxide merged commit 2198ac2 into dev Feb 23, 2022
@marxide marxide deleted the bump-jinja branch February 23, 2022 22:59
Pipeline Backlog automation moved this from Reviewer approved to Done Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants