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

Lookup github action values from nox #800

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chadrik
Copy link

@chadrik chadrik commented Mar 12, 2024

Hi, thanks for this awesome project!

This PR establishes a single source of truth for python versions and tests matrix, driven by nox.

I had this idea for my own projects as I've often been frustrated with duplicated data between pre-commit, nox, and gitlab. I wanted to implement this in this project for a few reasons:

  • If there were any missing features for nox, I was planning to add them as part of this PR. Turns out, nothing new is required, though there are some features that could simplify the workflow.
  • This project has some non-trivial Github Actions, and I wanted to add Github to my list of destinations supported by this idea
  • Serves as interesting reference for others. If you like it, I can add something to the nox docs about this.

Changes

These are the differences in behavior from before:

  • I wanted to eliminate the need for if: matrix.python-version != '3.7' within ci.yml because that is already in the noxfile.py as if (python, tox_version) != ("3.7", "latest"), and the goal of this PR is to create a single source of truth. In order to do this, I had to change the dimensions of the strategy:matrix from [os, python-version] to [os, python-version, tox-version]. This may actually improve run time as more jobs run in parallel
  • nox -s 'tests(tox_version=latest)' will only run test_tox_to_nox.py by default. This was done to make the local test and CI behave the same, which reduces developer confusion as well as special-case code in ci.yml. To override, just pass -- tests

Other thoughts

The most difficult part of this project was discovering the right incantations for jq to mutate the data from nox -l --json (though it is fairly readable once it's done). If this idea catches on, I foresee a need for a CLI tool (either part of nox or separate) which can reduce the complexity and boilerplate by preparing nox session data in ways that can be more easily consumed by Gitlab, Github, pre-commit, etc.

Establish a single source of truth for python versions and tests matrix, driven by nox.
Make the local test and CI behave the same: the default behavior for tests(tox_version="latest") is to only run test_tox_to_nox.
@cjolowicz
Copy link
Collaborator

Thank you for working on this! It's interesting to see how this feature works out for our CI.

In this particular case, I'm not sure how the benefit of single sourcing the build matrix stacks up against the costs in complexity and runtime overhead. I'm a bit worried about having all jobs block on that initial job computing the matrix, when they could otherwise start running in parallel right away.

@cjolowicz
Copy link
Collaborator

Would it make sense to pre-compute the build matrix in pre-commit?

@chadrik
Copy link
Author

chadrik commented Mar 15, 2024

I'm a bit worried about having all jobs block on that initial job computing the matrix, when they could otherwise start running in parallel right away.

I understand the concern, but to be fair the job completes in 7s, so it's not a major delay.

Would it make sense to pre-compute the build matrix in pre-commit?

The easiest thing to do would be to add a pre-commit task that writes nox -l --json to a file that gets committed in the repo. It could obviously be setup to only run if noxfile.py changes. Would you prefer that approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants