For v1-10-test PRs and pushes, use target branch scripts for images #12339

potiuk · 2020-11-13T12:36:10Z

Previously, always master scripts were used to build images
for workflow_run, because workflow_run always runs from master
branch. However that causes some surprising effects becuase the
sripts from master had to support both master and 1.10.

This change utilises a new feature in the "get-workflow-origin"
action - to get the target branch of PR and uses ci scripts from that
target branch.

This is perfectly secure, because both v1-10-test, v1-10-stable
and future 2-0 branches can only be updated by committers,
either by direct push or by merge.

^ 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.

github-actions · 2020-11-13T12:39:42Z

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

potiuk · 2020-11-13T12:40:42Z

Testing it here:

Direct push to master: https://github.com/potiuk/airflow/actions/runs/361589802

Setting output: sourceHeadRepo: potiuk/airflow
Setting output: sourceHeadBranch: master
Setting output: sourceHeadSha: 040222baa32639b74451dce9c2917bd3483ddb46
Setting output: sourceEvent: push
Setting output: pullRequestNumber: 
Setting output: pullRequestLabels: []
Setting output: mergeCommitSha: 
Setting output: targetCommitSha: 040222baa32639b74451dce9c2917bd3483ddb46
Setting output: targetBranch: master

PR: from a fork to master: https://github.com/potiuk/airflow/actions/runs/361596665 higrys -> potiuk

Setting output: sourceHeadRepo: higrys/airflow
Setting output: sourceHeadBranch: test-8
Setting output: sourceHeadSha: 93a92091e64abb1068608816a4fb5b3e7474149c
Setting output: sourceEvent: pull_request
Setting output: pullRequestNumber: 150
Setting output: pullRequestLabels: []
Setting output: mergeCommitSha: 318fbafce42cb2d9b44b07ec495b8f545a4a87bc
Setting output: targetCommitSha: 318fbafce42cb2d9b44b07ec495b8f545a4a87bc
Setting output: targetBranch: master

Direct push to v1-10-test (stable will work the same): https://github.com/potiuk/airflow/actions/runs/361597050

Setting output: sourceHeadRepo: potiuk/airflow
Setting output: sourceHeadBranch: v1-10-test
Setting output: sourceHeadSha: 040222baa32639b74451dce9c2917bd3483ddb46
Setting output: sourceEvent: push
Setting output: pullRequestNumber: 
Setting output: pullRequestLabels: []
Setting output: mergeCommitSha: 
Setting output: targetCommitSha: 040222baa32639b74451dce9c2917bd3483ddb46
Setting output: targetBranch: v1-10-test

PR from a fork to v1-10-test (stable will work the same): https://github.com/potiuk/airflow/actions/runs/361597777 higrys -> potiuk

Setting output: sourceHeadRepo: higrys/airflow
Setting output: sourceHeadBranch: test-8-v1
Setting output: sourceHeadSha: 93a92091e64abb1068608816a4fb5b3e7474149c
Setting output: sourceEvent: pull_request
Setting output: pullRequestNumber: 151
Setting output: pullRequestLabels: []
Setting output: mergeCommitSha: b2242877cacb038a31c8c6a5d9d32f6df0ecaed9
Setting output: targetCommitSha: b2242877cacb038a31c8c6a5d9d32f6df0ecaed9
Setting output: targetBranch: v1-10-test

This should cause much more predictable builds for "non-master-branch" PRs and direct pushes (scripts from the target branch are used to build images in "workflow_run" Build Image runs ) .

potiuk · 2020-11-13T12:55:22Z

One small update @kaxil -> i noticed that the Job name overriding the scripts still mentioned master, now it will tell exactly from which branch the scripts are used.

potiuk · 2020-11-13T12:56:22Z

.github/workflows/build-images-workflow-run.yml

        if: steps.defaults.outputs.proceed == 'true'
      - name: "Setup python"
        uses: actions/setup-python@v2
        with:
          python-version: ${{ needs.build-info.outputs.defaultPythonVersion }}
        if: steps.defaults.outputs.proceed == 'true'
-      - name: "Override 'scripts/ci' with the ${{ github.ref }} version so that the PR cannot override it."
+      - name: |


Here we tell exactly which version of the scripts is used to build the images @kaxil

potiuk · 2020-11-13T13:07:16Z

Right:

potiuk · 2020-11-13T13:08:30Z

And checkout still has "master" (just in the job name) .

kaxil · 2020-11-13T13:09:47Z

And checkout still has "master" (just in the job name) .

eeh yeah, can we update that

potiuk · 2020-11-13T13:10:05Z

And checkout still has "master" (just in the job name) .

eeh yeah, can we update that

absolutely!

potiuk · 2020-11-13T13:15:37Z

Good Enough :)

Previously, always master scripts were used to build images for workflow_run, because workflow_run always runs from master branch. However that causes some surprising effects becuase the sripts from master had to support both master and 1.10. This change utilises a new feature in the "get-workflow-origin" action - to get the target branch of PR and uses ci scripts from that target branch. This is perfectly secure, because both v1-10-test, v1-10-stable and future 2-0 branches can only be updated by committers, either by direct push or by merge.

…12339) Previously, always master scripts were used to build images for workflow_run, because workflow_run always runs from master branch. However that causes some surprising effects becuase the sripts from master had to support both master and 1.10. This change utilises a new feature in the "get-workflow-origin" action - to get the target branch of PR and uses ci scripts from that target branch. This is perfectly secure, because both v1-10-test, v1-10-stable and future 2-0 branches can only be updated by committers, either by direct push or by merge. (cherry picked from commit 7c4fe19)

…pache#12339) Previously, always master scripts were used to build images for workflow_run, because workflow_run always runs from master branch. However that causes some surprising effects becuase the sripts from master had to support both master and 1.10. This change utilises a new feature in the "get-workflow-origin" action - to get the target branch of PR and uses ci scripts from that target branch. This is perfectly secure, because both v1-10-test, v1-10-stable and future 2-0 branches can only be updated by committers, either by direct push or by merge. (cherry picked from commit 7c4fe19)

boring-cyborg bot added the area:dev-tools label Nov 13, 2020

potiuk requested review from ashb, kaxil, mik-laj and turbaszek November 13, 2020 12:36

kaxil approved these changes Nov 13, 2020

View reviewed changes

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 13, 2020

potiuk force-pushed the use-v1-10-scripts-in-v1-10-pulls-push branch from 040222b to 229a951 Compare November 13, 2020 12:54

potiuk commented Nov 13, 2020

View reviewed changes

ashb approved these changes Nov 13, 2020

View reviewed changes

potiuk force-pushed the use-v1-10-scripts-in-v1-10-pulls-push branch from 229a951 to 86eefec Compare November 13, 2020 13:10

potiuk force-pushed the use-v1-10-scripts-in-v1-10-pulls-push branch from 86eefec to cd0d1b2 Compare November 13, 2020 13:18

potiuk mentioned this pull request Nov 13, 2020

StreamLogWriter: Provide (no-op) close method (backport to v1-10) #10885

Merged

potiuk merged commit 7c4fe19 into apache:master Nov 13, 2020

potiuk deleted the use-v1-10-scripts-in-v1-10-pulls-push branch November 13, 2020 13:28

potiuk added this to the Airflow 1.10.13 milestone Nov 14, 2020

potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For v1-10-test PRs and pushes, use target branch scripts for images #12339

For v1-10-test PRs and pushes, use target branch scripts for images #12339

potiuk commented Nov 13, 2020

github-actions bot commented Nov 13, 2020

potiuk commented Nov 13, 2020 •

edited

Loading

potiuk commented Nov 13, 2020 •

edited

Loading

potiuk Nov 13, 2020

kaxil Nov 13, 2020

potiuk commented Nov 13, 2020

potiuk commented Nov 13, 2020 •

edited

Loading

kaxil commented Nov 13, 2020

potiuk commented Nov 13, 2020

potiuk commented Nov 13, 2020

For v1-10-test PRs and pushes, use target branch scripts for images #12339

For v1-10-test PRs and pushes, use target branch scripts for images #12339

Conversation

potiuk commented Nov 13, 2020

github-actions bot commented Nov 13, 2020

potiuk commented Nov 13, 2020 • edited Loading

potiuk commented Nov 13, 2020 • edited Loading

potiuk Nov 13, 2020

Choose a reason for hiding this comment

kaxil Nov 13, 2020

Choose a reason for hiding this comment

potiuk commented Nov 13, 2020

potiuk commented Nov 13, 2020 • edited Loading

kaxil commented Nov 13, 2020

potiuk commented Nov 13, 2020

potiuk commented Nov 13, 2020

potiuk commented Nov 13, 2020 •

edited

Loading

potiuk commented Nov 13, 2020 •

edited

Loading

potiuk commented Nov 13, 2020 •

edited

Loading