Skip to content

[github] static and unit test checks as GHA - #15392

Merged
ehigham merged 3 commits into
hail-is:mainfrom
cjllanwarne:cjl_simple_gha_checks_try_2
Apr 15, 2026
Merged

ehigham merged 3 commits into
hail-is:mainfrom
cjllanwarne:cjl_simple_gha_checks_try_2

Conversation

@cjllanwarne

Copy link
Copy Markdown
Collaborator

Change Description

Resurrects the "tier 1" actions in #15276. Replacing all of CI with GHA is not trivial because of limits on free compute and concurrent job runs, but providing quick feedback to people without CI access via duplicative github checks is still extremely valuable.

We should watch:

  • That we remain way below the free usage limits in github
  • That the jobs run quickly

Security Assessment

  • This change cannot impact the Hail Batch instance as deployed by Broad Institute in GCP

Github actions only

@cjllanwarne
cjllanwarne requested a review from ehigham April 13, 2026 20:23

@ehigham ehigham left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about removin the corresponding checks in build.yaml?Is it worth running them twice?

@cjllanwarne

Copy link
Copy Markdown
Collaborator Author

@ehigham re "What do you think about removing the corresponding checks in build.yaml?Is it worth running them twice?". I tried this in my prior PR, but it turns out it's not trivial, eg:

  • Do we want to remove all trace of them from build.yaml or just the CI test runs? Some are upstream of other test steps, or deploy steps.
  • Some of these are extracted portions of longer steps (eg the batch unit tests are a portion of the larger batch build step, check-hail-fast is a faster subset of the build.yaml check-hail).
  • Is it safe (I don't think they auto-rerun when the main branch changes). Maybe not a big deal?
  • Are they used elsewhere? Is there a path that goes through build.yaml but not through github?

I guess I'd be happy to "just do it and reconsider if it becomes a problem" but OTOH, these are all short steps (even shorter in build.yaml where the pip install is shared), so "doing everything twice" is not as much overhead as it appears - at least for these "trivial setup" test cases.

So - to me the big win here is "getting information to contributors quickly" rather than "moving work out of CI", so I'd be inclined to leave it.

ehigham commented Apr 13, 2026

Copy link
Copy Markdown
Member

Fwiw I'm not particularly fussed about the runtime overhead. I am more bothered about the checks being called in different yaml files and maintaining that. The wording on my comment was pretty vague.

I think it should be safe to purge from build.yaml as we test by rebasing and the deploy by rebase and squash. It's hard to think of a scenario where that would cause problems for the check-* checks. The tests you can probably keep. I'm also ok with deferring this to a later change to have a discussion with the rest of the team.

@cjllanwarne
cjllanwarne requested a review from ehigham April 14, 2026 17:58

@ehigham ehigham left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed in team meeting, keep logic in both build.yaml and this

@cjllanwarne
cjllanwarne requested a review from chrisvittal April 15, 2026 01:08
@ehigham
ehigham merged commit e31d2f3 into hail-is:main Apr 15, 2026
13 checks passed
@cjllanwarne
cjllanwarne deleted the cjl_simple_gha_checks_try_2 branch April 15, 2026 14:15
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