fix(CI): properly configure cancel duplicates #12625

ktmud · 2021-01-20T18:26:17Z

SUMMARY

Updating the "Cancel Duplicates" job to use our own Python script, which will try to cancel all duplicate jobs of all workflows:

For each workflow, we keep only the latest run for a given branch + event type (pull_request or push).
Older runs will be cancelled even if they are already running.

The benefit of using a custom Python script instead of a black-box like 3rd party action is that even if the cancel workflow is blocked be other long-running jobs scheduled before it, committers can still run the script locally to trigger the cancellation.

Tried to use https://github.com/potiuk/cancel-workflow-runs#most-often-used-canceling-example
but it's not very easy to configure.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

The job should start correctly and duplicate runs should be properly cancelled.

Tested on my personal fork.

Check https://github.com/ktmud/superset/actions?query=workflow%3A%22Cancel+Duplicates%22 or verification.

ADDITIONAL INFORMATION

Has associated issue: build: try to speed up Github workflows #12090 build: add cancel-workflow-runs action locally #12394
Changes UI
Requires DB Migration.
Confirm DB Migration upgrade and downgrade tested.
Introduces new feature or API
Removes existing feature or API

codecov-io · 2021-01-20T21:40:03Z

Codecov Report

Merging #12625 (197e876) into master (32f2c45) will decrease coverage by 0.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #12625      +/-   ##
==========================================
- Coverage   67.01%   66.80%   -0.21%     
==========================================
  Files        1022     1022              
  Lines       50102    50102              
  Branches     5191     5191              
==========================================
- Hits        33574    33469     -105     
- Misses      16397    16502     +105     
  Partials      131      131

Flag	Coverage Δ
cypress	`50.91% <ø> (-0.01%)`	⬇️
javascript	`61.73% <ø> (ø)`
python	`63.73% <ø> (-0.35%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/db_engines/hive.py	`0.00% <0.00%> (-85.72%)`	⬇️
superset/db_engine_specs/hive.py	`73.84% <0.00%> (-17.31%)`	⬇️
superset/db_engine_specs/presto.py	`81.38% <0.00%> (-6.71%)`	⬇️
superset/views/database/mixins.py	`80.70% <0.00%> (-1.76%)`	⬇️
...set-frontend/src/dashboard/util/getDropPosition.js	`92.06% <0.00%> (-1.59%)`	⬇️
superset/models/core.py	`88.04% <0.00%> (-0.82%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32f2c45...197e876. Read the comment docs.

villebro

LGTM. I wish there was a way to avoid copying the action into the codebase 🙁

ktmud · 2021-01-24T07:32:03Z

@potiuk, thanks for making the awesome cancel-workflow-runs GitHub action! Could you check here if I used your action correctly?

Much appreciated!

potiuk

It can be done a bit simpler (and without error state). You can use results of the previous step in the conditional execution of the cancel one.

.github/workflows/cancel_duplicates.yml

potiuk · 2021-01-24T07:41:31Z

LGTM. I wish there was a way to avoid copying the action into the codebase

Submodules works great for it!

.gitmodules

potiuk · 2021-01-24T10:25:03Z

.github/workflows/cancel_duplicates.yml

+          echo "Found $count unfinished jobs."
+          echo "::set-output name=count::$count"
+
+      - uses: potiuk/cancel-workflow-runs@953e057dc81d3458935a18d1184c386b0f6b5738


Just refer to it locally same as for other actions

And add the checkout removed above (otherwise it won't work)

potiuk · 2021-01-24T10:31:02Z

I choose 2 (periodical runs) instead of workflow_run -> requested event type as recommended by potiuk/cancel-workflow-runs's documentation because the cancel_duplicates jobs trigger this way are often put into the back of the workflow queue and therefore won't be able to execute before other jobs are finished anyway. By inserting the cancel job periodically to the queue, it may have a larger chance of being actually executed.

BTW. This is not necessarily true. If you have a lot of jobs in the queue and EACH of the jobs triggers "cancel" on their own, you do not need to add schedule. Whenever any of the scheduled jobs gets in the queue it will cancel all runs. And if there are no jobs in the queue, then ..... you have nothing to cancel because there are no jobs in the queue. So those scheduled cancels will only increase number of jobs but IMHO they will not increase the chance of cancel step actually running.

BTW. Remember that scheduled runs are also put in the queue. If you have a lot of jobs in the queue they will be added at the end of the queue. So there is little point in running them really for this case.

This was the main design decision behind "cancelAllDuplicates" behaviour - to go through all runs and cancel all duplicates.

ktmud · 2021-01-25T20:54:18Z

IMHO they will not increase the chance of cancel step actually running.

You are right. It's not like the scheduled run can insert in between the long list of checks once a PR is filed. I'll revise.

mistercrunch · 2021-01-26T06:00:02Z

@potiuk, good to see you here! and thanks for cancel-workflow-runs

ktmud · 2021-01-26T21:54:14Z

HOLD: current configuration seems to be only cancelling the Cancel Duplicates job itself: https://github.com/ktmud/superset/runs/1771801161?check_suite_focus=true

amitmiran137

looking forward for this one
CI is a living hell right now

potiuk · 2021-01-28T08:55:45Z

.github/workflows/cancel_duplicates.yml

-          notifyPRCancel: true
-          skipEventTypes: '["push", "pull_request", "pull_request_target"]'
+          token: ${{ secrets.GITHUB_TOKEN }}
+          notifyPRCancel: false


I believe since you are trying to cancel other workflows, you need to specify also
workflowFileName parameter and specify the workflow run file name that you want to cancel.

If you need to cancel more than one worklfow, you need to copy the step and use different workflowFileName,

ktmud · 2021-01-28T11:26:16Z

@potiuk , thanks for the additional information!

I decided to extend a previous Python script I wrote that was intended to use locally.

Now it's working properly: https://github.com/ktmud/superset/runs/1783573660?check_suite_focus=true

Also updated the PR description to include the intended effect.

ktmud · 2021-01-28T11:35:17Z

@villebro @amitmiran137 could you take another look on this one? I've completely changed how the jobs are cancelled. More details in PR description.

potiuk · 2021-01-28T11:41:30Z

scripts/cancel_github_workflows.py

@@ -23,13 +23,13 @@
  export GITHUB_TOKEN=394ba3b48494ab8f930fbc93


Just wondering Is this a real github token here :) ?

Nope. I just randomly typed one that look real.

A real token has 40 characters, this one has 24.

amitmiran137

LGTM

amitmiran137 · 2021-01-29T12:37:47Z

.github/workflows/cancel_duplicates.yml

-    workflows: ["CI"]
-    types: ["requested"]
+    workflows:
+      - "Miscellaneous"


so once merge and will work only with Miscellaneous workflow then we'll add all workflows here right?

Only this workflow will trigger the cancel job, but a scheduled cancel job will try to cancel duplicates of all workflows.

amitmiran137 · 2021-01-29T21:31:39Z

I just tested this
works amazing!

junlincc · 2021-01-29T22:56:54Z

many thanks for improving the CI process!

cc @guhan

pull-request-size bot added the size/XS label Jan 20, 2021

ktmud force-pushed the ci-cancel-duplicates branch from 816db71 to 67861ae Compare January 20, 2021 23:59

pull-request-size bot added size/L and removed size/XS labels Jan 21, 2021

ktmud force-pushed the ci-cancel-duplicates branch 3 times, most recently from 67bdfdb to 4ad214a Compare January 21, 2021 20:26

ktmud changed the title ~~fix(CI): cancel dupliates should not skip pull requests~~ fix(CI): properly configure cancel duplicates Jan 21, 2021

ktmud force-pushed the ci-cancel-duplicates branch 3 times, most recently from 0cf38da to bf893a5 Compare January 22, 2021 08:14

junlincc requested review from robdiciuccio and villebro January 22, 2021 16:35

villebro approved these changes Jan 23, 2021

View reviewed changes

ktmud force-pushed the ci-cancel-duplicates branch from 2a4e5c7 to d918bdc Compare January 24, 2021 07:19

pull-request-size bot added size/M and removed size/L labels Jan 24, 2021

ktmud force-pushed the ci-cancel-duplicates branch from d918bdc to c167c98 Compare January 24, 2021 07:36

potiuk reviewed Jan 24, 2021

View reviewed changes

.github/workflows/cancel_duplicates.yml Outdated Show resolved Hide resolved

.github/workflows/cancel_duplicates.yml Outdated Show resolved Hide resolved

.github/workflows/cancel_duplicates.yml Outdated Show resolved Hide resolved

potiuk reviewed Jan 24, 2021

View reviewed changes

.gitmodules Show resolved Hide resolved

ktmud force-pushed the ci-cancel-duplicates branch 2 times, most recently from 6144c6c to f5218b4 Compare January 24, 2021 08:13

potiuk reviewed Jan 24, 2021

View reviewed changes

ktmud force-pushed the ci-cancel-duplicates branch from f5218b4 to dfc623d Compare January 25, 2021 21:05

ktmud force-pushed the ci-cancel-duplicates branch 3 times, most recently from 6ab5f6f to 29e3713 Compare January 26, 2021 19:27

junlincc added the hold! On hold label Jan 27, 2021

ktmud force-pushed the ci-cancel-duplicates branch from 597b020 to adf6d70 Compare January 27, 2021 06:30

amitmiran137 approved these changes Jan 28, 2021

View reviewed changes

potiuk reviewed Jan 28, 2021

View reviewed changes

ktmud added 4 commits January 28, 2021 00:38

fix(CI): properly configure cancel duplicates action

70d6b6e

Use local checkout of cancel-workflow-runs; revert scheduled

7c26175

Try add sourceRunId

75d0218

Allow local cancel github script to cancel all branches and PRs

67fbd82

ktmud force-pushed the ci-cancel-duplicates branch from adf6d70 to 98f43c2 Compare January 28, 2021 10:43

pull-request-size bot added size/L and removed size/M labels Jan 28, 2021

Use python script to cancel

d6d02c8

ktmud force-pushed the ci-cancel-duplicates branch from 98f43c2 to d6d02c8 Compare January 28, 2021 10:50

ktmud added 2 commits January 28, 2021 01:00

Uniqueness by event and head branch

6a0d7ec

Add branch name to print logs

f8dc66f

Add some comments

197e876

potiuk reviewed Jan 28, 2021

View reviewed changes

amitmiran137 approved these changes Jan 29, 2021

View reviewed changes

ktmud merged commit 9c5ec3d into apache:master Jan 29, 2021

ktmud mentioned this pull request Feb 5, 2021

chore: remove example token to avoid confusion #12967

Merged

6 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(CI): properly configure cancel duplicates #12625

fix(CI): properly configure cancel duplicates #12625

ktmud commented Jan 20, 2021 •

edited

Loading

codecov-io commented Jan 20, 2021 •

edited

Loading

villebro left a comment

ktmud commented Jan 24, 2021

potiuk left a comment

potiuk commented Jan 24, 2021

potiuk Jan 24, 2021

potiuk Jan 24, 2021

potiuk commented Jan 24, 2021 •

edited

Loading

ktmud commented Jan 25, 2021

mistercrunch commented Jan 26, 2021

ktmud commented Jan 26, 2021

amitmiran137 left a comment

potiuk Jan 28, 2021

potiuk Jan 28, 2021

ktmud commented Jan 28, 2021

ktmud commented Jan 28, 2021

potiuk Jan 28, 2021

ktmud Jan 28, 2021 •

edited

Loading

ktmud Jan 28, 2021

amitmiran137 left a comment

amitmiran137 Jan 29, 2021

ktmud Jan 29, 2021

amitmiran137 commented Jan 29, 2021

junlincc commented Jan 29, 2021

		@@ -23,13 +23,13 @@
		export GITHUB_TOKEN=394ba3b48494ab8f930fbc93

fix(CI): properly configure cancel duplicates #12625

fix(CI): properly configure cancel duplicates #12625

Conversation

ktmud commented Jan 20, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov-io commented Jan 20, 2021 • edited Loading

Codecov Report

villebro left a comment

Choose a reason for hiding this comment

ktmud commented Jan 24, 2021

potiuk left a comment

Choose a reason for hiding this comment

potiuk commented Jan 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potiuk commented Jan 24, 2021 • edited Loading

ktmud commented Jan 25, 2021

mistercrunch commented Jan 26, 2021

ktmud commented Jan 26, 2021

amitmiran137 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud commented Jan 28, 2021

ktmud commented Jan 28, 2021

Choose a reason for hiding this comment

ktmud Jan 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitmiran137 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitmiran137 commented Jan 29, 2021

junlincc commented Jan 29, 2021

ktmud commented Jan 20, 2021 •

edited

Loading

codecov-io commented Jan 20, 2021 •

edited

Loading

potiuk commented Jan 24, 2021 •

edited

Loading

ktmud Jan 28, 2021 •

edited

Loading