You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/12/05 20:42:33 UTC
[GitHub] [airflow] XD-DENG opened a new pull request #12845: Only trigger action 'Label when reviewed' for specific event type
XD-DENG opened a new pull request #12845:
URL: https://github.com/apache/airflow/pull/12845
We notice sometimes the Action job `"Label PRs when reviewed"` is triggered for multiple times, even if there is only one approval https://github.com/apache/airflow/pull/12816#issuecomment-739023107
According to https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request_review
there are three types, '_submitted_', '_edited_', and '_dismissed_', for `pull_request_review`.
We should only trigger when the type is '_submitted_'.
I don't think this can fully resolve the issue we found above, but should be done anyway before we make any further refactoring.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG closed pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
XD-DENG closed pull request #12845:
URL: https://github.com/apache/airflow/pull/12845
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12845:
URL: https://github.com/apache/airflow/pull/12845#discussion_r553276014
##########
File path: .github/workflows/label_when_reviewed.yml
##########
@@ -17,7 +17,9 @@
#
---
name: Label when reviewed
-on: pull_request_review # yamllint disable-line rule:truthy
+on: # yamllint disable-line rule:truthy
+ pull_request_review:
+ types: [submitted]
Review comment:
Why do we want to remove `dismissed` or `edited` ?
For sure at least dismissed for sure can change the status of the review, so it should be there (for example if the request was not approved by one reviewer and approved by another, dismissing the 'non-approved' status will change overall status of the PR to "approved". So we definitely need this one.
I am not sure about `edited`, but I believe this is the same (documentation is quite vague about it). you can change the state of your review from "request changes" to "approved" and I believe this is the trigger for the event.
Note that this is not "pull_request_review_comment" type of event. This is a totally different event and we have no workflow for it https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request_review_comment
From my observations 'pull_request_review' even is NOT triggered every time you make a comment, if this is what you are trying to prevent. But I might be wrong regarding the 'edit' behaviour.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12845:
URL: https://github.com/apache/airflow/pull/12845#issuecomment-739484372
[The Workflow run](https://github.com/apache/airflow/actions/runs/403825129) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12845:
URL: https://github.com/apache/airflow/pull/12845#discussion_r553276014
##########
File path: .github/workflows/label_when_reviewed.yml
##########
@@ -17,7 +17,9 @@
#
---
name: Label when reviewed
-on: pull_request_review # yamllint disable-line rule:truthy
+on: # yamllint disable-line rule:truthy
+ pull_request_review:
+ types: [submitted]
Review comment:
Why do we want to remove `dismissed` or `edited` ?
For sure at least dismissed can change the status of the review, so it should be there (for example if the request was not approved by one reviewer and approved by another, dismissing the 'non-approved' status will change overall status of the PR to "approved". So we definitely need this one.
I am not sure about `edited`, but I believe this is the same (documentation is quite vague about it). you can change the state of your review from "request changes" to "approved" and I believe this is the trigger for the event.
Note that this is not "pull_request_review_comment" type of event. This is a totally different event and we have no workflow for it https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request_review_comment
From my observations 'pull_request_review' event is NOT triggered every time you make a comment, if this is what you are trying to prevent. But I might be wrong regarding the 'edit' behaviour.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12845:
URL: https://github.com/apache/airflow/pull/12845#discussion_r553276014
##########
File path: .github/workflows/label_when_reviewed.yml
##########
@@ -17,7 +17,9 @@
#
---
name: Label when reviewed
-on: pull_request_review # yamllint disable-line rule:truthy
+on: # yamllint disable-line rule:truthy
+ pull_request_review:
+ types: [submitted]
Review comment:
Why do we want to remove `dismissed` or `edited` ?
For sure at least dismissed can change the status of the review, so it should be there (for example if the request was not approved by one reviewer and approved by another, dismissing the 'non-approved' status will change overall status of the PR to "approved". So we definitely need this one.
I am not sure about `edited`, but I believe this is the same (documentation is quite vague about it). you can change the state of your review from "request changes" to "approved" and I believe this is the trigger for the event.
Note that this is not "pull_request_review_comment" type of event. This is a totally different event and we have no workflow for it https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request_review_comment
From my observations 'pull_request_review' even is NOT triggered every time you make a comment, if this is what you are trying to prevent. But I might be wrong regarding the 'edit' behaviour.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG commented on pull request #12845: Only trigger action 'Label when reviewed' for specific event type
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on pull request #12845:
URL: https://github.com/apache/airflow/pull/12845#issuecomment-756317834
This was a trial to address the very weird behaviour of the pipeline, as described in https://github.com/apache/airflow/pull/12816#issuecomment-739023107 .
I'm absolutely fine to close this PR, but let's not ignore the issue itself and keep looking for some fixes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org