XComp commented on code in PR #24426:
URL: https://github.com/apache/flink/pull/24426#discussion_r1611659779
##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,51 @@ 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 }}
+
+ build_python_wheels:
+ name: "Build Python Wheels on ${{ matrix.os }}"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - os: ubuntu-latest
+ os_name: linux
+ python-version: 3.9
+ - os: macos-latest
+ os_name: macos
+ python-version: 3.9
+ 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: ${{ matrix.python-version }}
+ - name: "Stringify workflow name"
+ uses: "./.github/actions/stringify"
+ id: stringify_workflow
+ with:
+ value: ${{ github.workflow }}
+ - name: "Build python wheels for ${{ matrix.os }}"
+ if: matrix.os_name == 'linux'
+ run: |
+ cd flink-python
+ bash dev/build-wheels.sh
+ - name: "Build python wheels for ${{ matrix.os }}"
+ if: matrix.os_name == 'macos'
+ run: |
+ cd flink-python
+ python -m pip install --upgrade pip
+ pip install cibuildwheel==2.8.0
Review Comment:
```suggestion
pip install cibuildwheel==2.16.5
```
Looks like there was an update on Azure in
[FLINK-34582](https://issues.apache.org/jira/browse/FLINK-34582) which we
should include. Maybe, put this in a separate commit and use
[FLINK-34582](https://issues.apache.org/jira/browse/FLINK-34582) as the issue
ID in the commit message. We should document this in
[FLINK-34582](https://issues.apache.org/jira/browse/FLINK-34582) after this PR
is merged.
##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,51 @@ 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 }}
+
+ build_python_wheels:
+ name: "Build Python Wheels on ${{ matrix.os }}"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - os: ubuntu-latest
+ os_name: linux
+ python-version: 3.9
+ - os: macos-latest
+ os_name: macos
+ python-version: 3.9
+ 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: ${{ matrix.python-version }}
+ - name: "Stringify workflow name"
+ uses: "./.github/actions/stringify"
+ id: stringify_workflow
+ with:
+ value: ${{ github.workflow }}
+ - name: "Build python wheels for ${{ matrix.os }}"
Review Comment:
```suggestion
- name: "Build python wheels for ${{ matrix.os_name }}"
```
I feel like using the `os_name` is less tied to GHA here. WDYT?
##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,51 @@ 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 }}
+
+ build_python_wheels:
+ name: "Build Python Wheels on ${{ matrix.os }}"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - os: ubuntu-latest
+ os_name: linux
+ python-version: 3.9
+ - os: macos-latest
+ os_name: macos
+ python-version: 3.9
Review Comment:
Looks like the python version also got changed in
[FLINK-34582](https://issues.apache.org/jira/browse/FLINK-34582) 🤔 We should
check why we don't upgrade the Python version for Linux.
##########
.github/workflows/wheels.yml:
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This workflow is meant as an extended CI run that includes certain features
that shall be tested
+# and JDK versions that are supported but not considered default.
+
+name: "Wheels Test"
+
+on:
+ push:
+ workflow_dispatch:
+
+permissions: read-all
+
+jobs:
+ build_python_wheels:
+ name: "Build Python Wheels on ${{ matrix.os }}"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - os: ubuntu-latest
+ os_name: linux
+ python-version: 3.9
+ - os: macos-latest
+ os_name: macos
+ python-version: 3.9
+ 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: ${{ matrix.python-version }}
+ - name: "Stringify workflow name"
+ uses: "./.github/actions/stringify"
+ id: stringify_workflow
+ with:
+ value: ${{ github.workflow }}
+ - name: "Build python wheels for ${{ matrix.os }}"
+ if: matrix.os_name == 'linux'
+ run: |
+ cd flink-python
+ bash dev/build-wheels.sh
+ - name: "Build python wheels for ${{ matrix.os }}"
+ if: matrix.os_name == 'macos'
+ run: |
+ cd flink-python
+ python -m pip install --upgrade pip
+ pip install cibuildwheel==2.8.0
Review Comment:
```suggestion
pip install cibuildwheel==2.16.5
```
Looks like there was an update on Azure in FLINK-34582 which we should
include. Maybe, put this in a separate commit and use FLINK-34582 as the issue
ID in the commit message. We should document this in FLINK-34582 after this PR
is merged.
##########
.github/actions/stringify/action.yml:
##########
@@ -0,0 +1,42 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+---
+name: "Stringify"
+description: "Stringifies the given input value."
+inputs:
+ value:
+ description: "Input value to stringify."
+ required: true
+outputs:
+ stringified_value:
+ description: "Stringified output value."
+ value: ${{ steps.stringify-step.outputs.stringified_value }}
+runs:
+ using: "composite"
+ steps:
+ - name: "Stringify workflow name"
Review Comment:
```suggestion
- name: "Stringify '${{ inputs.value }}'"
```
The workflow substring is obsolete. Does that work?
##########
.github/workflows/wheels.yml:
##########
@@ -0,0 +1,74 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This workflow is meant as an extended CI run that includes certain features
that shall be tested
+# and JDK versions that are supported but not considered default.
+
+name: "Wheels Test"
Review Comment:
It's better to comment out certain parts of the nightly workflow that are
not needed (test runs and packaging/licensing jobs) and run the actual nightly
workflow with the wheels creation. That way we actually test the code rather
than some other workflow that would be deleted in the end.
Commenting out of parts of the nightly workflow can happen in a separate
commit which can be easily dropped before merging the PR.
##########
.github/workflows/nightly.yml:
##########
@@ -94,3 +94,51 @@ 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 }}
+
+ build_python_wheels:
+ name: "Build Python Wheels on ${{ matrix.os }}"
Review Comment:
Can you double-check? It looks like we need to build `flink-dist` as part of
the wheels creation? That is then packaged into the wheels as far as I
understand? 🤔
--
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
