-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(ci): Optimizations for dev env workflow #29658
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
Conversation
- Do not use the Brew cache as it takes a long time - On CI, only install `libxmlsec1` since we have full coverage via bootstrap-script - Add some checks as part of the set up step
| env: | ||
| WORKDIR: ${{ inputs.workdir }} | ||
| run: | | ||
| source "$WORKDIR/scripts/lib.sh" && upgrade-pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is merged in the next step.
| name: dev docs set up | ||
| runs-on: ${{ matrix.os }} | ||
| timeout-minutes: 90 | ||
| timeout-minutes: 40 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't fetch from Docker and it takes less than 15 minutes, thus, this should be plenty.
| # -> ImportError: libffi.so.6: cannot open shared object file: No such file or directory | ||
| os: [macos-11.0, ubuntu-18.04] | ||
| python-version: [3.8.12] | ||
| os: [macos-11.0, ubuntu-20.04] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that Ubuntu 20 magically works now :)
| os: [macos-11.0, ubuntu-20.04] | ||
| fail-fast: false | ||
| env: | ||
| PIP_DISABLE_PIP_VERSION_CHECK: on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dealt as part of the setup-python action.
| with: | ||
| path: ${{ steps.info.outputs.brew-cache-dir }} | ||
| key: devenv-${{ runner.os }}-brew-${{ hashFiles('Brewfile') }} | ||
| restore-keys: devenv-${{ runner.os }}-brew |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was adding a couple of minutes. It's probably been growing indefinitely.
| run: | | ||
| make prerequisites | ||
| - name: Setup Python ${{ matrix.python-version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use a matrix anymore.
| with: | ||
| path: ${{ steps.info.outputs.yarn-cache-dir }} | ||
| key: devenv-${{ matrix.os }}-v2-yarn-${{ hashFiles('yarn.lock') }} | ||
| key: devenv-${{ matrix.os }}-v1-yarn-${{ hashFiles('yarn.lock') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using actions/cache@v1 I assume we should have named the cache v1.
| export VOLTA_HOME="$HOME/.volta" | ||
| export PATH="$HOME/.volta/bin:$PATH" | ||
| make setup-pyenv | ||
| eval "$(pyenv init --path)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using source below helps testing a more similar code path as engineers would. We can't use exec $SHELL.
| eval "$(pyenv init --path)" | ||
| [[ $(which python) != "${HOME}/.pyenv/shims/python" ]] | ||
| source ~/.zprofile | ||
| [[ $(which python) == "${HOME}/.pyenv/shims/python" ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After sourcing, we should be using the Python version from pyenv.
| if [ -z "${CI+x}" ]; then | ||
| brew update -q && brew bundle -q | ||
| else | ||
| HOMEBREW_NO_AUTO_UPDATE=on brew install libxmlsec1 pyenv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the minimum required packages on CI to get us to success.
This comment has been minimized.
This comment has been minimized.
| - 'scripts/*' | ||
| - 'src/sentry/runner/commands/devserver.py' | ||
| - 'src/sentry/runner/commands/devservices.py' | ||
| - 'requirements-*.txt' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I've deleted the python deps workflow.
| - '.pre-commit-config.yaml' | ||
| - 'Makefile' | ||
| - '.github/actions/*' | ||
| - '.github/actions/setup-python' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only use the setup-python action.
libxmlsec1since we have full coverage via bootstrap-script