tvalentyn commented on a change in pull request #12452:
URL: https://github.com/apache/beam/pull/12452#discussion_r465209691
##########
File path: sdks/python/apache_beam/io/parquetio_test.py
##########
@@ -296,8 +296,10 @@ def test_sink_transform_int96(self):
path, self.SCHEMA96, num_shards=1, shard_name_template='')
def test_sink_transform(self):
- with tempfile.NamedTemporaryFile() as dst:
- path = dst.name
+ fd, path = tempfile.mkstemp()
Review comment:
What is the context of these changes?
(note: it's would be better to have them in a separate commit and tag a JIRA
in that commit).
Will https://github.com/apache/beam/pull/12150#discussion_r459807591 work?
##########
File path: sdks/python/apache_beam/runners/worker/log_handler_test.py
##########
@@ -87,7 +87,8 @@ def _verify_fn_log_handler(self, num_log_entries):
self.assertEqual(
'%s: %s' % (msg, num_received_log_entries), log_entry.message)
self.assertTrue(
- re.match(r'.*/log_handler_test.py:\d+', log_entry.log_location),
+ re.match(
+ r'.*(/|\\)log_handler_test.py:\d+', log_entry.log_location),
Review comment:
use `os.sep`
##########
File path: .github/workflows/python_tests.yml
##########
@@ -0,0 +1,196 @@
+# 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: Python tests
+
+on:
+ schedule:
+ - cron: '10 2 * * *'
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['sdks/python/**', 'model/**']
+
+
+env:
+ GCP_WHEELS_STAGING_PATH: "gs://${{ secrets.GCP_BUCKET
}}/${GITHUB_REF##*/}/${GITHUB_SHA}-${GITHUB_RUN_ID}/"
Review comment:
Is this used anywhere?
##########
File path: .github/workflows/python_tests.yml
##########
@@ -0,0 +1,196 @@
+# 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: Python tests
+
+on:
+ schedule:
+ - cron: '10 2 * * *'
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['sdks/python/**', 'model/**']
+
+
+env:
+ GCP_WHEELS_STAGING_PATH: "gs://${{ secrets.GCP_BUCKET
}}/${GITHUB_REF##*/}/${GITHUB_SHA}-${GITHUB_RUN_ID}/"
+ GCP_BUCKET: ${{ secrets.GCP_BUCKET }} # beam-wheels-staging
+ GCP_PROJECT_ID: ${{ secrets.GCP_PROJECT_ID }} # apache-beam-testing
+ GCP_REGION: ${{ secrets.GCP_REGION}} # us-central
+
+jobs:
+
+ build_python_sdk_source:
+ name: 'Build Python SDK Source'
+ if: github.repository_owner == 'apache' && (github.event_name == 'push' ||
github.event_name == 'schedule')
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout code
+ uses: actions/checkout@v2
+ - name: Install python
+ uses: actions/setup-python@v2
+ with:
+ python-version: 3.7
+ - name: Get build dependencies
+ working-directory: ./sdks/python
+ run: pip install -r build-requirements.txt
+ - name: Install wheels
+ run: pip install wheel
+ - name: Build source
+ working-directory: ./sdks/python
+ run: python setup.py sdist --formats=zip
+ - name: Add checksums
+ working-directory: ./sdks/python/dist
+ run: |
+ file=$(ls | grep .zip | head -n 1)
+ sha512sum $file > ${file}.sha512
+ - name: Unzip source
+ working-directory: ./sdks/python
+ run: unzip dist/$(ls dist | grep .zip | head -n 1)
+ - name: Rename source directory
+ working-directory: ./sdks/python
+ run: mv $(ls | grep apache-beam) apache-beam-source
+ - name: Upload compressed sources as artifacts
+ uses: actions/upload-artifact@v2
+ with:
+ name: source_zip
+ path: sdks/python/dist
+
+ python_unit_tests:
+ name: 'Python Unit Tests'
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ os: [ubuntu-latest, macos-latest, windows-latest]
+ params: [
+ {"py_ver": "3.5", "tox_env": "py35"},
+ {"py_ver": "3.6", "tox_env": "py36"},
+ {"py_ver": "3.7", "tox_env": "py37"},
+ {"py_ver": "3.8", "tox_env": "py38"},
+ ]
+ exclude:
+ # TODO remove exclusion after issue with protobuf is solved
+ # https://github.com/protocolbuffers/protobuf/issues/7765
+ - os: windows-latest
+ params: {"py_ver": "3.8", "tox_env": "py38"}
+ steps:
+ - name: Checkout code
+ uses: actions/checkout@v2
+ - name: Install python
+ uses: actions/setup-python@v2
+ with:
+ python-version: ${{ matrix.params.py_ver }}
+ - name: Get build dependencies
+ working-directory: ./sdks/python
+ run: pip install -r build-requirements.txt
+ - name: Install tox
+ run: pip install tox
+ - name: Run tests basic unix
+ if: startsWith(matrix.os, 'ubuntu') || startsWith(matrix.os, 'macos')
+ working-directory: ./sdks/python
+ run: tox -c tox.ini -e ${{ matrix.params.tox_env }}
+ - name: Run tests basic windows
+ if: startsWith(matrix.os, 'windows')
+ working-directory: ./sdks/python
+ run: tox -c tox.ini -e ${{ matrix.params.tox_env }}-win
+
+ python_wordcount_direct_runner:
+ name: 'Python Wordcount Direct Runner'
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ os: [ubuntu-latest, macos-latest, windows-latest]
+ python: [3.5, 3.6, 3.7, 3.8]
+ exclude:
+ # TODO remove exclusion after issue with protobuf is solved
+ # https://github.com/protocolbuffers/protobuf/issues/7765
+ - os: windows-latest
+ python: 3.8
+ steps:
Review comment:
Do actions allow to extract a common subroutine and call it from other
actions? For example following steps seem to be repeated several times, and
perhaps could be defined in one place:
```
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Install python
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python }}
- name: Get build dependencies
working-directory: ./sdks/python
run: pip install -r build-requirements.txt
- name: Install requirements
working-directory: ./sdks/python
run: pip install setuptools --upgrade && pip install -e .
```
##########
File path: sdks/python/apache_beam/typehints/typecheck_test_py3.py
##########
@@ -92,23 +93,29 @@ def test_wrapper_pass_through(self):
# We use a file to check the result because the MyDoFn instance passed is
# not the same one that actually runs in the pipeline (it is serialized
# here and deserialized in the worker).
- with tempfile.NamedTemporaryFile(mode='w+t') as f:
- dofn = MyDoFn(f.name)
+
+ fd, path = tempfile.mkstemp()
+ try:
+ os.close(fd)
Review comment:
Same comment as above - this approach is not intuitive.
##########
File path: .github/workflows/python_tests.yml
##########
@@ -0,0 +1,196 @@
+# 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: Python tests
+
+on:
+ schedule:
+ - cron: '10 2 * * *'
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['sdks/python/**', 'model/**']
+
+
+env:
+ GCP_WHEELS_STAGING_PATH: "gs://${{ secrets.GCP_BUCKET
}}/${GITHUB_REF##*/}/${GITHUB_SHA}-${GITHUB_RUN_ID}/"
+ GCP_BUCKET: ${{ secrets.GCP_BUCKET }} # beam-wheels-staging
+ GCP_PROJECT_ID: ${{ secrets.GCP_PROJECT_ID }} # apache-beam-testing
+ GCP_REGION: ${{ secrets.GCP_REGION}} # us-central
+
+jobs:
+
+ build_python_sdk_source:
+ name: 'Build Python SDK Source'
+ if: github.repository_owner == 'apache' && (github.event_name == 'push' ||
github.event_name == 'schedule')
Review comment:
Also, all actions workflow run in parallel, right? so it takes roughly
the same time as running a regular precommit? In such case we could consider
running it on PR.
##########
File path: sdks/python/apache_beam/testing/datatype_inference_test.py
##########
@@ -174,6 +175,7 @@ def test_infer_typehints_schema(self, _, data, schema):
@parameterized.expand([(d["name"], d["data"], d["pyarrow_schema"])
for d in TEST_DATA])
@unittest.skipIf(pa is None, "PyArrow is not installed")
+ @unittest.skipIf(sys.platform == "win32", "[BEAM-10624]")
Review comment:
Pyarrow upgrade can be decoupled from dropping py2, unless we need code
changes in Beam:
https://github.com/apache/beam/blob/5ded4aedb651da0c2d0778bfac45f0a7abb3e959/sdks/python/setup.py#L157
##########
File path: sdks/python/apache_beam/runners/portability/portable_runner_test.py
##########
@@ -284,6 +284,7 @@ def create_options(self):
return options
[email protected](sys.platform == "win32", reason="[BEAM-10625]")
Review comment:
Took a look and replied on the bug. It may be an issue in GRPC or the
networking configuration of the Windows host that Github uses. I do not see
this error in the test suite that we run internally (on Py2.7).
@TobKed do you have a recommendation how Beam Devs could recreate the same
environment as used by GitHub actions for the purposes of debugging a test or
run a single test via actions? It would be good to document this in our Actions
users guide.
The goal is to minimize the feedback loop as much as possible.
Feel free to reply on https://issues.apache.org/jira/browse/BEAM-10625 to
keep this conversation in one place.
----------------------------------------------------------------
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:
[email protected]
