629 speedup GitHub actions

[stan-dot]

Fixes #629

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.62%. Comparing base (cd91f0f) to head (26be98c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #650   +/-   ##
=======================================
  Coverage   92.62%   92.62%           
=======================================
  Files          35       35           
  Lines        1654     1654           
=======================================
  Hits         1532     1532           
  Misses        122      122           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stan-dot]

@gilesknap how can I actually measure if this was successful in speeding up? I'd like to compare against a recent successful merge like this one
#640

[stan-dot]

maybe I do need to add 'cache-from to all the workflows?

[stan-dot]

it applies only when we run a new branch but the cache hits - so this PR's pipeline time is normal as it still had to create the cache.

has this kind of caching been considered for hyperion, @DominicOram ? or have there been any counterindications?

[DominicOram]

has this kind of caching been considered for hyperion, @DominicOram ? or have there been any counterindications?

Our actions don't take too long and developers generally run the test suite locally before pushing to catch errors anyway so it's rare the time it takes for actions to run slows us down at all.

[DominicOram]

I would also worry about caching too much and missing things, it's quite valuable to know that a PR works from a clean slate.

[gilesknap]

I would also worry about caching too much and missing things, it's quite valuable to know that a PR works from a clean slate.

Note that with respect to container build cache it only hits on layers that have not changed - However, I suppose that if a layer is fetching a remote resource and not locking the version of that resource then the cache could mask issues.

[stan-dot]

@callumforrester based on all the discussion above, is the functionality from this PR desirable and pursuing more caching is not needed at the moment?

[stan-dot]

still I think we could lint - test - build docs - dist all in one job, using one cache. always clearing the cache on the first job, if possible

[Relm-Arrowny]

Just a note, the way caches is set up on this atm will always run pip install after caching the env which mean it will update before running test so it should not miss anything and it should also down grade correctly.
As for making it lint - test - build docs - dist all in one job, it is possible but you lose a bit of the speed as they are slightly different so they will have to install those, plus because of the racing condition of the single caches you lose a bit more time time as you only get to update cache the one finish first which is most likely be the dist or lint.

[callumforrester]

Agree with @Relm-Arrowny, additionally if we condensed all the CI into one job we would lose the nice display showing which parts have passed and which parts have failed, which I find useful when reviewing a PR.
@stan-dot I'm happy to examine your workflow with you and see if we can figure out why the CI is bottlenecking you, it isn't bottlenecking me.

[callumforrester]

@stan-dot I'm inclined to close this unless you seriously object

[stan-dot]

@callumforrester I don't mean 'all CI in one job', just to make the docs go off the dev tests, and there also the dist.

It is not bottlenecking me personally

[stan-dot]

@callumforrester I agree we don't need to deepen the caching level. is the caching in this PR not fine? some caching is a best practice afaik

[callumforrester]

It depends, I'm unclear on whether caching was not included in the copier template due to lack of time or for a specific reason. @coretl or @gilesknap can probably comment.

[gilesknap]

I believe we worked out that the costs were greater than the benefits for a small python container. I do use cache to great effect on generic IOCs. In fact reflecting on how fast container builds on GitHub were just yesterday, for iic-adandor3, maybe this is worth revisiting.

[gilesknap]

Oh just realized that this is not about container build. In case it was not obvious my comments were about containers. We did look at cache for copier template container build only and decided against.

[coretl]

I would prefer to investigate how long it would take to pixi install the deps rather than pip install. Wheels are just zip files after all, so if the installer (pixi rather than pip) is fast enough, then it should get close to the speed of restoring a cache without the complexity of cache invalidation (without a lockfile we intentionally pull the latest version of things).