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

feat(ci): Python deps to install specific packages #28901

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Sep 28, 2021

This reduces the number of packages that get installed by brew and it prevents upgrading packages.

@armenzg armenzg self-assigned this Sep 28, 2021
@armenzg armenzg added the Component: CI Continuous Integration pipeline (GitHub Actions) label Sep 28, 2021
@armenzg armenzg marked this pull request as ready for review September 28, 2021 20:06
@armenzg armenzg requested a review from a team as a code owner September 28, 2021 20:06
@armenzg
Copy link
Member Author

armenzg commented Sep 28, 2021

@untitaker fyi

@@ -28,9 +28,11 @@ jobs:
- uses: actions/checkout@v2

- name: Install prerequisites
# brew can be finicky but it does not always means that the rest of the job will fail
env:
HOMEBREW_NO_AUTO_UPDATE: on
Copy link
Member

Choose a reason for hiding this comment

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

The python-deps workflow is not intended to test against user's dev env setup right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. We're using brew because it's available and Mac and Linux.

No auto update with just brew install is to make it execute faster.

If we had kept make prerequisites which calls brew bundle, it might have saved updating to some new library that breaks as we see on CI

From the man page:

HOMEBREW_NO_AUTO_UPDATE
              If set, do not automatically update before running some commands e.g. brew install, brew upgrade and brew tap.

Comment on lines -33 to +35
make prerequisites
brew install libxmlsec1
Copy link
Member

Choose a reason for hiding this comment

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

Why not run make prerequisites?

Copy link
Member Author

Choose a reason for hiding this comment

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

brew bundle includes a lot of packages that could suddenly be upgraded and start failing.
Reducing it to libxmlsec1 reduces it to only one package.

I want the python-deps to fail the least to avoid engineers be confused.
It is also almost what we used to do before make prerequisites 3f71db8#diff-9bec90e4ad9a179ec1c8e1266c2ddc2fc0fbd73db55720b3425cfdf667b89e14

@armenzg armenzg requested a review from billyvg September 29, 2021 13:50
@armenzg armenzg merged commit 3385770 into master Sep 29, 2021
@armenzg armenzg deleted the armenzg/ci/python-dep branch September 29, 2021 15:04
@armenzg
Copy link
Member Author

armenzg commented Sep 29, 2021

I marked as this being related to Apple M1 work because this is the fallout from work in #28607

vuluongj20 pushed a commit that referenced this pull request Sep 30, 2021
This reduces the number of packages that get installed by brew and it prevents upgrading packages.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: CI Continuous Integration pipeline (GitHub Actions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants