Skip to content

Conversation

@mik-laj
Copy link
Member

@mik-laj mik-laj commented Jan 9, 2021

Description

This PR aims to reduce the GitHub Action usage by canceling the previous build. In most case, the last commit is meaningful.

We want to support fork as well, so we have to use workflow_run, so this change will only take effect after merging.

If you use forks, you should create a separate "Cancelling" workflow_run triggered workflow. The workflow_run should be responsible for all canceling actions. The examples below show the possible ways the action can be utilized.

https://github.com/potiuk/cancel-workflow-runs#usage

Note: This event will only trigger a workflow run if the workflow file is on the default branch.

https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#workflow_run

Similar PRs in other Apache projects;
Apache Spark: apache/spark#31104
Apache Pulsar: apache/pulsar#9159

INfRA Mailing lists discussion: https://lists.apache.org/thread.html/r5303eec41cc1dfc51c15dbe44770e37369330f9644ef09813f649120%40%3Cbuilds.apache.org%3E

@potiuk Can I ask for a review?

Upgrade Notes

N/A

Release Notes

N/A

Documentation

N/A

@codecov-io
Copy link

codecov-io commented Jan 9, 2021

Codecov Report

Merging #6429 (b1ad486) into master (1beaab5) will decrease coverage by 1.39%.
The diff coverage is 56.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6429      +/-   ##
==========================================
- Coverage   66.44%   65.05%   -1.40%     
==========================================
  Files        1075     1320     +245     
  Lines       54773    64451    +9678     
  Branches     8168     9397    +1229     
==========================================
+ Hits        36396    41931    +5535     
- Misses      15700    19525    +3825     
- Partials     2677     2995     +318     
Flag Coverage Δ
unittests 65.05% <56.80%> (?)

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

Impacted Files Coverage Δ
...e/pinot/broker/api/resources/PinotBrokerDebug.java 0.00% <0.00%> (-79.32%) ⬇️
...ot/broker/broker/AllowAllAccessControlFactory.java 71.42% <ø> (-28.58%) ⬇️
.../helix/BrokerUserDefinedMessageHandlerFactory.java 33.96% <0.00%> (-32.71%) ⬇️
...ker/routing/instanceselector/InstanceSelector.java 100.00% <ø> (ø)
...ava/org/apache/pinot/client/AbstractResultSet.java 66.66% <0.00%> (+9.52%) ⬆️
.../main/java/org/apache/pinot/client/Connection.java 35.55% <0.00%> (-13.29%) ⬇️
...inot/client/JsonAsyncHttpPinotClientTransport.java 10.90% <0.00%> (-51.10%) ⬇️
...not/common/assignment/InstancePartitionsUtils.java 73.80% <ø> (+0.63%) ⬆️
...common/config/tuner/NoOpTableTableConfigTuner.java 100.00% <ø> (ø)
...ot/common/config/tuner/RealTimeAutoIndexTuner.java 100.00% <ø> (ø)
... and 1155 more

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 a5c4ed2...b1ad486. Read the comment docs.

@mik-laj
Copy link
Member Author

mik-laj commented Jan 10, 2021

This action has been added to white-list by INFRA: https://issues.apache.org/jira/browse/INFRA-21287

@mik-laj
Copy link
Member Author

mik-laj commented Jan 10, 2021

Should I do anything else to make this change merged?

Copy link
Contributor

@xiangfu0 xiangfu0 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for submitting this change!

@xiangfu0 xiangfu0 merged commit 3e4c325 into apache:master Jan 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants