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): Remove automation to initialize Docker on Mac #28352

Merged
merged 10 commits into from
Sep 8, 2021

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Sep 1, 2021

Docker has been doing all that is possible to prevent installing Docker Desktop
without user interaction, thus, we can't run sentry devservices up.
See docker/for-mac#2359 (comment) for details.

This PR removes all code that at times had managed to automate the process on Mac.

We will rely on Ubuntu to test the make bootstrap path.

We also add caching for Brew.

Docker has been doing all that is possible to prevent installing Docker Desktop
without user interaction, thus, we can't run `sentry devservices up`.
See docker/for-mac#2359 (comment) for details.

This PR removes all code that at times had managed to automate the process on Mac.

We will rely on Ubuntu to test the `make bootstrap` path.

We also add caching for Brew.
@armenzg armenzg added the Component: CI Continuous Integration pipeline (GitHub Actions) label Sep 1, 2021
@armenzg armenzg self-assigned this Sep 1, 2021
@armenzg
Copy link
Member Author

armenzg commented Sep 1, 2021

TODO:

  • Update bootstrap code
  • Update dev documentation

@@ -3,6 +3,7 @@ on:
pull_request:
paths:
- 'Makefile'
- '.github/actions/*'
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we depend on actions, we should schedule this workflow when it changes.

run: |
echo "::set-output name=python-version::$(SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh get-pyenv-version)"
echo "::set-output name=pip-cache-dir::$(pip3 cache dir)"
echo "::set-output name=pip-version::$(pip -V | awk -F ' ' '{print $2}')"
Copy link
Member Author

Choose a reason for hiding this comment

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

These are not necessary because the local action setup-python now takes care of it.


- name: Cache (pyenv)
uses: actions/cache@v2
with:
path: ~/.pyenv
key: devenv-${{ matrix.os }}-pyenv-${{ hashFiles('.python-version') }}

- name: Cache (pip)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not necessary because the local action setup-python now takes care of it.

@@ -83,9 +74,10 @@ jobs:
eval "$(pyenv init --path)"
python -m venv .venv
source .venv/bin/activate
make bootstrap
make develop init-config
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we can't get Docker properly initialized we will go back to what we used to have.

uses: ./.github/actions/setup-python
with:
# XXX: We need to pass this python-deps-${{ matrix.os }}-py${{ matrix.python-version }}-${{ hashFiles('requirements-*.txt') }}
cache-files-hash: ${{ hashFiles('requirements-*.txt') }}
Copy link
Member Author

Choose a reason for hiding this comment

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

We should be fine using the default cache key.

@dashed
Copy link
Member

dashed commented Sep 1, 2021

⚰️

@armenzg
Copy link
Member Author

armenzg commented Sep 1, 2021

⚰️

🤣

@@ -83,9 +75,4 @@ jobs:
eval "$(pyenv init --path)"
python -m venv .venv
source .venv/bin/activate
make bootstrap

- name: Test direnv
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this as it does not seem to execute. I wonder if it ever did.

@armenzg armenzg marked this pull request as ready for review September 2, 2021 21:48
@armenzg armenzg requested a review from a team as a code owner September 2, 2021 21:48
Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

:(

@armenzg armenzg merged commit 0628cec into master Sep 8, 2021
@armenzg armenzg deleted the armenzg/ci/no-make-boostrap branch September 8, 2021 13:01
armenzg added a commit that referenced this pull request Sep 10, 2021
This is a regression from PR #28352 since the cache for Python deps did not bust,
thus, it used the wheel from the cache (`xmlsec-1.3.11-cp36-cp36m-macosx_10_14_x86_64.whl`).

`xmlsec` does not produce wheels for Mac.
armenzg added a commit that referenced this pull request Sep 10, 2021
This is a regression from PR #28352 since the cache for Python deps did not bust,
thus, it used the wheel from the cache (`xmlsec-1.3.11-cp36-cp36m-macosx_10_14_x86_64.whl`).

`xmlsec` does not produce wheels for Mac.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 24, 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.

3 participants