Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Oct 4, 2020

The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: #10471


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: apache#10471
@potiuk
Copy link
Member Author

potiuk commented Oct 4, 2020

On top of fixing the dreadful #10471 problem with merge PRs, this one adds useful, informative messages for contriibutors - directly in the PR comments.

You can see several examples of the PRs run on my private fork:

https://github.com/potiuk/airflow/pulls

Just a few screenshots:

This is a comment that will be automatically posted when PR starts. It supplements missing feature of GitHub to provide a link to 'workflow_run' triggered event: this way the PR creators will get the link to the "Build Image" workflow for their PR - directly as comment in the PR:

Screenshot 2020-10-04 at 22 07 46

When they will get the image build failure, they will get this comment in their PR (they will be able to click trough to get:

Screenshot 2020-10-04 at 22 11 25

Also, if there is a reason for cancelling a build (duplicate runs, failing jobs) the users will get a message as comment in their PRs, explaining why their PR builds got cancelled.

@potiuk potiuk changed the title Improve running and canceliling of the PR-triggered builds. Improve running and cancelling of the PR-triggered builds. Oct 4, 2020
@dimberman
Copy link
Contributor

LGTM!

@potiuk potiuk merged commit a4478f5 into apache:master Oct 4, 2020
@potiuk potiuk deleted the add-comments-in-build-workflow branch October 4, 2020 20:53
potiuk added a commit that referenced this pull request Oct 6, 2020
The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: #10471
(cherry picked from commit a4478f5)
RaviTezu pushed a commit to RaviTezu/airflow that referenced this pull request Oct 25, 2020
…1268)

The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: apache#10471
(cherry picked from commit a4478f5)
kaxil pushed a commit that referenced this pull request Nov 12, 2020
The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: #10471
(cherry picked from commit a4478f5)
@potiuk potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 14, 2020
potiuk added a commit that referenced this pull request Nov 16, 2020
The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: #10471
(cherry picked from commit a4478f5)
kaxil pushed a commit that referenced this pull request Nov 18, 2020
The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: #10471
(cherry picked from commit a4478f5)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
…1268)

The PR builds are now better handled with regards to both
running (using merge-request) and canceling (with cancel notifications).

First of all we are using merged commit from the PR, not the original commit
from the PR.

Secondly - the workflow run notifies the original PR with comment
stating that the image is being built in a separate workflow -
including the link to that workflow.

Thirdly - when canceling duplicate PRs or PRs with failed
jobs, the workflow will add a comment to the PR stating the
reason why the PR is being cancelled.

Last but not least, we also add cancel job for the CodeQL duplicate
messages. They run for ~ 12 miinutes so it makes perfect sense to
also cancel those CodeQL jobs for which someone pushed fixups in a
quick succession.

Fixes: apache#10471
(cherry picked from commit a4478f5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Current CI builds slightly different sources than the one in PR build

2 participants