Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new M1 macOS GitHub runner to CI #4442

Merged
merged 40 commits into from
Feb 4, 2024
Merged

Conversation

RMeli
Copy link
Member

@RMeli RMeli commented Jan 30, 2024

Copy link

github-actions bot commented Jan 30, 2024

Linter Bot Results:

Hi @RMeli! Thanks for making this PR. We linted your code and found the following:

There are currently no issues detected! 🎉

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ba7682) 93.37% compared to head (9c23ba0) 93.37%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4442      +/-   ##
===========================================
- Coverage    93.37%   93.37%   -0.01%     
===========================================
  Files          171      185      +14     
  Lines        21737    22851    +1114     
  Branches      4012     4012              
===========================================
+ Hits         20298    21338    +1040     
- Misses         952     1025      +73     
- Partials       487      488       +1     

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

.github/workflows/gh-ci-cron.yaml Outdated Show resolved Hide resolved
@@ -269,7 +269,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu, macos, windows]
os: [ubuntu, macos, macos-14, windows]
python-version: ["3.9", "3.10", "3.11"]
wheels: ['true', 'false']
Copy link
Member

Choose a reason for hiding this comment

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

would be worth enabling cron CI - I can't remember if non-wheel build will work here, you might need to exclude macos-14 & wheels=false

Copy link
Member Author

Choose a reason for hiding this comment

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

@RMeli
Copy link
Member Author

RMeli commented Jan 31, 2024

macos-14 jobs are still queuing... Maybe it's work testing just 3.12 in the cron job too?

@IAlibay
Copy link
Member

IAlibay commented Jan 31, 2024

@RMeli I think there's a runner limit, I might be the one using that up on my other PR. I can definitely run 4 concurrent runners there.

@IAlibay
Copy link
Member

IAlibay commented Jan 31, 2024

Actually I didn't see, these have been queing for 20h?! That looks like a problem with github actions itself. Wouldn't surprise me given the beta nature of these runners. Maybe we're jumping the gun too early?

@RMeli RMeli closed this Jan 31, 2024
@RMeli RMeli reopened this Jan 31, 2024
.github/workflows/gh-ci-cron.yaml Outdated Show resolved Hide resolved
@RMeli
Copy link
Member Author

RMeli commented Feb 3, 2024

That looks like a problem with github actions itself. Wouldn't surprise me given the beta nature of these runners. Maybe we're jumping the gun too early?

Possibly a bit unstable, but I encountered not such problem in RMeli/spyrmsd#102. Maybe it's just that everyone rushed to test them as we did?

@RMeli
Copy link
Member Author

RMeli commented Feb 3, 2024

All green, at last.

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

One blocking question.

P.S. the runner failure you were saying is down to the runs-on being incorrect - if it isn't recognise gh actions doesn' t fail but just goes on indeterminate hold.

python-version: ["3.9", "3.10", "3.11", "3.12"]
wheels: ['true', 'false']
exclude:
- {os: "macos-14", python-version: "3.9"}
Copy link
Member

Choose a reason for hiding this comment

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

why is this being disabled? as show in #4445 we should be able to build on 3.9 with macos-14

Copy link
Member Author

Choose a reason for hiding this comment

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

Version 3.9 was not found in the local cache
Error: The version '3.9' with architecture 'arm64' was not found for macOS 14.2.1.
The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

I got this failure. Looking at the versions-manifest.json as suggested, it looks like all 3.9 is not provided with arch: arm64.

Copy link
Member

Choose a reason for hiding this comment

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

That's.. odd. I've not glanced upstream, but we should probably keep an eye on updates - it's very possible they couldn't backport stuff properly until they had their own CI runners for this.

@RMeli RMeli marked this pull request as ready for review February 3, 2024 21:16
Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Couple of extra things. Also please do update the changelog with your handle.

.github/workflows/gh-ci-cron.yaml Outdated Show resolved Hide resolved
python-version: ["3.9", "3.10", "3.11", "3.12"]
wheels: ['true', 'false']
exclude:
- {os: "macos-14", python-version: "3.9"}
Copy link
Member

Choose a reason for hiding this comment

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

That's.. odd. I've not glanced upstream, but we should probably keep an eye on updates - it's very possible they couldn't backport stuff properly until they had their own CI runners for this.

.github/workflows/gh-ci.yaml Outdated Show resolved Hide resolved
@RMeli RMeli requested a review from IAlibay February 4, 2024 09:09
Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

cheers, lgtm!

@IAlibay IAlibay merged commit dc34a08 into MDAnalysis:develop Feb 4, 2024
21 of 24 checks passed
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.

2 participants