Convert reusable-nox workflowk to using nox GHA#3194
Convert reusable-nox workflowk to using nox GHA#3194x1101 wants to merge 7 commits intoansible:develfrom
Conversation
.github/workflows/reusable-nox.yml
Outdated
| description: Space-seperated list of sessions | ||
| type: string | ||
| required: false | ||
| default: "static formatters_check typing spelling checkers(rstcheck) checkers(rst-yamllint) checkers(docs-build) actionlint" |
There was a problem hiding this comment.
Could we just have "checkers" and run all three of those checks? I'm not sure it's necessary to specify which ones we want because it's all of them.
There was a problem hiding this comment.
Given the other conversations here, I think the desire is to have each of the checkers be their own session.
But, if that's not the case, I don't object to swapping it around
x1101
changed the title
- Per discussions, added the sessions matrix back in, instead of a single string list of sessions to support cleaner output of statuses
- Replaced (temp) x1101/github-action-run-nox with ansible-community/github-action-run-nox - Repalced "matrix.sessions" with "matrix.session" - All sessions now in quotes
|
I did a few fixups, and now it looks like its down to failing the |
- Replaced `inputs.` with `matrix.` This resovles the `actionlint` errors
|
I think I found it. I was still using At this point, I think this is ready to remove the "Draft" tag and be considered for full PR, unless there's additional feedback from @webknjaz or @oraNod on its current state. |
| @@ -11,17 +11,17 @@ jobs: | |||
| fail-fast: false | |||
| matrix: | |||
There was a problem hiding this comment.
What I meant originally is that this matrix should be in the calling workflow, not the reusable one. That would be ci.yaml.
There was a problem hiding this comment.
I see what you mean. But I think that's a larger change to the workflow. As I understand it, the goal here is to take the existing workflow, which calls nox directly from a matrix and replace it with one that uses the GHA.
My preference would be to get the in-place change made and functional first before looking at reorganizing things.
That being said, if there's a strong desire to wrap that effort into this one, we can better outline that in the initial issue and I can look into tacking that too.
There was a problem hiding this comment.
The problem is that integrating this action the way it is now kinda makes it impossible to have this nice separation of steps. So I see it as contributing to poor inspectability of the log.
There was a problem hiding this comment.
It's probably a good idea to handle that first.
There was a problem hiding this comment.
The problem is that integrating this action the way it is now kinda makes it impossible to have this nice separation of steps ...
I agree here. As written, github-action-run-nox doesn't work like that. That means we either decide not to use it, contribute upstream changes to enable that work (if the folks that maintain in accept them), or spin out a new GHA for this.
| - name: "Run nox session" | ||
| uses: ansible-community/github-action-run-nox@v1 | ||
| with: | ||
| sessions: "${{ matrix.session }}" | ||
| extra-args: "${{ matrix.extra-args || '' }}" | ||
| force-pythons: "${{ matrix.python-versions }}" |
There was a problem hiding this comment.
Here's examples of splitting provisioning the venv and running the logic:
- https://github.com/pyca/cryptography/blob/09fb854e009d560e5edbe6677ce53f2b8c0b6011/.github/workflows/linkcheck.yml#L39-L45
- https://github.com/ansible/receptor/blob/59e0da47a39518b9a2742c7efae7bb7985a4d9a7/.github/workflows/coverage_reporting.yml#L52-L58
TL;DR for two steps to show up separately in CI, they should be actual separate entries in the step list (for the same session). The first one would run with nox --install-only, and it'd be followed by nox --no-install.
There was a problem hiding this comment.
I'll have to dig into this. This might be the way to use the GHA as written and still achieve the two-step process you're looking for. What I'm not clear on (and I'll be looking into this) is if it will correctly re-use the sessions as expected.
that commits have been accepted
3 participants
Marked as a DRAFT because it depends on a local fork of mine making changes to the upstream GHA. I have a PR in for that change. Once that's accepted, or the feature added in other ways, we can more accurately review this.
Addressed the request in #2511.
I am a little unsure of the approach I took on this, as the GHA looks for a single list of sessions to run. I converted the matrix over to that, but I'm unclear on what (if any) visibility into the details we'll lose that way. The other potential loss is if we intend to run different sessions against different python version in the same run (as opposed to running the whole action against a matrix of python versions).
If we determine that we're losing too many details or that we do need the ability to target multiple distinct pythons for different parts of the run, I can easily swap it back to the matrix setup, but still using this GHA.
Thoughts/questions/comments greatly appreciated.