Skip to content

[CI] Use concurrency to cancel-in-progress workflows on new commit#427

Merged
joshmoore merged 1 commit intozarr-developers:mainfrom
psobolewskiPhD:CI/use_concurrency
Jul 19, 2023
Merged

[CI] Use concurrency to cancel-in-progress workflows on new commit#427
joshmoore merged 1 commit intozarr-developers:mainfrom
psobolewskiPhD:CI/use_concurrency

Conversation

@psobolewskiPhD
Copy link
Contributor

This PR adds concurrency and cancel-in-progress to the CI workflows. See:
https://docs.github.com/en/actions/using-jobs/using-concurrency
What this means is if workflows are running when a new commit is pushed, they are canceled and restarted, rather than piling up.

I tested this on my fork, see: https://github.com/psobolewskiPhD/numcodecs/actions/runs/4600373158

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

@psobolewskiPhD
Copy link
Contributor Author

BTW, looking at all the actions, it occurs to me that every commit gets tested twice?
Once in the OS_CI action, e.g. ci-osx:

- name: Run tests
shell: "bash -l {0}"
run: |
conda activate env
pytest -v

But also when the wheel is built:
CIBW_TEST_COMMAND: pytest --pyargs numcodecs

Maybe the wheels action should use a simpler test command, like:
python -c 'import numcodecs'

@joshmoore
Copy link
Member

Thanks again, @psobolewskiPhD! (Workflows launched)

Maybe the wheels action should use a simpler test command, like:

MSTM.

@psobolewskiPhD
Copy link
Contributor Author

Not familiar with MSTM 😬
Any help?

@joshmoore
Copy link
Member

Sorry, "makes sense to me" :)

@psobolewskiPhD
Copy link
Contributor Author

Sorry, "makes sense to me" :)

thanks 😅
Do you think it makes sense to:

  1. add to this PR—it's tangentially relevant?
  2. Make a new PR for that?
  3. Add it to the PR with arm64 wheels, so we see the times better?

@psobolewskiPhD
Copy link
Contributor Author

Gentle bump—I should have some time to return to this topic.

@joshmoore
Copy link
Member

Hey, @psobolewskiPhD. Thanks for the bump. Very appreciated.

Do you think it makes sense to:

  • add to this PR—it's tangentially relevant?
  • Make a new PR for that?
  • Add it to the PR with arm64 wheels, so we see the times better?

Let's get this PR in. If you think we can fast-lane a new PR, let's do that. Otherwise, use the arm64 PR.

If you would like to see this PR listed in the release notes, please feel free to add it in one of your (many) other PRs. 😄

@joshmoore joshmoore merged commit 948da6d into zarr-developers:main Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants