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(dev_env): Dev env bootstrap support for two Python versions #28213

Merged
merged 29 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
df7e7b4
feat(dev_env): Dev env bootstrap support for two Python versions
armenzg Aug 26, 2021
54ac396
Fix the CI
armenzg Aug 26, 2021
954436d
Debug info
armenzg Aug 26, 2021
6f7d952
Fix
armenzg Aug 27, 2021
354a915
Fix
armenzg Aug 27, 2021
b206418
Debug info
armenzg Aug 27, 2021
cf06765
Fix
armenzg Aug 27, 2021
859f94e
Do not check venv
armenzg Aug 27, 2021
3d0d20f
Minor changes
armenzg Aug 27, 2021
25374e5
Merge branch 'master' into armenzg/devenv/support-dual-python-apple-m1
armenzg Aug 27, 2021
c83ec68
Fix dev env Ubuntu
armenzg Aug 27, 2021
195314c
Merge branch 'master' into armenzg/devenv/support-dual-python-apple-m1
armenzg Aug 31, 2021
95b50a9
Apply suggestions from code review
armenzg Aug 31, 2021
581e547
Address feedback
armenzg Aug 31, 2021
79e981d
Update version
armenzg Aug 31, 2021
1f06ea7
Pin google-crc32c
armenzg Aug 31, 2021
d062e20
Pin google-crc32c
armenzg Aug 31, 2021
559c42e
Try this
armenzg Aug 31, 2021
bcc2be3
Add note
armenzg Aug 31, 2021
b64df79
Merge branch 'master' into armenzg/devenv/support-dual-python-apple-m1
armenzg Aug 31, 2021
baa79fa
Fix variable name
armenzg Aug 31, 2021
00c954a
Fix Python deps
armenzg Aug 31, 2021
9c1d793
Try brew bundle in case it failed when we did not update
armenzg Aug 31, 2021
191d019
Fix calling setup-python
armenzg Aug 31, 2021
9f326a6
Fix mismatch
armenzg Sep 1, 2021
ea8aeb9
Merge branch 'master' into armenzg/devenv/support-dual-python-apple-m1
armenzg Sep 1, 2021
9f2778b
Typo
armenzg Sep 1, 2021
57d4a02
Try again
armenzg Sep 1, 2021
aaecdf4
Try again
armenzg Sep 1, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 29 additions & 29 deletions .envrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@
# initialization (e.g. activating the venv) or giving recommendations on how to reach the desired state.
# It also sets useful environment variables.
# If you'd like to override or set any custom environment variables, this .envrc will read a .env file at the end.

set -e
HERE="$(
SENTRY_ROOT="$(
cd "$(dirname "${BASH_SOURCE[0]}")"
pwd -P
)"

bold="$(tput bold)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Centralizing this code for other scripts to use.

red="$(tput setaf 1)"
green="$(tput setaf 2)"
yellow="$(tput setaf 3)"
reset="$(tput sgr0)"
source "${SENTRY_ROOT}/scripts/lib.sh"

# XXX: we can't trap bash EXIT, because it'll override direnv's finalizing routines.
# consequently, using "exit" anywhere will skip this notice from showing.
Expand Down Expand Up @@ -58,10 +53,6 @@ report_to_sentry() {
rm "$_SENTRY_LOG_FILE"
}

require() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving it to the main library.

command -v "$1" >/dev/null 2>&1
}

debug() {
if [ "${SENTRY_DIRENV_DEBUG}" ]; then
echo -e "${@}"
Expand All @@ -86,6 +77,27 @@ die() {
return 1
}

prompt_python_venv_creation() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving this code into a function so we can call it from within the .envrc context in getsentry (since we source it).

# We depend on a Sentry checkout for this to work.
source ../sentry/.envrc

echo -e "${yellow}You are missing a Python virtualenv and we ${bold}need${reset}${yellow} to run a bootstrapping script (it can take a few minutes)"
info "About to create ${venv_name}..."
echo -e "\nContinue (y/N)?"
read -r resp
case "$resp" in
y | Y) echo "Okay, let's do this." ;;
*)
die "Aborted!"
;;
esac
}

show_commands_info() {
echo -e "\n${red}Run the following commands to bring your environment up-to-date:"
for cmd in "${commands_to_run[@]}"; do
warn " ${red}$cmd"
done
echo ""
}

### Environment ###

commands_to_run=()
Expand All @@ -111,9 +123,9 @@ export SENTRY_UI_HOT_RELOAD=1

### You can override the exported variables with a .env file
# All exports should happen before here unless they're safeguarded (see devenv error reporting below)
if [ -f "${HERE}/.env" ]; then
debug "Loading variables from ${HERE}/.env"
dotenv "${HERE}/.env"
if [ -f "${SENTRY_ROOT}/.env" ]; then
info "Loading variables from ${SENTRY_ROOT}/.env"
dotenv "${SENTRY_ROOT}/.env"
fi

## Notify of reporting to Sentry
Expand Down Expand Up @@ -156,15 +168,7 @@ make setup-git-config
venv_name=".venv"

if [ ! -f "${venv_name}/bin/activate" ]; then
warn "You are missing a Python virtualenv and we ${bold}need${reset} to run a bootstrapping script (it can take a few minutes)"
echo -e "\nContinue (y/N)?"
read -r resp
case "$resp" in
y | Y) echo "Okay, let's do this." ;;
*)
die "Aborted!"
;;
esac
prompt_python_venv_creation
# This is time consuming but it has to be done
source ./scripts/bootstrap-py3-venv
fi
Expand All @@ -181,7 +185,7 @@ source "${venv_name}/bin/activate"
unset PS1

debug "Ensuring proper virtualenv..."
"${HERE}/scripts/ensure-venv.sh"
"${SENTRY_ROOT}/scripts/ensure-venv.sh"

if ! require sentry; then
warn "Your virtualenv is activated, but sentry doesn't seem to be installed."
Expand Down Expand Up @@ -216,11 +220,7 @@ PATH_add node_modules/.bin

# These are commands that can take a significant amount of time
if [ ${#commands_to_run[@]} -ne 0 ]; then
echo -e "\n${red}Run the following commands to bring your environment up-to-date:"
for cmd in "${commands_to_run[@]}"; do
warn " ${red}$cmd"
done
echo ""
show_commands_info
fi

if [ "${log_level}" != "info" ]; then
Expand Down
4 changes: 2 additions & 2 deletions .github/actions/setup-python/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ inputs:
description: 'pip cache version in order to bust cache'
required: false
python-version:
description: "python version to install"
description: 'python version to install'
required: false

outputs:
Expand All @@ -38,7 +38,7 @@ runs:
shell: bash
run: |
if [ "${{ inputs.python-version }}" == "" ]; then
echo "::set-output name=python-version::$(cat .python-version)"
echo "::set-output name=python-version::$(SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh get-pyenv-version)"
else
echo "::set-output name=python-version::${{ inputs.python-version }}"
fi
Expand Down
20 changes: 10 additions & 10 deletions .github/workflows/development-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ name: dev env
on:
pull_request:
paths:
- 'Makefile'
- '.github/workflows/development-environment.yml'
- '.envrc'
- 'Brewfile'
- 'scripts/*'
- 'src/sentry/runner/commands/devserver.py'
- 'src/sentry/runner/commands/devservices.py'
- 'Makefile'
- '.github/workflows/development-environment.yml'
- '.envrc'
- 'Brewfile'
- 'scripts/*'
- 'src/sentry/runner/commands/devserver.py'
- 'src/sentry/runner/commands/devservices.py'
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 auto formatting. No changes.


jobs:
dev-environment:
Expand All @@ -20,7 +20,7 @@ jobs:
# macosx-11.0 is Big Sur, however, it takes long for jobs to get started
# Using Ubuntu 18 until I figure out this error:
# -> ImportError: libffi.so.6: cannot open shared object file: No such file or directory
os: [ macos-11.0, ubuntu-18.04 ]
os: [macos-11.0, ubuntu-18.04]
fail-fast: false
env:
PIP_DISABLE_PIP_VERSION_CHECK: on
Expand All @@ -36,15 +36,15 @@ jobs:
# Sometimes, brew needs to be updated before brew bundle would work
# After installing Docker (via homebrew) we need to make sure that it is properly initialized on Mac
run: |
brew update -q && brew bundle -q
brew update && brew bundle -q
# This code is mentioned in our dev docs. Only remove if you adjust the docs as well
SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh init-docker

# The next few steps are to set up the cache quickly
- name: Set environment variables & others
id: info
run: |
echo "::set-output name=python-version::$(cat .python-version)"
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}')"
echo "::set-output name=yarn-cache-dir::$(yarn cache dir)"
Expand Down
31 changes: 11 additions & 20 deletions .github/workflows/python-deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ name: python deps
on:
pull_request:
paths:
- '.github/workflows/python-deps.yml'
- 'requirements*'
- '.github/workflows/python-deps.yml'
- 'requirements*'
Copy link
Member Author

Choose a reason for hiding this comment

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

Auto-formatting.

Copy link
Member

Choose a reason for hiding this comment

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

What is doing the auto formatting?

Copy link
Member Author

Choose a reason for hiding this comment

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

The Prettier VS Code extension (we recommend it).

I spend some time looking into it. It does not always execute to completion, thus, it does not always auto-format.
image

Copy link
Member

Choose a reason for hiding this comment

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

Ah we should make this run on precommit

Copy link
Member Author

Choose a reason for hiding this comment

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

Added to backlog


jobs:
# This workflow makes sure that Python dependencies install correctly for
Expand All @@ -14,8 +14,8 @@ jobs:
timeout-minutes: 20
strategy:
matrix:
os: [ macos-11.0, ubuntu-20.04 ]
python-version: [ 3.6.13, 3.8.10 ]
os: [macos-11.0, ubuntu-20.04]
python-version: [3.6.13, 3.8.11]
Copy link
Member Author

Choose a reason for hiding this comment

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

Updating the version to 3.8.11

fail-fast: false
env:
PIP_DISABLE_PIP_VERSION_CHECK: on
Expand All @@ -27,25 +27,16 @@ jobs:
- uses: actions/checkout@v2

- name: Install prerequisites
# Sometimes, brew needs to be updated before brew bundle would work
run: |
HOMEBREW_NO_AUTO_UPDATE=1 brew bundle --no-upgrade
brew update && brew bundle -q

# Until GH composite actions can use `uses`, we need to setup python here
- uses: actions/setup-python@v2
- name: Setup python
id: setup-python
uses: ./.github/actions/setup-python
with:
python-version: ${{ matrix.python-version }}

- name: Setup pip
uses: ./.github/actions/setup-pip
id: pip

- name: Cache
uses: actions/cache@v2
with:
path: |
${{ steps.pip.outputs.pip-cache-dir }}
key: |
python-deps-${{ matrix.os }}-py${{ matrix.python-version }}-${{ hashFiles('requirements-*.txt') }}
# XXX: We need to pass this python-deps-${{ matrix.os }}-py${{ matrix.python-version }}-${{ hashFiles('requirements-*.txt') }}
cache-files-hash: ${{ hashFiles('requirements-*.txt') }}

- name: Install dependencies
run: |
Expand Down
1 change: 0 additions & 1 deletion .python-version

This file was deleted.

3 changes: 3 additions & 0 deletions requirements-base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ google-cloud-core==1.5.0
googleapis-common-protos==1.52.0
google-cloud-pubsub==2.2.0
google-cloud-storage==1.35.0
# Only necessary to prevent installing the latest version
# https://github.com/googleapis/python-crc32c/issues/83
google-crc32c==1.1.2; python_version == '3.8'
Copy link
Member Author

Choose a reason for hiding this comment

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

See googleapis/python-crc32c#83

These google dependencies might end up moving into the dev requirements (#28305).

jsonschema==3.2.0
lxml==4.6.3
maxminddb==2.0.3
Expand Down
16 changes: 11 additions & 5 deletions scripts/bootstrap-py3-venv
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
# trap "echo 'bootstrap FAILED.' && trap - ERR && return" ERR

# So just going to sprinkle returns everywhere.
SCRIPTS_DIR="$(
cd "$(dirname "${BASH_SOURCE[0]}")"
pwd -P
)"
source "${SCRIPTS_DIR}/lib.sh"
Copy link
Member Author

@armenzg armenzg Aug 27, 2021

Choose a reason for hiding this comment

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

This is used in order to import query-valid-python-version and use it below.


command -v pyenv >/dev/null || {
echo "You need to install pyenv. https://develop.sentry.dev/environment/#python"
Expand All @@ -24,7 +29,10 @@ command -v direnv >/dev/null || {

gitroot="$(git rev-parse --show-toplevel)"
cd "$gitroot"
export venv_name="${PWD}/.venv"
# This executes very quickly if the Python versions are already installed
make setup-pyenv
export PYENV_VERSION=$(SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh get-pyenv-version)
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to set the variable so pyenv will know what Python version to change to.
I've tested this on both my machines.

venv_name="${PWD}/.venv"

if [[ -f "${venv_name}/bin/activate" ]]; then
prompt_text="You have an existing virtualenv. This script will OVERWRITE it. Continue (y/N)?"
Expand All @@ -40,17 +48,15 @@ fi
deactivate 2>/dev/null || true
rm -rf "$venv_name"

# XXX: You'll also need to temporarily pyenv local "$SENTRY_PYTHON_VERSION"
# and then revert changes to the .python-version after bootstrap finishes.
if [[ -z "$SENTRY_PYTHON_VERSION" ]] && ! [[ "$(python3 -V 2>&1)" = "Python $(grep "3.6" .python-version)" ]]; then
if [[ -z "$SENTRY_PYTHON_VERSION" ]] && ! query-valid-python-version; then
echo "Your python3 version isn't as expected. Please run: make setup-pyenv"
return 1
fi

python3 -m pip install -U pip || { echo "bootstrap failed!"; return 1; }
python3 -m venv "${venv_name}" || { echo "bootstrap failed!"; return 1; }
source "${venv_name}/bin/activate" || { echo "bootstrap failed!"; return 1; }
python3 -m pip install -U pip || { echo "bootstrap failed!"; return 1; }
python3 -m pip install -U pip wheel || { echo "bootstrap failed!"; return 1; }
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 it makes the installation of many packages faster. For instance, make setup-git installs from requirements-pre-commit.txt, thus, the chance we can save some time if any package was to need to build a wheel.

make setup-git || { echo "bootstrap failed!"; return 1; }
make install-py-dev || { echo "bootstrap failed!"; return 1; }
deactivate || { echo "bootstrap failed!"; return 1; }
Expand Down
6 changes: 4 additions & 2 deletions scripts/do.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
# This script is an interface to any of the methods of lib.sh
# Call this script as "do.sh method_from_lib" to execute any function from that library
set -eu

HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd -P)"
HERE="$(
cd "$(dirname "${BASH_SOURCE[0]}")"
pwd -P
)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Auto-formatting.

# shellcheck disable=SC1090
source "${HERE}/lib.sh"

Expand Down
49 changes: 24 additions & 25 deletions scripts/ensure-venv.sh
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
#!/bin/bash
HERE="$(
cd "$(dirname "${BASH_SOURCE[0]}")" || exit
pwd -P
)"
# shellcheck disable=SC1090
source "${HERE}/lib.sh"

# optionally opt out of virtualenv creation
# WARNING: this will be removed (most likely renamed) soon!
if [[ "$SENTRY_NO_VIRTUALENV_CREATION" == "1" ]]; then
exit 0
fi

red="$(tput setaf 1)"
yellow="$(tput setaf 3)"
bold="$(tput bold)"
reset="$(tput sgr0)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Now being imported from lib.sh.


venv_name=".venv"

die() {
Expand All @@ -21,33 +22,31 @@ EOF
}

if [[ -n "$VIRTUAL_ENV" ]]; then
minor=$(python -c "import sys; print(sys.version_info[1])")
Copy link
Member Author

Choose a reason for hiding this comment

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

The rest of this diff is easier to see side-by-side.
The main idea is to simplify the logic.

# If .venv is less than Python 3.6 fail
[[ "$minor" -lt 6 ]] &&
die "Remove $VIRTUAL_ENV and try again since the Python version installed should be at least 3.6."
# If .venv is created with Python greater than 3.6 you might encounter problems and we want to ask you to downgrade
# unless you explicitely set an environment variable
if [[ "$minor" -gt 6 ]]; then
if [[ -n "$SENTRY_PYTHON_VERSION" ]]; then
cat <<EOF
# The developer is inside a virtualenv *and* has set a SENTRY_PYTHON_VERSION
# Let's assume that they know what they're doing
if [[ -n "$SENTRY_PYTHON_VERSION" ]]; then
cat <<EOF
${yellow}${bold}
You have explicitly set a non-recommended Python version (${SENTRY_PYTHON_VERSION}). You're on your own.
You have explicitly set a non-recommended Python version ($(python -V | awk '{print $2}')). You're on your own.
Copy link
Member Author

Choose a reason for hiding this comment

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

Using this since SENTRY_PYTHON_VERSION does not actually set the version but allows non-supported Python versions.

${reset}
EOF
else
cat <<EOF
${red}${bold}
ERROR! You are running a virtualenv with a Python version different than 3.6
We recommend you start with a fresh virtualenv or to set the variable SENTRY_PYTHON_VERSION
to the Python version you want to use (e.g. 3.7).
${reset}
exit 0
fi

# Let's make sure they know that they're not using a different version by mistake

if ! query-valid-python-version; then
cat <<EOF
${yellow}${bold}
WARNING! You are running a virtualenv with a Python version ($(which python))
different than 3.6.13 or 3.8.11. We recommend you start with a fresh virtualenv OR"
use SENTRY_PYTHON_VERSION to by-pass this check.
${reset}
EOF
exit 1
fi
fi
else
if [[ ! -f "${venv_name}/bin/activate" ]]; then
die "You don't seem to have a virtualenv. Please create one by running: python3.6 -m venv ${venv_name}"
die "You don't seem to have a virtualenv. Please create one by running: source ./scripts/bootstrap-py3-venv"
Copy link
Member Author

Choose a reason for hiding this comment

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

Updating this old way of doing things.

fi
die "You have a virtualenv, but it doesn't seem to be activated. Please run: source ${venv_name}/bin/activate"
fi
Expand Down
Loading