openedx / edx-lint

Custom tooling for pylint and other repo management tools
Apache License 2.0
51 stars 26 forks source link

build: Remove tox constraint. #354

Closed feanil closed 10 months ago

feanil commented 1 year ago

The tox package is already 6 minor revisions ahead at 4.6.4. If there are still plugins that don't support 4.x.x, then they are likely stagnant and need to be removed or updated ourselves.

However, as long as we keep this constraint here, we can't easily find and fix those issues. In many of the cases, this constraint was added due to the incompatibility of tox-battery with tox 4.x.x. However, tox-battery has updated its install_requires to be explicit of this dependency.

https://github.com/signalpillar/tox-battery/blob/master/setup.py#L20

Another issue we're running into is that some of the dependencies of tox are starting to publish security vulnerabilities. It's lower risk since this is in dev and CI but leaving this as is will increase security noise making it harder to respnod to real signals.

Specifically, tox<4.0.0 depends on a version of py which has a security vulnerability. Dependabot is picking this up and making some noise in a lot of our repos.

Closes https://github.com/openedx/public-engineering/issues/203

Process Note

Before merging this, I'll be posting a detailed message on Discourse(cross posted to slack) to let people know it's coming, what to look for and how to fix it locally in their repos. That should allow people to fix forward locally without the need for the global constraint. This will also allow us to incrementally move forward and fix one or two repos at a time.

iamsobanjaved commented 1 year ago

We have an issue on our board for this and was pending due to the team focusing on the Django upgrade, but now we have prioritized this and will work on first upgrading tox in edx-platform and then remove this constraint. Also this constraint removal work is blocked this issue https://github.com/edx/edx-arch-experiments/issues/130

feanil commented 10 months ago

This got fixed in a different PR.