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