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 2021/03/11 11:25:03 UTC

[GitHub] [airflow] ashb opened a new pull request #14718: Don't use author_association for self-hosted vs public runner decision.

ashb opened a new pull request #14718:
URL: https://github.com/apache/airflow/pull/14718


   Using this has two draw-backs for us.
   
   1. MEMBER applies to _anyone in the org_, not just members/commiters to this repo
   2. The value of this setting depends upon the user's "visiblity" in the org. I.e. if they hide their membership of the org, the author_association will show up as "CONTRIBUTOR" instead.
   
   Both of these combined mean we should instead use an alternative list.  Airflow committers is a scret that contains a list of GH user ids, such as `["ashb", "..."]` etc.


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796667526


   Hmmm.
   
   
   ```
   The workflow is not valid. .github/workflows/ci.yml (Line: 76, Col: 14): Unrecognized named-value: 'secrets'. Located at position 94 within expression: ( ( github.event_name == 'push' || github.event_name == 'schedule' || contains(secrets.AIRFLOW_COMMITERS, github.actor) ) && github.repository == 'apache/airflow' ) && 'self-hosted' || 'ubuntu-20.04'
   ```


----------------------------------------------------------------
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 #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#discussion_r592666795



##########
File path: .github/workflows/ci.yml
##########
@@ -26,7 +26,7 @@ on:  # yamllint disable-line rule:truthy
     branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
 
 env:
-
+  AIRFLOW_COMMITERS: ${{ secrets.AIRFLOW_COMMITERS }}

Review comment:
       I guess we can remove this one? That turned out to be not working for forked PRs I believe?




----------------------------------------------------------------
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] ashb merged pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb merged pull request #14718:
URL: https://github.com/apache/airflow/pull/14718


   


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796702553


   There, working, and not too horrible/duplictative. I would have liked to be able to use secrets, but sadly not possible.


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796703388


   Jobs after the first are running on self-hosted runners, don't need to wait on tests.


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796677470


   `contains(fromJSON('["ashb"]'), github.actor)` works, but is... ugly.


----------------------------------------------------------------
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 #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#discussion_r592666795



##########
File path: .github/workflows/ci.yml
##########
@@ -26,7 +26,7 @@ on:  # yamllint disable-line rule:truthy
     branches: ['master', 'v1-10-test', 'v1-10-stable', 'v2-0-test']
 
 env:
-
+  AIRFLOW_COMMITERS: ${{ secrets.AIRFLOW_COMMITERS }}

Review comment:
       I guess we can remove this one? That turned out to be not working for PRs I believe?




----------------------------------------------------------------
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] ashb commented on a change in pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#discussion_r592319392



##########
File path: .github/workflows/ci.yml
##########
@@ -127,10 +159,23 @@ jobs:
       pullRequestLabels: ${{ steps.source-run-info.outputs.pullRequestLabels }}
       runsOn: ${{ steps.set-runs-on.outputs.runsOn }}
     steps:
-      # Avoid having to specify the runs-on logic every time
+      # Avoid having to specify the runs-on logic every time. We use the custom
+      # env var AIRFLOW_SELF_HOSTED_RUNNER set only on our runners, but never
+      # on the public runners
       - name: Set runs-on
         id: set-runs-on
-        run: echo "::set-output name=runsOn::$(jq -n 'env.RUNS_ON')"
+        run: |
+          set -x
+          echo "$AIRFLOW_SELF_HOSTED_RUNNER"
+          echo "$GITHUB_EVENT_NAME"
+          echo "::set-output name=runsOn::$(jq -n '
+            if env.AIRFLOW_SELF_HOSTED_RUNNER and (["push", "schedule"] | index(env.GITHUB_EVENT_NAME)) then

Review comment:
       Whops. `or` not `and` here!




----------------------------------------------------------------
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 #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796678193


   [The Workflow run](https://github.com/apache/airflow/actions/runs/642645504) is cancelling this PR. Building image for the PR has been cancelled


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796670128


   Damn, It seems you can't reference secrets _nor_ env in the `runs-on` step :/ Hmmm what to do.


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796697918


   Down to at least one hard-coded list now.


----------------------------------------------------------------
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] ashb commented on pull request #14718: Don't use author_association for self-hosted vs public runner decision.

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14718:
URL: https://github.com/apache/airflow/pull/14718#issuecomment-796676071


   Trying to include a hard-coded list in the ci.yaml (it's verifired/checked again on the runner so is "safe") fails too:
   
   > Unexpected symbol: '['.
   
   Damn, not sure this is actually possible :/


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