-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-11569] Enabling wheels to be built using Beam-based submodules workflow #13736
Conversation
@@ -256,7 +256,7 @@ jobs: | |||
echo "Tagging ${BRANCH_NAME}" | |||
git tag -f nightly-${BRANCH_NAME} HEAD | |||
- name: Push tags | |||
uses: apache/airflow-github-push-action@b007e7b818e33b04afd056e4c4b57ba917145d7a | |||
uses: ./.github/actions/github-push-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to checkout the submodules on the checkout action just above this. Look at the Airflow ci.yml workflow.
(Sorry, on phone else I'd be more precise)
thanks! I believe the latest commit includes the necessary changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we test these actions before merging?
@@ -30,7 +30,12 @@ jobs: | |||
name: "Cancel duplicate workflow runs" | |||
runs-on: ubuntu-latest | |||
steps: | |||
- uses: apache/airflow-cancel-workflow-runs@953e057dc81d3458935a18d1184c386b0f6b5738 | |||
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to checkout before cancelling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per this post, we need to get the code for the action (thus checking out with submodules) so that it can be run.
@@ -30,7 +30,12 @@ jobs: | |||
name: "Cancel duplicate workflow runs" | |||
runs-on: ubuntu-latest | |||
steps: | |||
- uses: apache/airflow-cancel-workflow-runs@953e057dc81d3458935a18d1184c386b0f6b5738 | |||
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" | |||
uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In build_wheels.yml we use checkout@master, why use checkout@v2 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. checkout@v2 is better - GitHub might introduce breaking changes in v3, so it's wise to change it in build_wheels.yml as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah thanks. Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
My change is the fix and improvement for github action which labels approved PRs (introduced in this [PR](#6239)). It is inspired by solution introduced and tested in [Apache Airflow](https://github.com/apache/airflow) (thanks @potiuk @ashb 🚀 ) Corresponding Apache Airflow workflows on which I based this PR: - https://github.com/apache/airflow/blob/main/.github/workflows/label_when_reviewed.yml - https://github.com/apache/airflow/blob/main/.github/workflows/label_when_reviewed_workflow_run.yml Problems which were solved in this PR: - **Permissions**. @morningman opened a related bug: [[Help] Error: Resource not accessible by integration](TobKed/label-when-approved-action#7). It is related to limited permissions of workflows being triggered by `pull_request_review` (`GITHUB_TOKEN` has read-only permissions). More information about it you can find in the article: [Keeping your GitHub Actions and workflows secure: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/). TL;DR: On pull request review event (`on: pull_request_review` ) "dummy" workflow `Label when reviewed` triggers another workflow `Label when approved workflow run` which has sufficient permissions (`on: workflow_run: workflows: ["Label when reviewed"]`). - **Safe use of 3rd-party Github Actions by using submodules pattern.** It is decribed in: https://cwiki.apache.org/confluence/display/BUILDS/GitHub+Actions+status > NEVER use 3rd-party actions directly in your workflows - use the "submodule" pattern. This pattern is successfully used by projects like: - [Apache Airflow](https://github.com/apache/airflow) ([PR](apache/airflow#13514)) - [Apache Beam](https://github.com/apache/beam) ([PR](apache/beam#13736)) - [Apache Superset](https://github.com/apache/superset) ([PR](apache/superset#12709))
Airflow repositories will be removed soon. Instead, this PR creates submodules for existing actions in the Beam repo, and uses them instead of relying on external repos.
r: @ibzib
cc: @potiuk @ashb
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.