Repository navigation
TST: Remaining GitHub Actions actions #11038
Description
Description
Remaining things to port from .travis.yml:
-
Cachingwithccacheand.hypothesis. (.hypothesiscaching PR opened at TST: Cache .hypothesis #11161, @saimn is interested to look at the rest when he gets the chance). UPDATE: Haven't seen a real need for caching so far.- Formal documentation at https://docs.github.com/en/free-pro-team@latest/actions/guides/caching-dependencies-to-speed-up-workflows (there is a 5 GB hard limit)
- There is a blog about using ccache in Actions that maybe we can try.
Do we want to start caching pip dependencies too?See https://github.com/actions/cache/blob/main/examples.md#python---pip (The answer is "no" according to TST: Remaining GitHub Actions actions #11038 (comment) .)
-
pytestconditions in test modules usingTRAVISenv var. See TST: Fix hypothesis fuzzing logic on CI #11128- Example:
Line 29 in f9ba0f8
- Example:
-
Slack notification(both on success and failure) for cron job. (@saimn tried something but it didn't work, @Cadair has a cunning plan at TST: Remaining GitHub Actions actions #11038 (comment).) UPDATE: Nothing is happening on this front and no one has asked for it lately. - Cron:
- Big-endian s390x architecture with apt (
arch: s390x,ASTROPY_USE_SYSTEM_ALL=1) (@astrofrog opened Add aarch64, ppc64le, and s390x builds to GitHub actions #11045 ) - ARM64 architecture with apt (
arch: arm64,ASTROPY_USE_SYSTEM_ALL=1) (@astrofrog opened Add aarch64, ppc64le, and s390x builds to GitHub actions #11045 )
- Big-endian s390x architecture with apt (
- Finally, delete
.travis.yml. (@astrofrog opened Add aarch64, ppc64le, and s390x builds to GitHub actions #11045 )
From .circleci/config.yml:
- 32-bit and parallel (custom
astropy.cfg, Python 3.7-n=4 --durations=50, Python 3.8py38-test-double). See TST: Move 32-bit and parallel job to Actions #11043 image-tests-mpl302(@Cadair says no need to port)image-tests-mpl310(@Cadair says no need to port)image-tests-mpldev(@Cadair says no need to port)
Things already in Actions we need to fix:
- Allow remote data job to fail. See TST: Allow remote data job to fail, cron org #11036
- Separate cron jobs into daily and weekly. See TST: Allow remote data job to fail, cron org #11036
- See if we can add
--open-filesback to a newer job in future PR. See TST: Add back --open-files to one job #11037 -
codecovuploads report fine but not reporting back to PR. How to fix?Try Codecov comments not popping up on pull requests? codecov/codecov-action#84 and see?Looks like a a bug on their side that is now fixed. - Do we still need branch filters? I guess there is no need for now but we can revisit if we change our minds.
- Fix numpy-dev/remote data job failures.
See MNT: Ignore divide by zero warning in np.log #11042(Went away on its own when numpy-dev wheel updated.) - Fix cron job failures. (Might be the same problem as numpy-dev/remote data job above,
so should wait until MNT: Ignore divide by zero warning in np.log #11042 is merged firstyes it was.) - Do we need to roll our own
[ci skip]? Failed attempts at TST: Directives to skip Actions #11044 and TST: Allow skip CI for PRs #11129, maybe TST: Remaining GitHub Actions actions #11038 (comment) . See TST: Skip CI in Action #11168. And now GitHub supports it, so also see TST: Skip CI natively, DOC: Update license year #11316 - Find a way to auto-cancel Actions, see TST: Remaining GitHub Actions actions #11038 (comment) . See TST: Skip CI in Action #11168
Additional ideas:
- Can we tell
ci_workflows.ymlto run Actions on the PR fork instead of here, but report back the status to the PR anyway? The astropy version reported will be wrong due to lack of tags on the fork and if some jobs require special tokens, they will not work. (@saimn will investigate and he actually meant the opposite, see Restrict builds on push events to master #11054)
Additional context
I milestoned this issue because ideally we should sort everything out by 4.3 release.
See #10388 for more info (e.g., pros, cons, related links).
Reacted by Nick Murphy
Activity
Cadair commentedon Nov 13, 2020
I don't think the current implementation of artifacts on GH actions are up to replacing circle at this point.
astrofrog commentedon Nov 13, 2020
We could probably move the 32-bit job over though?
pllim commentedon Nov 13, 2020
Just to clarify, not everything in CircleCI has to port over. I am just putting them up there for discussion. "Won't fix" is an acceptable answer if current solution is sustainable and is working.
saimn commentedon Nov 13, 2020
About the "cancel previous builds" feature, there are some actions to do that:
pllim commentedon Nov 14, 2020
Re: #11038 (comment)
Thanks, @saimn ! Personally, I still find it ridiculous that Actions does not support that natively and we have to rely on third-party apps.
pllim commentedon Nov 14, 2020
@Zac-HD , any advise on how to cache
.hypothesisproperly on GitHub Actions? Thanks!astrofrog commentedon Nov 14, 2020
Can we use https://github.com/actions/cache for .hypothesis?
Zac-HD commentedon Nov 14, 2020
Yep, that's how I'd do it!
The simple option: just cache
.hypothesis. That's it; it'll work just fine - in fact Hypothesis works fine without any cache at all.More complex: you could also use the
restore-keysoption to ensure that the cache was specific to the current branch and/or Python version and/or Hypothesis version. This could in principle help because it avoids Hypothesis evicting failing examples which are specific to a different job, but I'm not sure it's worth the extra complexity.embray commentedon Nov 15, 2020
Something I discovered since the switchover which is frustrating (if I understand correctly) it seems that each "check" performed by tox is performed in a single job. So if one check fails (e.g. remote data) I can't re-run that one check since it's not a separate job.
Update: @pllim I see you already addressed this by adding a separate allowed_fail job containing, for now, the remote data tests. That might be good enough for most cases.
I wonder if GH Actions allows reuse though, as that would seem to make it easier to create separate jobs with less duplication. GitLab CI's file format is very nice in this regard; I have to study how reuse is enabled in GH Actions, if at all...
embray commentedon Nov 15, 2020
OK, apparently YAML anchors are explicitly not supported by Actions, and this is still a requested feature. I wonder why not. It's a basic feature of YAML that YAML parsers for all major languages support. What a let-down. Use of anchors can be really nice in GitLab CI configs.
astrofrog commentedon Nov 15, 2020
One can create composite actions (basically a set of steps to re-use) but frustratingly for now composite actions can't use actions, they can only include run steps (so no e.g. set up Python). Once that is fixed, we could use composite actions to reduce duplication.
embray commentedon Nov 15, 2020
All the more reason it would be nice to have a simple feature of YAML that would solve the problem without any convolutions like "composite actions" 😅
mhvk commentedon Nov 16, 2020
Thanks all for the amazing work! My really wished for missing feature is
[skip ci]...pllim commentedon Nov 19, 2020
pllim commentedon Dec 3, 2020
Back to #11038 (comment) on using third-party apps to auto-cancel duplicate jobs (and in general), I am still hesitant because there is no way to vet the quality of the Actions code or whether it is malicious or not. Does benefit outweigh the risk? Still not supported natively... https://github.community/t/github-actions-cancel-redundant-builds-not-solved/16025/34
Also if we are open to third-party actions, for the
[ci skip]feature, maybe can try https://github.com/marketplace/actions/ci-skip-action if we get desperate enough. I haven't tested it out yet.pllim commentedon Dec 4, 2020
Re: skipping CI (not to be confused with cancelling duplicate runs) -- I implemented action-skip-ci with proof-of-concept at kali-maa#41 . Not pretty but it works. Maybe easier to customize than some third-party app written in JS?
saimn commentedon Dec 11, 2020
I think it's fine to use a third-party app for a feature like skip-ci. For example https://github.com/styfle/cancel-workflow-action seems widely used and has ~80 lines of code, I don't think there is a huge risk.
pllim commentedon Dec 11, 2020
On the topic of Slack notification, GitHub says they don't have plan to add official support in the near future (see integrations/slack#940). @Cadair suggested pushing to Matrix and have the bridge push to Slack. He has the Matrix portion working already for Sunpy, as follows, and willing to help us set up a bot account for this purpose:
https://github.com/sunpy/sunpy/blob/master/tools/send_matrix_message.xsh
https://github.com/sunpy/sunpy/blob/94b31a412fe9486db1a448e4e2e99a77d57ec2ae/azure-pipelines.yml#L162
pllim commentedon Jan 19, 2021
Update: Just caching and Slack notifications left.
jdavies-st commentedon Feb 26, 2021
I will comment here that caching
pipdependencies can mask things like build issues caused by newly-released build dependencies. We saw this over atjwstwhere we were caching the pip cache in our Github action workflow, and it caused us to not catch when Numpy released 1.20, and caused users of 1.19x to have errors about API incompatibility between the built and runtime versions of Numpy. Pip just uses the latest version of a PEP518 dependency likenumpyor the latest cached version if it is not specifically limited in pyproject.toml. See discussion herespacetelescope/jwst#5744 (comment)
We did not catch this in our CI because we were caching wheels in the pip cache. It showed up when run with
pip install --no-cache-dirand also would have if we had turned off pip caching in our actions.saimn commentedon Feb 26, 2021
I don't understand how it is a cache issue. Reading the issue you linked, it seems you were building wheels with the latest numpy (using
numpywithout pinning inpyproject.toml) which then indeed leads to wheels that are not compatible with older numpy.oldest-supported-numpyis the solution to this, or you could pin manually the numpy version used to build the wheels inpyproject.toml. Typically you want to build wheels with the oldest numpy supported for each Python version, whicholdest-supported-numpydoes.jdavies-st commentedon Feb 26, 2021
With the cache, for some reason,
pipbuilds thedrizzlewheels with an older, cached version ofnumpy. I'm not sure why.Agree that
oldest-supported-numpyis the current solution given the lack of tooling to deal with this inpip.embray commentedon Mar 3, 2021
For numpy in particular shouldn't there be a test job for testing against "latest numpy". I agree with @saimn though that this is not an issue with caching.
pllim commentedon Mar 8, 2021
Re:
jwst-- Out of topic now, but testing a pipeline against latest numpy is a little excessive, no?pllim commentedon Apr 19, 2021
We have decided not to do caching, as we haven't found a real need for it and it introduces extra complexity.
As for Slack notification, still haven't seen a reliable solution that works. If we really want that, please open a new issue.
I think as it is, Actions works pretty well and satisfied our current CI needs.