BUG - GH action workflows fixes #1970
Conversation
trallard
added
kind: bug
| @@ -91,6 +91,7 @@ jobs: | |||
| name: coverage-data-${{ matrix.python-version }} | |||
| path: .coverage | |||
| if-no-files-found: ignore | |||
| include-hidden-files: true | |||
There was a problem hiding this comment.
actions/upload-artifact recently added this key which broke the upload for .coverage
There was a problem hiding this comment.
curious if they said why they default to false, thereby breaking backward compatibility?
Coverage report
This PR does not seem to contain any modification to coverable code. |
trallard
changed the title
| @@ -91,6 +91,7 @@ jobs: | |||
| name: coverage-data-${{ matrix.python-version }} | |||
| path: .coverage | |||
| if-no-files-found: ignore | |||
| include-hidden-files: true | |||
There was a problem hiding this comment.
curious if they said why they default to false, thereby breaking backward compatibility?
| # needed for the coverage action | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write |
There was a problem hiding this comment.
I'm slightly confused why this is needed even though the coverage job has a if: github.event.workflow_run.event != 'release' clause. But as you say, permissions are weird. I think you were saying it's because coverage runs anyway (for some reason) and pushes the coverage data to a new branch? Would be nice to just make it so that coverage doesn't try to run at all on release workflows... but I'm assuming that since you're not doing that here, it's probably not that simple. 🤷🏻
There was a problem hiding this comment.
It all seems to come from the fact that we have the coverage step within our main CI workflow, which is in turn called by the release workflow.
So even if the coverage comment part is not triggered per se, GH actions checks that the permissions granted within the caller workflow (release) and the called one (CI) match.
The one way I could think of decoupling these workflows would be to split the tests and docs building jobs into a reusable action and call that within the CI and release workflows independently but that seemed like too much hassle.
I can do that if you see it as worthwhile.
There was a problem hiding this comment.
ah ok, now I get it. thanks for explaining. I agree that splitting into reusable actions seems not worth the hassle... I recall last time I tried to do that it was a pain and I gave up :)
|
@drammock the changes in the action are from a security POV to prevent credential/secret files to be accidentally uploaded. |
There are a couple of fixes in this PR:
upload-artifactaction which is making our workflows fail now