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

Several minor improvements to tox config and dependency declarations #1070

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Jan 8, 2025

I initially wanted to work up a new requirements.txt locking strategy here, using uv instead of pip-compile, to refine the art we developed in globus-sdk.
That work did not succeed, but these preparatory changes are all minor improvements which make our tox.ini config much easier to read and maintain.

  • Move test dependencies to a dependency group
  • Move 'test-mindeps' to a dependency group
  • Improve the implementation of tox -localsdk
  • Improve declaration of coverage tox envs
  • Remove the publish-release tox env
  • Add globus_cli_rmtree via toxfile plugin

We had dependency data for tests in a `test` extra, which is not
intended to be part of our public package interface. Move it to a
dependency group and remove the `test` extra.

Update usage of `.[test]` in the `Makefile`, which appears to be
highly outdated but which could be in use for some developers.

The `test` group had an entry for `setuptools` which appears to be
unnecessary, so it is removed here.

The dependency update script (`make update-dependencies`) gets a small
update to find where `mypy` is currently pinned.
Of necessity, this change also updates the min bound for `requests`
(for simplicity, to the latest version). Moving these data into a
dependency group revealed that our bound for requests was not
effective because it was in conflict with our version of `responses`.
With `deps` installed before the package and test dependencies, we
were actually not using the old version of `requests` which was
declared.

This change fixes the issue for the most part. It's still notable that
package install and dependency group installation are separate steps
in tox and can conflict.
It does not appear to be necessary to pass `GLOBUS_SDK_PATH` to a
subprocess, which won't work under `tox_uv` anyway (because the
virutalenv won't have `pip`).
Most likely, this method of passing is something which didn't work
in tox v3 and which v4 fixed. (It seems unlikely that we wouldn't have
tried this config.) In any case, the new simpler config works fine.
- `testenv:clean` was still using `setup.py clean`! Remove.
- Make them all inherit from a common `coverage` env.
- Use the `coverage` dependency group to share with `test` deps
- Declare `depends` correctly (fixes `tox p` usage!)
We now use Trusted Publishers config, so this is no longer needed.
This imitates our prior art in globus-sdk to move
`allowlist_externals = rm` usage into a plugin which can take care of
this for us. It also sets up `toxfile.py` for us in case we want
to put any other customizations in there.
@sirosen sirosen added the no-news-is-good-news This change does not require a news file label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant