feat(ci): Skip of jobs instead of most steps within job #34004

armenzg · 2022-04-27T14:52:39Z

Until now we have used skipping of steps in a job as a way of reducing CI usage when certain jobs should not be triggered depending on the files being touched.

Now that #33455 & #33784 have been merged, we now have collector jobs (backend/frontend jobs) that represent all the other jobs in them.

This change will make a variable number of jobs be skipped depending on the files being touched.

Until now we have used skipping of steps in a job as a way of reducing CI usage when certain jobs should not be triggered depending on the files being touched. Now that #33455 & #33784 have been merged, we now have collector jobs (backend/frontend jobs) that represent all the other jobs in them. This change will make a variable number of jobs be skipped depending on the files being touched.

armenzg · 2022-05-05T12:23:01Z

.github/workflows/acceptance.yml

@@ -33,6 +33,7 @@ jobs:
          filters: .github/file-filters.yml

  frontend:
+    if: ${{ needs.files-changed.outputs.acceptance == 'true' }}


This is the same as FE or BE [1]
Checked that the variable is exported in files-changed [2]
[1]

sentry/.github/file-filters.yml

Lines 59 to 61 in eb1efc4

acceptance: &acceptance

- *backend

- *frontend

[2]

sentry/.github/workflows/acceptance.yml

Lines 17 to 23 in eb1efc4

files-changed:

name: detect what files changed

runs-on: ubuntu-20.04

timeout-minutes: 3

# Map a step output to a job output

outputs:

acceptance: ${{ steps.changes.outputs.acceptance }}

armenzg · 2022-05-05T12:25:29Z

.github/workflows/backend.yml

@@ -29,6 +29,7 @@ jobs:
          filters: .github/file-filters.yml

  api-docs:
+    if: needs.files-changed.outputs.api_docs == 'true'


Checked that api_docs is exported here:

sentry/.github/workflows/backend.yml

Lines 10 to 20 in eb1efc4

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 }}

armenzg · 2022-05-05T12:26:35Z

.github/workflows/backend.yml

        # 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'


backend is exported:

sentry/.github/workflows/backend.yml

Lines 10 to 20 in eb1efc4

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 }}

armenzg · 2022-05-05T12:27:59Z

.github/workflows/backend.yml

@@ -200,12 +194,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()


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

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

armenzg · 2022-05-05T12:28:19Z

.github/workflows/backend.yml

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

  migration:
+    if: needs.files-changed.outputs.migration_lockfile == 'true'


Exported:

sentry/.github/workflows/backend.yml

Lines 10 to 20 in eb1efc4

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 }}

armenzg · 2022-05-05T16:00:36Z

.github/file-filters.yml

@@ -24,7 +24,7 @@ frontend: &frontend
  - 'docs-ui/**'
  - 'static/**'
  - 'tests/js/**'
-  - '.github/workflows/frontend.yml'
+  # - '.github/workflows/frontend.yml'


Temp change just for testing

armenzg · 2022-05-05T16:02:19Z

.github/workflows/frontend.yml

-        with:
-          list-files: shell
-          token: ${{ github.token }}
-          filters: .github/file-filters.yml


I'm removing this since it's not being used. Check that we don't use steps.changes but we use needs.files-changed.

This reverts commit b8d2716.

This reverts commit 760afcd.

armenzg · 2022-05-05T16:29:25Z

Fake visual snapshot [1]
Screenshots of the jobs skipped [2]

[1]

[2]

armenzg · 2022-05-05T16:45:41Z

This shows that this PR skipped all the jobs I wanted and satisfied the GH required checks. I'm going to push one commit to undo my testing changes

This reverts commit 8ec947c.

armenzg · 2022-05-05T18:12:11Z

I will not merge this until Monday morning.

asottile-sentry

asottile-sentry · 2022-05-05T19:01:13Z

.github/workflows/acceptance.yml

@@ -112,6 +113,7 @@ jobs:
          type: frontend

  webpack:
+    if: ${{ needs.files-changed.outputs.acceptance == 'true' }}


some of these use if ${{ ... }} and others just use if: ... -- the brackets aren't needed I think

asottile-sentry · 2022-05-05T19:01:50Z

.github/workflows/acceptance.yml

+    steps:
+      - name: Sentaur attack
+        run: |
+          echo "This check pretends to be the Visual Snapshot to satisfy Github required checks"


clever, lol

asottile-sentry

…FE changes This is a fallout from #34004. This got noticed in [this run](https://github.com/getsentry/sentry/runs/6379373504?check_suite_focus=true) were an FE check failed yet the collector job got skipped instead.

…FE changes (#34486) This is a fallout from #34004. This got noticed in [this run](https://github.com/getsentry/sentry/runs/6379373504?check_suite_focus=true) were an FE check failed yet the collector job got skipped instead.

armenzg self-assigned this Apr 27, 2022

armenzg added the Component: CI Continuous Integration pipeline (GitHub Actions) label Apr 27, 2022

vercel bot deployed to Preview – storybook April 27, 2022 14:54 View deployment

vercel bot deployed to Preview – sentry April 27, 2022 14:54 View deployment

Merge branch 'master' into armenzg/ci/skipping-of-jobs

f093ed0

vercel bot deployed to Preview – sentry May 5, 2022 12:24 View deployment

vercel bot deployed to Preview – storybook May 5, 2022 12:24 View deployment

armenzg added 2 commits May 5, 2022 08:32

XXX: Remove this

8ec947c

Fix

e9132b5

vercel bot deployed to Preview – sentry May 5, 2022 12:37 View deployment

vercel bot deployed to Preview – storybook May 5, 2022 12:37 View deployment

Minor tweaks

bfd65a3

vercel bot deployed to Preview – storybook May 5, 2022 12:47 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 12:47 View deployment

armenzg commented May 5, 2022

View reviewed changes

Skip trigger visual snapshots

760afcd

vercel bot deployed to Preview – storybook May 5, 2022 16:09 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 16:09 View deployment

Fake visual snapshot

b8d2716

vercel bot deployed to Preview – storybook May 5, 2022 16:14 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 16:14 View deployment

armenzg added 3 commits May 5, 2022 12:22

Revert "Fake visual snapshot"

2b21296

This reverts commit b8d2716.

Revert "Skip trigger visual snapshots"

0880247

This reverts commit 760afcd.

Fake visual snapshot

6a1db23

vercel bot deployed to Preview – storybook May 5, 2022 16:26 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 16:26 View deployment

Revert "XXX: Remove this"

331787a

This reverts commit 8ec947c.

vercel bot deployed to Preview – storybook May 5, 2022 16:49 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 16:49 View deployment

armenzg marked this pull request as ready for review May 5, 2022 17:58

armenzg requested a review from a team as a code owner May 5, 2022 17:58

asottile-sentry approved these changes May 5, 2022

View reviewed changes

Remove unnecessary symbols

230fb20

vercel bot deployed to Preview – storybook May 5, 2022 19:36 View deployment

vercel bot deployed to Preview – sentry May 5, 2022 19:36 View deployment

asottile-sentry approved these changes May 5, 2022

View reviewed changes

armenzg mentioned this pull request May 6, 2022

Reduce Github required checks #33408

Closed

armenzg changed the title ~~feat(ci): Support skipping of jobs (#33541)~~ feat(ci): Skip of jobs instead of most steps within job May 6, 2022

Merge branch 'master' into armenzg/ci/skipping-of-jobs

ad200f8

armenzg enabled auto-merge (squash) May 9, 2022 14:28

vercel bot deployed to Preview – storybook May 9, 2022 14:30 View deployment

vercel bot deployed to Preview – sentry May 9, 2022 14:35 View deployment

armenzg disabled auto-merge May 9, 2022 14:49

armenzg merged commit 21cf43f into master May 9, 2022

armenzg deleted the armenzg/ci/skipping-of-jobs branch May 9, 2022 14:50

armenzg mentioned this pull request May 11, 2022

fix(ci): Do not skip acceptance collector job if there are FE changes #34486

Merged

github-actions bot locked and limited conversation to collaborators May 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ci): Skip of jobs instead of most steps within job #34004

feat(ci): Skip of jobs instead of most steps within job #34004

armenzg commented Apr 27, 2022

armenzg May 5, 2022

armenzg May 5, 2022

armenzg May 5, 2022

armenzg May 5, 2022

asottile-sentry May 5, 2022

armenzg May 5, 2022

armenzg May 5, 2022

armenzg May 5, 2022

armenzg commented May 5, 2022

armenzg commented May 5, 2022

armenzg commented May 5, 2022

asottile-sentry left a comment

asottile-sentry May 5, 2022

asottile-sentry May 5, 2022

asottile-sentry left a comment

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

feat(ci): Skip of jobs instead of most steps within job #34004

feat(ci): Skip of jobs instead of most steps within job #34004

Conversation

armenzg commented Apr 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armenzg commented May 5, 2022

armenzg commented May 5, 2022

armenzg commented May 5, 2022

asottile-sentry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asottile-sentry left a comment

Choose a reason for hiding this comment