-
Notifications
You must be signed in to change notification settings - Fork 14
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
Use Nix and Nox to get fast local CI #23
Conversation
28e7c01
to
e9b662e
Compare
fabd12e
to
afc25df
Compare
afc25df
to
85bd4a0
Compare
I think this is ready to be considered for merging now. The commit message (and PR description) has been updated. I spent too much time getting Nox and Poetry to play nicely together, but I hope the end result here is worth it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really love this and appreciate the workaround to use poetry groups! It will improve greatly our development flow.
I left some questions and one change suggestion.
Apart from that, even with nix-shell, I have trouble running tests on Python<=3.8.
Bug report:
[nix-shell:~/Tweag/FawltyDeps]$ nox -s tests-3.7
nox > Running session tests-3.7
nox > Re-using existing virtual environment at .nox/tests-3-7.
nox > python -m pip install -r .nox/.cache/tests-3.7/reqs_from_poetry.txt
nox > Command python -m pip install -r .nox/.cache/tests-3.7/reqs_from_poetry.txt failed with exit code 1:
Traceback (most recent call last):
File "/usr/lib/python3.7/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/usr/lib/python3.7/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/__main__.py", line 29, in <module>
from pip._internal.cli.main import main as _main
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/cli/main.py", line 9, in <module>
from pip._internal.cli.autocompletion import autocomplete
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/cli/autocompletion.py", line 10, in <module>
from pip._internal.cli.main_parser import create_main_parser
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/cli/main_parser.py", line 9, in <module>
from pip._internal.build_env import get_runnable_pip
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/build_env.py", line 20, in <module>
from pip._internal.cli.spinners import open_spinner
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/cli/spinners.py", line 9, in <module>
from pip._internal.utils.logging import get_indentation
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/utils/logging.py", line 29, in <module>
from pip._internal.utils.misc import ensure_dir
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/utils/misc.py", line 42, in <module>
from pip._internal.locations import get_major_minor_version
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/locations/__init__.py", line 67, in <module>
from . import _distutils
File "/home/maria/Tweag/FawltyDeps/.nox/tests-3-7/lib/python3.7/site-packages/pip/_internal/locations/_distutils.py", line 20, in <module>
from distutils.cmd import Command as DistutilsCommand
ModuleNotFoundError: No module named 'distutils.cmd'
nox > Session tests-3.7 failed.
From @mknorps' bug report:
Hmm. Even though you're running inside a nix-shell, it seems that the Nox virtualenv (in I'm going to take a wild guess that you first ran If my guess is right, the workaround for you is to delete the I might want to reconsider enabling |
85bd4a0
to
9d4695e
Compare
d9d0792
to
406205a
Compare
I went ahead and moved most of the commits from #40 into this pull request. It makes this PR quite a bit bigger, but resolves several of the questions discussed above:
|
@jherland Thank you for looking into the bug report. Indeed when I removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works great. I have one more request for different Python versions in lint.
Use shell.nix to bring in all Python versions that we want to support. Also bring in Nox via its own dependency group. This, together with noxfile.py defines a set of new "actions" that we can run locally (and maybe also in CI in a future commit). For now we define three actions: - tests (run on all supported Python versions) - lint (run linters) - reformat (run formatting actions) Examples of how to run this: - nox -s tests # Run test suite on all supported Python versions - nox -s tests-3.7 # Run test suite on v3.7 only - nox -s reformat # Run formatting action (black + isort) - nox -s lint # Run linters (mypy, pylint, isort, black) - nox # Run all of the above Some complications worth mentioning: We have organized our dependencies using Poetry dependency groups (see [1] for more information on those), and we would therefore like to use those groups when telling Nox which dependencies are needed for each of the Nox actions described above. However, we cannot use `poetry install ...` to install these dependencies, because Poetry insists on installing into its own virtualenv - i.e. NOT the virtualenv that Nox creates for each session. We could have used nox-poetry (https://github.com/cjolowicz/nox-poetry), but unfortunately it does not support Poetry dependency groups, yet[2]. The workaround/solution is to export the required dependency groups from Poetry into a requirements.txt file that we can then pass on to Nox's session.install(). This is implemented by the install_groups() helper function in our noxfile.py. On my work laptop, the nox command (i.e. running all of the actions) completes in ~50s for the initial run, and ~17s on subsequent runs (when Nox can reuse its per-session virtualenvs). [1]: https://python-poetry.org/docs/master/managing-dependencies/#dependency-groups [2]: see cjolowicz/nox-poetry#895 or cjolowicz/nox-poetry#977 for more discussion.
Poetry dependency groups are missing a feature: you cannot specify that one dependency groups depends on (or includes) another dependency group. This would be useful e.g. when a module (that is not a hard runtime dependency) is needed in more than one scenario. For example, in our project pytest is needed both when running tests (for obvious reasons), but also when running mypy or pylint (as they need to be able to read type annotations when following import statements from our test code.) In these situations we have two alternatives: - Include each dependency in exactly one dependency group, and then combine dependency groups when installing dependencies for the various scenarios, e.g. a developer scenario needs to install the nox + test + dev groups, dev is insufficient on its own. - Include each dependency in as many dependency groups as needed, so that each group is self-sufficient. This makes setting up scenarios simpler and less error-prone, but increases duplication inside pyproject.toml, and developers must take extra care when updating dependencies in this file. Until now we have gone for the first approach. With this commit, we switch to the second approach.
It seems that both Mypy and Pylint are sensitive to which Python interpreter is active while they run, and will provide slightly different results on different versions. Therefore we want to run both on top of all our supported Python versions. Replace the 'linters' action (in GitHub CI, aka. 'lint' in Nox) with two new actions: - lint: Run Mypy and Pylint linters. This action is parametrized with our supported Python versions. - format: Run isort and black in check-only mode to verify correct formatting of our code. This only runs on Python v3.10. (The 'format' actions now resembles the existing 'reformat' Nox action that runs isort + black to _fix_ the formatting of the code.) Add 'lint' and 'format' as separate dependency groups as well, to complement the CI actions.
Instead, developers will have to use the -r/--reuse-existing-virtualenvs option (or even -R) to get faster execution when the Nox virtualenvs are unchanged. Before this, developers would run into trouble when they had made changes to their default python executables, but Nox had already created a virtualenv now pointing to the wrong Python interpreter. It's safer to have a slower/robust default, rather than forcing developers to find the -N/--no-reuse-existing-virtualenvs option (or alternatively deleting the .nox/ directory between runs).
406205a
to
6c2f30b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Use shell.nix to bring in all Python versions that we want to support.
Also bring in Nox via its own dependency group.
This, together with noxfile.py defines a set of new "actions" that we
can run locally (with corresponding actions running in Github CI).
We define several actions:
Examples of how to run this:
Some complications worth mentioning:
Each action has a corresponding Poetry dependency group defined, which
includes all of the dependencies needed to run that action.
These groups are also used when running the corresponding actions inside
GitHub CI (but, crucially, GitHub CI does not itself use Nox
yet, rather it duplicates
the commands run in each action.
On my work laptop, the nox command (i.e. running all of the actions)
completes in <90s for the initial run, and <10s on subsequent runs
(using
nox -R
to minimize virtualenv/installation churn).