XComp commented on code in PR #24426:
URL: https://github.com/apache/flink/pull/24426#discussion_r1609654509


##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job

Review Comment:
   Can move this logic into a custom action? 🤔 



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}

Review Comment:
   Where is `PYTHON_VERSION` defined? 🤔 



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare

Review Comment:
   Prepare seems to be too generic in this context. What about 
`prepare-python-wheel-upload`? But we could get rid of this one job if we move 
the stringification into a custom action anyway, I guess. 🤔 



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          bash dev/build-wheels.sh
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist

Review Comment:
   ```suggestion
           run: tar -czvf python-wheel-linux.tar.gz flink-python/dist
   ```
   We should try to avoid having ambigious names. Here it might be not really 
necessary because we're updating the name in the `upload-artifact` step anyway. 
But it might help certain readers with understanding the code.



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          bash dev/build-wheels.sh
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist
+      - name: Upload artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: wheel_${{ needs.prepare.outputs.stringified-workflow-name 
}}-${{ github.run_number }}
+          path: python-wheel.tar.gz
+  macos:
+    needs: prepare
+    runs-on: macos-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          python -m pip install --upgrade pip
+          pip install cibuildwheel==2.8.0
+          cibuildwheel --platform macos --output-dir dist .
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist
+      - name: Upload artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: wheel_${{ needs.prepare.outputs.stringified-workflow-name 
}}-${{ github.run_number }}

Review Comment:
   See my statement above.



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          bash dev/build-wheels.sh
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist
+      - name: Upload artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: wheel_${{ needs.prepare.outputs.stringified-workflow-name 
}}-${{ github.run_number }}

Review Comment:
   Shouldn't we add the `linux` phrase here somewhere? Otherwise, we would have 
the same build artifact name twice (for macos and linux). Or am I missing 
something?



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository

Review Comment:
   nit: Quoting the names enables better YAML syntax highlighting in the IDE 
and improves the readability of the code



##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,65 @@ jobs:
       s3_bucket: ${{ secrets.IT_CASE_S3_BUCKET }}
       s3_access_key: ${{ secrets.IT_CASE_S3_ACCESS_KEY }}
       s3_secret_key: ${{ secrets.IT_CASE_S3_SECRET_KEY }}
+  prepare:
+    name: Prepare
+    runs-on: ubuntu-latest
+    outputs:
+      stringified-workflow-name: ${{ 
steps.workflow-prep-step.outputs.stringified-workflow-name }}
+    steps:
+      - name: "Stringify workflow name"
+        id: workflow-prep-step
+        run: |
+          # Similar to the template.flink-ci.yml compile job
+          stringified_workflow_name=$(echo "${{ github.workflow }}" | tr -C 
'[:alnum:]._' '-' |  tr '[:upper:]' '[:lower:]' | sed -e 's/--*/-/g' -e 
's/^-*//g' -e 's/-*$//g')
+          echo "stringified-workflow-name=${stringified_workflow_name}" >> 
$GITHUB_OUTPUT
+  linux:
+    needs: prepare
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          bash dev/build-wheels.sh
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist
+      - name: Upload artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: wheel_${{ needs.prepare.outputs.stringified-workflow-name 
}}-${{ github.run_number }}
+          path: python-wheel.tar.gz
+  macos:
+    needs: prepare
+    runs-on: macos-latest
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+      - name: Build wheels
+        run: |
+          cd flink-python
+          python -m pip install --upgrade pip
+          pip install cibuildwheel==2.8.0
+          cibuildwheel --platform macos --output-dir dist .
+      - name: Tar artifact
+        run: tar -czvf python-wheel.tar.gz flink-python/dist

Review Comment:
   See my statement above.



-- 
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: issues-unsubscr...@flink.apache.org

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

Reply via email to