Skip to content

[16.0][FIX] upgrade_analysis: Revert #3707 and do it on test-requirements.txt - #3708

Merged
OCA-git-bot merged 2 commits into
OCA:16.0from
Tecnativa:16.0-upgrade_analysis-deps
Aug 20, 2026
Merged

[16.0][FIX] upgrade_analysis: Revert #3707 and do it on test-requirements.txt#3708
OCA-git-bot merged 2 commits into
OCA:16.0from
Tecnativa:16.0-upgrade_analysis-deps

Conversation

@pedrobaeza

Copy link
Copy Markdown
Member

@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 20, 2026
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @StefanRijnhart, @legalsylvain,
some modules you are maintaining are being modified, check this out!

@pedrobaeza

Copy link
Copy Markdown
Member Author

@StefanRijnhart @sbidoul should I trigger the merge?

@StefanRijnhart StefanRijnhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I agree. I think the previous version would also fix the issue in some production deploys (depending on the method of installation) and in other OCA PRs on other repos that (theoretically) would pull in upgrade_analysis as a dependency.

@sbidoul

sbidoul commented Aug 20, 2026

Copy link
Copy Markdown
Member

You are right that the same constraint will need to be applied on other OCA repos that depend on upgrade_analysis.

But assume a security vulnerability appears in mako < 1.4. With this you prevent everyone from upgrading, for a bad reason. If you upgrade jinja2 too everything probably works, since the problem seems to come from the old version of jinja2?

It's complicated and I agree this solution is not ideal but its better than the direct upper bound, I'm quite sure of that.

There is a similar issue that pops up frequently with cryptography and openssl, and we apply a similar solution, because forcing everyone to use an old version of the cryptography library is obviously not a good thing to do.

@StefanRijnhart StefanRijnhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sbidoul Alright, thanks for confirming.

@StefanRijnhart

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-3708-by-StefanRijnhart-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 5ea08a6 into OCA:16.0 Aug 20, 2026
9 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 48086bf. Thanks a lot for contributing to OCA. ❤️

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.

4 participants