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

Fix CI workflows #1364

Merged
merged 3 commits into from
Apr 30, 2024
Merged

Fix CI workflows #1364

merged 3 commits into from
Apr 30, 2024

Conversation

barshaul
Copy link
Collaborator

@barshaul barshaul commented Apr 30, 2024

Based on #1358.
This PR:

  • Disables CD workflows on forks (checks fort he repo owner)
  • Differentiate between OS and RUNNER ('ubuntu' vs 'ubuntu-latest') argument, to allow fine-grained dependency installation later on
  • The github runner for macos-latest was updated, and now it requires some modifications in the CI to support the newer macOS version:
    1. The latest MacOS runs on ARM64 vs the former which runs on Intel
      2.The latest MacOS requires installation flag for python installation

Rebased over #1366

@barshaul barshaul requested a review from a team as a code owner April 30, 2024 06:10
@barshaul barshaul changed the title Fix CI Fix CI workflows Apr 30, 2024
@barshaul barshaul mentioned this pull request Apr 30, 2024
@barshaul barshaul force-pushed the fix_CI branch 8 times, most recently from 60e0971 to df3c233 Compare April 30, 2024 14:23
os: "macos-latest"
target: "x86_64-apple-darwin"
os: "macos"
target: "aarch64-apple-darwin"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we do it just for aarch and not both arch and x86?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question for ubuntu

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for mac

Re: ubuntu on arm CPU - we never tried it before. Worth adding in scope of another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@avifenesh out of scope of this PR - can be added in a different one

- ubuntu-latest
- macos-latest
runs-on: ${{ matrix.os }}
host:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like that
TIL

os: "macos-latest"
target: "x86_64-apple-darwin"
os: "macos"
target: "aarch64-apple-darwin"
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for mac

Re: ubuntu on arm CPU - we never tried it before. Worth adding in scope of another PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, rebase, #1366 already merged

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Re: ubuntu on arm CPU - we never tried it before. Worth adding in scope of another PR.

On our deployment workflows we build for arm64Xubuntu with the self-hosted runner.
We need to open a test to have some minimal integration tests on all platforms, but as I said above - out of scope for this PR

Copy link
Collaborator

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

Nicely done!

@Yury-Fridlyand Yury-Fridlyand merged commit 59e9e34 into main Apr 30, 2024
62 checks passed
@Yury-Fridlyand Yury-Fridlyand deleted the fix_CI branch April 30, 2024 17:47
tjzhang-BQ pushed a commit to Bit-Quill/valkey-glide that referenced this pull request May 8, 2024
* C#: Added a dotnet framework flag to the C# benchmark

* CI: Fix CI workflows to work with MacOS 13 (the version of the new macos-latest runner)

* Addressing comments
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
* C#: Added a dotnet framework flag to the C# benchmark

* CI: Fix CI workflows to work with MacOS 13 (the version of the new macos-latest runner)

* Addressing comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD CI/CD related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants