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

Remove Uses of tox-battery #191

Closed
2 of 68 tasks
feanil opened this issue Apr 14, 2023 · 13 comments
Closed
2 of 68 tasks

Remove Uses of tox-battery #191

feanil opened this issue Apr 14, 2023 · 13 comments
Labels
help wanted Ready to be picked up by anyone in the community maintenance Routine upkeep necessary for the health of the platform

Comments

@feanil
Copy link

feanil commented Apr 14, 2023

Context

The tox-battery python package provides only one feature to the tox tool. It detects changes to the requirements files that are the deps of a tox environment and optionally re-builds the environments if there are changes to the requirements. The tox-battery package can't run with tox 4.0.0 because of incompatibilites. So make upgrade won't upgrade us to the latest version of tox in a bunch of repos. However tox>=4.0.0 has the requirements monitoring feature built-in and if we drop the tox-battery dependency, we'll get the latest tox with the right featureset.

References

Task

  • Update common constraints to not require tox<4.0.0
  • In any repo that depends on tox-battery, drop the tox-battery dependency and upgrade to tox>=4.0.0.

Task List

  • Add tox<4.0.0 as a constraint in every repo that uses tox and tox-battery #217
  • Remove tox<4.0.0 from the common constraints #203
  • Remove tox-battery and upgrade tox in api-doc-tools
  • Remove tox-battery and upgrade tox in auth-backends
  • Remove tox-battery and upgrade tox in bok-choy
  • Remove tox-battery and upgrade tox in code-annotations
  • Remove tox-battery and upgrade tox in codejail-includes
  • Remove tox-battery and upgrade tox in completion
  • Remove tox-battery and upgrade tox in devstack
  • Remove tox-battery and upgrade tox in django-config-models
  • Remove tox-battery and upgrade tox in django-lang-pref-middleware
  • Remove tox-battery and upgrade tox in django-pyfs
  • Remove tox-battery and upgrade tox in django-splash
  • Remove tox-battery and upgrade tox in django-user-tasks
  • Remove tox-battery and upgrade tox in ecommerce
  • Remove tox-battery and upgrade tox in ecommerce-worker
  • Remove tox-battery and upgrade tox in edx-ace
  • Remove tox-battery and upgrade tox in edx-analytics-dashboard
  • Remove tox-battery and upgrade tox in edx-analytics-data-api
  • Remove tox-battery and upgrade tox in edx-bulk-grades
  • Remove tox-battery and upgrade tox in edx-celeryutils
  • Remove tox-battery and upgrade tox in edx-cookiecutters
  • Remove tox-battery and upgrade tox in edx-developer-docs
  • Remove tox-battery and upgrade tox in edx-django-utils
  • Remove tox-battery and upgrade tox in edx-enterprise
  • Remove tox-battery and upgrade tox in edx-enterprise-data
  • Remove tox-battery and upgrade tox in edx-enterprise-subsidy
  • Remove tox-battery and upgrade tox in edx-lint
  • Remove tox-battery and upgrade tox in edx-milestones
  • Remove tox-battery and upgrade tox in edx-notes-api
  • Remove tox-battery and upgrade tox in edx-platform
  • Remove tox-battery and upgrade tox in edx-proctoring
  • Remove tox-battery and upgrade tox in edx-rbac
  • Remove tox-battery and upgrade tox in edx-repo-health
  • Remove tox-battery and upgrade tox in edx-rest-api-client
  • Remove tox-battery and upgrade tox in edx-sphinx-theme
  • Remove tox-battery and upgrade tox in edx-toggles
  • Remove tox-battery and upgrade tox in edx-tools
  • Remove tox-battery and upgrade tox in edx-val
  • Remove tox-battery and upgrade tox in edx-when
  • Remove tox-battery and upgrade tox in enterprise-catalog
  • Remove tox-battery and upgrade tox in enterprise-subsidy
  • Remove tox-battery and upgrade tox in event-bus-kafka
  • Remove tox-battery and upgrade tox in event-bus-redis
  • Remove tox-battery and upgrade tox in event-routing-backends
  • Remove tox-battery and upgrade tox in event-tracking
  • Remove tox-battery and upgrade tox in help-tokens
  • Remove tox-battery and upgrade tox in opaque-keys
  • Remove tox-battery and upgrade tox in openedx-atlas
  • Remove tox-battery and upgrade tox in openedx-events
  • Remove tox-battery and upgrade tox in openedx-filters
  • Remove tox-battery and upgrade tox in openedx-learning
  • Remove tox-battery and upgrade tox in openedx-ledger
  • Remove tox-battery and upgrade tox in pytest-repo-health
  • Remove tox-battery and upgrade tox in pytest-warnings-report
  • Remove tox-battery and upgrade tox in repo-tools-data-schema
  • Remove tox-battery and upgrade tox in super-csv
  • Remove tox-battery and upgrade tox in taxonomy-connector
  • Remove tox-battery and upgrade tox in testeng-ci
  • Remove tox-battery and upgrade tox in web-fragments
  • Remove tox-battery and upgrade tox in xapi-db-load
  • Remove tox-battery and upgrade tox in XBlock
  • Remove tox-battery and upgrade tox in xblock-drag-and-drop-v2
  • Remove tox-battery and upgrade tox in xblock-google-drive
  • Remove tox-battery and upgrade tox in xblock-sdk
  • Remove tox-battery and upgrade tox in xblock-utils
  • Remove tox-battery and upgrade tox in xqueue
  • Remove tox-battery and upgrade tox in xss-utils
@feanil feanil converted this from a draft issue Apr 14, 2023
@feanil feanil added help wanted Ready to be picked up by anyone in the community maintenance Routine upkeep necessary for the health of the platform labels Apr 14, 2023
@Agrendalath
Copy link
Member

@feanil, before removing tox-battery from any of these repositories, we should remove tox<4.0.0 from the common constraints. Otherwise, we won't be able to upgrade it to tox>=4.0.0.

@feanil
Copy link
Author

feanil commented Jun 22, 2023

Good point, and that should be safe to do since tox-battery now explicitly declares it's requirment on tox<4.0.0

I'll update the ticket description.

@bpostow
Copy link

bpostow commented Oct 12, 2023

I'm new here so I want to make sure I understand what I'm doing. tox-battery only appears in requirements files, right? so we just need to remove that line? or is there something more complicated that I'm missing?

@feanil
Copy link
Author

feanil commented Oct 12, 2023

@bpostow This upgrade is a little tricky because of how we're using common constraints. The best way to do this upgrade incrementally right now is to:

  1. Update any repo that uses tox to locally constrain tox<4.0.0
  2. Remove the global common constraint.
  3. Incrementally remove the local constraint, as well as the depedency on tox-battery if it exists in that repo, and then run make upgrade and fix any tox issues that arise from the upgrade. Some notes related to that here: https://tox.wiki/en/latest/upgrading.html

@Agrendalath
Copy link
Member

Agrendalath commented Oct 13, 2023

@feanil,

Update any repo that uses tox to locally constrain tox<4.0.0

This step should only be needed for repos that use tox without tox-battery. For them, we risk that running make upgrade will bump tox to 4.x and break some tests.
If a repo uses tox-battery, running make upgrade will not bump the tox version to 4.x because it is incompatible with tox-battery, as you already mentioned above.

cc: @bpostow

@bpostow
Copy link

bpostow commented Oct 13, 2023

My question is mostly, WHERE is tox-battery being used? I only see it in requirements. So, if we just remove the requirement for something that isn't actually being used, shouldn't that be safe?

or are there constraints somewhere else that I'm not looking?

If this is a more subtle thing that a newbie shouldn't be touching, I fully understand that and will look for other better intro tickets.

@Agrendalath
Copy link
Member

Agrendalath commented Oct 13, 2023

@bpostow, you don't need to explicitly use the tox-battery package - it plugs into the tox with this entrypoint. As mentioned in the description of this issue, this plugin is responsible for tracking the requirement changes.
If you simply remove it (without updating tox to 4.x), it will cause some confusion in the dev environments.
On the other hand, removing tox-battery and automatically upgrading tox in all repositories can break some tests (e.g., because of edx/edx-arch-experiments#130). Therefore, we should do this repo-by-repo.

or are there constraints somewhere else that I'm not looking?

Do you mean this constraint (described in #203)? The process described in this comment (with this clarification) sums up everything we should do to move this forward.
Edit: it looks like there is already a PR (openedx/edx-lint#354) for step 2 ("Remove the global common constraint.").

@bpostow
Copy link

bpostow commented Oct 13, 2023

ah, ok, I was unclear on how tox works, I thought that this was a code library that would be referenced in the code somewhere.

@Younggregs
Copy link

Hello @feanil are these issues still open? I'll like to take some

@feanil
Copy link
Author

feanil commented Dec 19, 2023

@Younggregs this was just completed by someone else. It looks like tox-battery has been removed everywhere.

@Younggregs
Copy link

Thank you @feanil for the feedback.
I'll really appreciate if you can point me to any available open issue please, I haven't been able to find one.

@feanil
Copy link
Author

feanil commented Dec 19, 2023

There are a lot in the "Help Wanted" section of public-engineering: https://github.com/openedx/public-engineering/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22

@Younggregs
Copy link

Thank you @feanil !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Ready to be picked up by anyone in the community maintenance Routine upkeep necessary for the health of the platform
Projects
Archived in project
Development

No branches or pull requests

4 participants