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): Skip of jobs instead of most steps within job #34004

Merged
merged 13 commits into from
May 9, 2022
23 changes: 18 additions & 5 deletions .github/workflows/acceptance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
filters: .github/file-filters.yml

frontend:
if: needs.files-changed.outputs.acceptance == 'true'
needs: files-changed
name: frontend tests
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -112,6 +113,7 @@ jobs:
type: frontend

webpack:
if: needs.files-changed.outputs.acceptance == 'true'
needs: files-changed
name: create frontend bundle
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -170,6 +172,7 @@ jobs:
path: dist.tar.gz

acceptance:
if: needs.files-changed.outputs.acceptance == 'true'
needs: files-changed
name: acceptance
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -237,9 +240,10 @@ jobs:

- name: Handle artifacts
uses: ./.github/actions/artifacts
if: ${{ needs.files-changed.outputs.backend == 'true' }}
if: needs.files-changed.outputs.backend == 'true'

chartcuterie:
if: needs.files-changed.outputs.acceptance == 'true'
needs: files-changed
name: chartcuterie integration
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -302,18 +306,16 @@ jobs:

- name: Handle artifacts
uses: ./.github/actions/artifacts
if: ${{ needs.files-changed.outputs.backend == 'true' }}
if: needs.files-changed.outputs.backend == 'true'

visual-diff:
if: needs.files-changed.outputs.backend_any_type == 'true'
# This guarantees that we will only schedule Visual Snapshots if all
# workflows that generate artifacts succeed
needs: [acceptance, frontend, chartcuterie]
name: triggers visual snapshot
# This is necessary since a failed/skipped dependent job would cause this job to be skipped
if: always()
runs-on: ubuntu-20.04
timeout-minutes: 20

steps:
# If any jobs we depend on fail, we will fail since this checks triggers Visual Snapshots which is a required check
- name: Check for failures
Expand All @@ -330,3 +332,14 @@ jobs:
api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
gcs-bucket: 'sentry-visual-snapshots'
gcp-service-account-key: ${{ secrets.SNAPSHOT_GOOGLE_SERVICE_ACCOUNT_KEY }}

# Since Visual Snapshot is a required check we need to pretend to have run
fake-visual-snapshot:
name: Visual Snapshot
# Opposite condition to "triggers visual snapshot" required check
if: needs.files-changed.outputs.backend_any_type != 'true'
runs-on: ubuntu-20.04
steps:
- name: Sentaur attack
run: |
echo "This check pretends to be the Visual Snapshot to satisfy Github required checks"
Copy link
Member

Choose a reason for hiding this comment

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

clever, lol

39 changes: 11 additions & 28 deletions .github/workflows/backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jobs:
filters: .github/file-filters.yml

api-docs:
if: needs.files-changed.outputs.api_docs == 'true'
Copy link
Member Author

Choose a reason for hiding this comment

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

Checked that api_docs is exported here:

files-changed:
name: detect what files changed
runs-on: ubuntu-20.04
timeout-minutes: 3
# Map a step output to a job output
outputs:
api_docs: ${{ steps.changes.outputs.api_docs }}
backend: ${{ steps.changes.outputs.backend }}
backend_any_type: ${{ steps.changes.outputs.backend_any_type }}
migration_lockfile: ${{ steps.changes.outputs.migration_lockfile }}
plugins: ${{ steps.changes.outputs.plugins }}

needs: files-changed
name: api docs test
runs-on: ubuntu-20.04
Expand All @@ -39,26 +40,24 @@ jobs:
- uses: actions/checkout@v2

- uses: volta-cli/action@v1
if: needs.files-changed.outputs.api_docs == 'true'

- name: Setup sentry python env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
id: setup
if: needs.files-changed.outputs.api_docs == 'true'
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
snuba: true

- name: Run API docs tests
if: needs.files-changed.outputs.api_docs == 'true'
# install ts-node for ts build scripts to execute properly without potentially installing
# conflicting deps when running scripts locally
# see: https://github.com/getsentry/sentry/pull/32328/files
run: |
yarn add ts-node && make test-api-docs

backend-test:
if: needs.files-changed.outputs.backend == 'true'
Copy link
Member Author

Choose a reason for hiding this comment

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

backend is exported:

files-changed:
name: detect what files changed
runs-on: ubuntu-20.04
timeout-minutes: 3
# Map a step output to a job output
outputs:
api_docs: ${{ steps.changes.outputs.api_docs }}
backend: ${{ steps.changes.outputs.backend }}
backend_any_type: ${{ steps.changes.outputs.backend_any_type }}
migration_lockfile: ${{ steps.changes.outputs.migration_lockfile }}
plugins: ${{ steps.changes.outputs.plugins }}

needs: files-changed
name: backend test
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -87,7 +86,6 @@ jobs:

- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
if: needs.files-changed.outputs.backend == 'true'
id: setup
with:
python-version: ${{ matrix.python-version }}
Expand All @@ -96,7 +94,6 @@ jobs:
pg-version: ${{ matrix.pg-version }}

- name: Run backend test (${{ steps.setup.outputs.matrix-instance-number }} of ${{ steps.setup.outputs.matrix-instance-total }})
if: needs.files-changed.outputs.backend == 'true'
run: |
# Note: `USE_SNUBA` is not used for backend tests because there are a few failing tests with Snuba enabled.
unset USE_SNUBA
Expand All @@ -106,6 +103,7 @@ jobs:
uses: ./.github/actions/artifacts

cli:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: cli test
runs-on: ubuntu-20.04
Expand All @@ -120,21 +118,20 @@ jobs:
- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
id: setup
if: needs.files-changed.outputs.backend == 'true'
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
pg-version: ${{ matrix.pg-version }}

- name: Run test
if: needs.files-changed.outputs.backend == 'true'
run: |
make test-cli

- name: Handle artifacts
uses: ./.github/actions/artifacts

lint:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: backend lint
runs-on: ubuntu-20.04
Expand All @@ -155,20 +152,17 @@ jobs:

- name: Setup Python ${{ matrix.python-version }}
uses: ./.github/actions/setup-python
if: needs.files-changed.outputs.backend == 'true'
with:
python-version: ${{ matrix.python-version }}
# Note this uses a different cache key than other backend-based workflows because this workflows dependencies are different
cache-files-hash: ${{ hashFiles('requirements-pre-commit.txt') }}

- uses: actions/cache@v3
if: needs.files-changed.outputs.backend == 'true'
with:
path: ~/.cache/pre-commit
key: cache-epoch-1|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}

- name: Setup pre-commit
if: needs.files-changed.outputs.backend == 'true'
env:
SENTRY_NO_VIRTUALENV_CREATION: 1
run: |
Expand All @@ -191,7 +185,6 @@ jobs:
- added|modified: 'requirements-base.txt'

- name: Run pre-commit on changed files
if: needs.files-changed.outputs.backend == 'true'
run: |
# Run pre-commit to lint and format check files that were changed (but not deleted) compared to master.
# XXX: there is a very small chance that it'll expand to exceed Linux's limits
Expand All @@ -200,12 +193,13 @@ jobs:

# If working tree is dirty, commit and update if we have a token
- name: Apply any pre-commit fixed files
if: steps.token.outcome == 'success' && github.ref != 'refs/heads/master' && needs.files-changed.outputs.backend == 'true' && always()
if: steps.token.outcome == 'success' && github.ref != 'refs/heads/master' && always()
Copy link
Member Author

Choose a reason for hiding this comment

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

Why is always() in that condition? It seems unnecessary without further investigation.

Copy link
Member

Choose a reason for hiding this comment

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

pre-commit will fail (exit nonzero) and the intent of this is to run anyways and push the files back to the PR

we could scrap all of this and use https://pre-commit.ci

uses: getsentry/action-github-commit@main
with:
github-token: ${{ steps.token.outputs.token }}

migration:
if: needs.files-changed.outputs.migration_lockfile == 'true'
Copy link
Member Author

Choose a reason for hiding this comment

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

Exported:

files-changed:
name: detect what files changed
runs-on: ubuntu-20.04
timeout-minutes: 3
# Map a step output to a job output
outputs:
api_docs: ${{ steps.changes.outputs.api_docs }}
backend: ${{ steps.changes.outputs.backend }}
backend_any_type: ${{ steps.changes.outputs.backend_any_type }}
migration_lockfile: ${{ steps.changes.outputs.migration_lockfile }}
plugins: ${{ steps.changes.outputs.plugins }}

needs: files-changed
name: check migration
runs-on: ubuntu-20.04
Expand All @@ -217,26 +211,24 @@ jobs:
steps:
- name: Checkout sentry
uses: actions/checkout@v2
if: needs.files-changed.outputs.migration_lockfile == 'true'

- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
if: needs.files-changed.outputs.migration_lockfile == 'true'
id: setup
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
pg-version: ${{ matrix.pg-version }}

- name: Migration & lockfile checks
if: needs.files-changed.outputs.migration_lockfile == 'true'
env:
SENTRY_LOG_LEVEL: ERROR
PGPASSWORD: postgres
run: |
./.github/workflows/scripts/migration-check.sh

plugins:
if: needs.files-changed.outputs.plugins == 'true'
needs: files-changed
name: plugins test
runs-on: ubuntu-20.04
Expand All @@ -250,18 +242,17 @@ jobs:
- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
id: setup
if: needs.files-changed.outputs.plugins == 'true'
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
snuba: true

- name: Run test
if: needs.files-changed.outputs.plugins == 'true'
run: |
make test-plugins

relay:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: relay test
runs-on: ubuntu-20.04
Expand All @@ -279,29 +270,27 @@ jobs:
- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
id: setup
if: needs.files-changed.outputs.backend == 'true'
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
snuba: true
kafka: true

- name: Pull relay image
if: needs.files-changed.outputs.backend == 'true'
run: |
# pull relay we'll run and kill it for each test
docker pull us.gcr.io/sentryio/relay:nightly
docker ps -a

- name: Run test
if: needs.files-changed.outputs.backend == 'true'
run: |
make test-relay-integration

- name: Handle artifacts
uses: ./.github/actions/artifacts

snuba:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: snuba test
runs-on: ubuntu-20.04
Expand Down Expand Up @@ -329,7 +318,6 @@ jobs:

- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
if: needs.files-changed.outputs.backend == 'true'
id: setup
with:
python-version: ${{ matrix.python-version }}
Expand All @@ -338,14 +326,14 @@ jobs:
kafka: true

- name: Run snuba test (${{ steps.setup.outputs.matrix-instance-number }} of ${{ steps.setup.outputs.matrix-instance-total }})
if: needs.files-changed.outputs.backend == 'true'
run: |
make test-snuba

- name: Handle artifacts
uses: ./.github/actions/artifacts

symbolicator:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: symbolicator test
runs-on: ubuntu-20.04
Expand All @@ -363,15 +351,13 @@ jobs:
- name: Setup sentry env (python ${{ matrix.python-version }})
uses: ./.github/actions/setup-sentry
id: setup
if: needs.files-changed.outputs.backend == 'true'
with:
python-version: ${{ matrix.python-version }}
pip-cache-version: ${{ secrets.PIP_CACHE_VERSION }}
snuba: true
kafka: true

- name: Start symbolicator
if: needs.files-changed.outputs.backend == 'true'
run: |
echo $PWD
docker run \
Expand All @@ -384,14 +370,14 @@ jobs:
docker ps -a

- name: Run test
if: needs.files-changed.outputs.backend == 'true'
run: |
make test-symbolicator

- name: Handle artifacts
uses: ./.github/actions/artifacts

typing:
if: needs.files-changed.outputs.backend == 'true'
needs: files-changed
name: backend typing
runs-on: ubuntu-20.04
Expand All @@ -404,13 +390,11 @@ jobs:

- name: Setup Python ${{ matrix.python-version }}
uses: ./.github/actions/setup-python
if: needs.files-changed.outputs.backend == 'true'
with:
python-version: ${{ matrix.python-version }}

# Since we don't call the setup-sentry action we need to install libxmlsec1-dev
- name: Setup backend typing
if: needs.files-changed.outputs.backend == 'true'
env:
SENTRY_LIGHT_BUILD: 1
run: |
Expand All @@ -421,7 +405,6 @@ jobs:
pip install -U -e ".[dev]"

- name: Run backend typing (${{ steps.setup.outputs.matrix-instance-number }} of ${{ strategy.job-total }})
if: needs.files-changed.outputs.backend == 'true'
run: |
make backend-typing

Expand Down
Loading