You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/08/14 09:12:39 UTC

[GitHub] [airflow] potiuk commented on a diff in pull request #33371: Migrate the CI workflows to the new runners on EKS cluster

potiuk commented on code in PR #33371:
URL: https://github.com/apache/airflow/pull/33371#discussion_r1293206681


##########
.github/workflows/ci.yml:
##########
@@ -115,6 +115,15 @@ jobs:
       helm-test-packages: ${{ steps.selective-checks.outputs.helm-test-packages }}
       debug-resources: ${{ steps.selective-checks.outputs.debug-resources }}
       runs-on: ${{ steps.selective-checks.outputs.runs-on }}
+      runs-on-small: >-
+        ${{ steps.selective-checks.outputs.runs-on == 'self-hosted'

Review Comment:
   I have bad experiences with conditions embedded using the expreession language. It's prone to errors. Would not it be better to generate `runs-on-small/medium/large` as direct output of selective checks? 
   
   That has the potential that we can also base it on labels later when we connect ARC - then we could for example apply an "ARC" label to PR to test it on the ARC setup. 
   
   This has the nice property that selective checks are also very well covered by unit tests so we can easily test any logic we will apply there.



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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org