From 3e37e3efaad35800042ad5c99006041a3e4ec3ef Mon Sep 17 00:00:00 2001 From: Udi Meiri Date: Fri, 16 Mar 2018 10:06:43 -0700 Subject: [PATCH 1/3] Enable py3 lint and cleanup tox.ini. - Add BEAM_EXPERIMENTAL_PY3 environment variable. Currently only used to run the Tox py3-lint environment. - Rename environments to use factors, such that py27-lint and py3-lint use Python versions 2.7.x and 3.x.x respectively. - Remove some redundant package requirements such as nose from being specified in tox.ini if already present in setup.py. - Use shared environment settings in tox.ini, under "[testenv]". - Factor out *.{pyc,c,so} cleanup into a separate script. --- sdks/python/build.gradle | 6 +- sdks/python/run_tox_cleanup.sh | 30 +++++++++ sdks/python/setup.py | 11 ++-- sdks/python/tox.ini | 109 ++++++++++----------------------- 4 files changed, 72 insertions(+), 84 deletions(-) create mode 100755 sdks/python/run_tox_cleanup.sh diff --git a/sdks/python/build.gradle b/sdks/python/build.gradle index 484adb3629e3..13b4ff65fd7d 100644 --- a/sdks/python/build.gradle +++ b/sdks/python/build.gradle @@ -64,7 +64,7 @@ build.dependsOn buildPython task lint (dependsOn: 'setupTest') { doLast { exec { - commandLine 'tox', '-e', 'lint', '-c', 'tox.ini' + commandLine 'tox', '-e', 'py27-lint', '-c', 'tox.ini' } } } @@ -74,7 +74,7 @@ check.dependsOn lint task testCython (dependsOn: ['setupTest', 'testPython', 'testGcp']) { doLast { exec { - commandLine 'tox', '-e', 'py27cython', '-c', 'tox.ini' + commandLine 'tox', '-e', 'py27-cython', '-c', 'tox.ini' } } } @@ -83,7 +83,7 @@ test.dependsOn testCython task testGcp (dependsOn: 'setupTest') { doLast { exec { - commandLine 'tox', '-e', 'py27gcp', '-c', 'tox.ini' + commandLine 'tox', '-e', 'py27-gcp', '-c', 'tox.ini' } } } diff --git a/sdks/python/run_tox_cleanup.sh b/sdks/python/run_tox_cleanup.sh new file mode 100755 index 000000000000..76b683cf588b --- /dev/null +++ b/sdks/python/run_tox_cleanup.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# +# 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 script is used to remove generated files in previous Tox runs so that +# they are not picked up by tests. + +set -e + +for dir in apache_beam target/build; do + if [ -d "${dir}" ]; then + for ext in pyc c so; do + find ${dir} -type f -name "*.${ext}" -delete + done + fi +done diff --git a/sdks/python/setup.py b/sdks/python/setup.py index 02c02f56c424..ffc4df784137 100644 --- a/sdks/python/setup.py +++ b/sdks/python/setup.py @@ -113,11 +113,8 @@ def get_version(): 'futures>=3.1.1,<4.0.0', ] -REQUIRED_SETUP_PACKAGES = [ - 'nose>=1.0', - ] - REQUIRED_TEST_PACKAGES = [ + 'nose>=1.3.7', 'pyhamcrest>=1.9,<2.0', ] @@ -177,6 +174,9 @@ def generate_common_urns(): + '\n') generate_common_urns() +python_requires = '>=2.7' +if os.environ.get('BEAM_EXPERIMENTAL_PY3') is None: + python_requires += ',<3.0' setuptools.setup( name=PACKAGE_NAME, @@ -202,9 +202,8 @@ def generate_common_urns(): 'apache_beam/utils/counters.py', 'apache_beam/utils/windowed_value.py', ]), - setup_requires=REQUIRED_SETUP_PACKAGES, install_requires=REQUIRED_PACKAGES, - python_requires='>=2.7,<3.0', + python_requires=python_requires, test_suite='nose.collector', tests_require=REQUIRED_TEST_PACKAGES, extras_require={ diff --git a/sdks/python/tox.ini b/sdks/python/tox.ini index 857e7b0e571f..705e370bf7e9 100644 --- a/sdks/python/tox.ini +++ b/sdks/python/tox.ini @@ -17,8 +17,7 @@ [tox] # new environments will be excluded by default unless explicitly added to envlist. -# TODO (after BEAM-3671) add lint_py3 back in. -envlist = py27,py27gcp,py27cython,lint_py2,docs +envlist = py27,py27-{gcp,cython,lint},py3-lint,docs toxworkdir = {toxinidir}/target/.tox [pycodestyle] @@ -26,128 +25,89 @@ toxworkdir = {toxinidir}/target/.tox # pylint does not check the number of blank lines. select = E3 +# Shared environment options. +[testenv] +# Set [] options for pip install, e.g., pip install apache-beam[test]. +extras = test +# Always recreate the virtual environment. +recreate = True +# Pass these environment variables to the test environment. +passenv = TRAVIS* +# Don't warn that these commands aren't installed. +whitelist_externals = + find + time + [testenv:py27] -# autocomplete_test depends on nose when invoked directly. -deps = - nose==1.3.7 - grpcio-tools==1.3.5 -whitelist_externals=find commands = - python --version - pip --version - # Clean up all previous python generated files. - - find apache_beam -type f -name '*.pyc' -delete - pip install -e .[test] + {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test -passenv = TRAVIS* + {toxinidir}/run_tox_cleanup.sh -[testenv:py27cython] +[testenv:py27-cython] # cython tests are only expected to work in linux (2.x and 3.x) # If we want to add other platforms in the future, it should be: # `platform = linux2|darwin|...` # See https://docs.python.org/2/library/sys.html#sys.platform for platform codes platform = linux2 -# autocomplete_test depends on nose when invoked directly. deps = - nose==1.3.7 - grpcio-tools==1.3.5 - cython==0.25.2 -whitelist_externals= - find - time + cython==0.26.1 commands = - python --version - pip --version - # Clean up all previous python generated files. - - find apache_beam -type f -name '*.pyc' -delete - # Clean up all previous cython generated files. - - find apache_beam -type f -name '*.c' -delete - - find apache_beam -type f -name '*.so' -delete - - find target/build -type f -name '*.c' -delete - - find target/build -type f -name '*.so' -delete - time pip install -e .[test] + {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test - # Clean up all cython generated files. Ignore if deletion fails. - - find apache_beam -type f -name '*.c' -delete - - find apache_beam -type f -name '*.so' -delete - - find target/build -type f -name '*.c' -delete - - find target/build -type f -name '*.so' -delete -passenv = TRAVIS* + {toxinidir}/run_tox_cleanup.sh -[testenv:py27gcp] -# autocomplete_test depends on nose when invoked directly. -deps = - nose==1.3.7 -whitelist_externals=find +[testenv:py27-gcp] +extras = test,gcp commands = - python --version - pip --version - pip install -e .[test,gcp] - # Clean up all previous python generated files. - - find apache_beam -type f -name '*.pyc' -delete + {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test -passenv = TRAVIS* + {toxinidir}/run_tox_cleanup.sh -[testenv:lint_py2] -deps= - nose==1.3.7 +[testenv:py27-lint] +deps = pycodestyle==2.3.1 pylint==1.7.2 future==0.16.0 isort==4.2.15 flake8==3.5.0 -whitelist_externals=time commands = python --version - pip --version - time pip install -e .[test] time {toxinidir}/run_pylint.sh -passenv = TRAVIS* -[testenv:lint_py3] -deps= - nose==1.3.7 +[testenv:py3-lint] +deps = pycodestyle==2.3.1 pylint==1.7.2 future==0.16.0 isort==4.2.15 flake8==3.5.0 -whitelist_externals=time +setenv = + BEAM_EXPERIMENTAL_PY3=1 commands = - time pip install -e .[test] + python --version time {toxinidir}/run_mini_py3lint.sh -passenv = TRAVIS* - [testenv:docs] -deps= - nose==1.3.7 - grpcio-tools==1.3.5 +extras = docs +deps = Sphinx==1.6.5 sphinx_rtd_theme==0.2.4 -whitelist_externals=time commands = python --version pip --version time pip install -e .[test,gcp,docs] time {toxinidir}/generate_pydoc.sh -passenv = TRAVIS* [testenv:cover] # This is not included in envlist which is defined in [tox]. deps = coverage==4.4.1 - nose==1.3.7 -whitelist_externals=find commands = - python --version - pip --version - pip install -e .[test,gcp] - # Clean up all previous python generated files. - - find apache_beam -type f -name '*.pyc' -delete + {toxinidir}/run_tox_cleanup.sh # Clean up previously collected data. coverage erase coverage run setup.py test @@ -155,4 +115,3 @@ commands = coverage report --skip-covered # Generate report in xml format coverage xml -passenv = TRAVIS* From 0282cd7c1be4e22394a05449c2be11251ff51e02 Mon Sep 17 00:00:00 2001 From: Udi Meiri Date: Tue, 20 Mar 2018 14:57:56 -0700 Subject: [PATCH 2/3] Address review comments. --- sdks/python/build.gradle | 2 +- sdks/python/tox.ini | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/sdks/python/build.gradle b/sdks/python/build.gradle index 13b4ff65fd7d..50bf6bf1413d 100644 --- a/sdks/python/build.gradle +++ b/sdks/python/build.gradle @@ -64,7 +64,7 @@ build.dependsOn buildPython task lint (dependsOn: 'setupTest') { doLast { exec { - commandLine 'tox', '-e', 'py27-lint', '-c', 'tox.ini' + commandLine 'tox', '-e', '{py27,py3}-lint', '-c', 'tox.ini' } } } diff --git a/sdks/python/tox.ini b/sdks/python/tox.ini index 705e370bf7e9..431d9ba19f48 100644 --- a/sdks/python/tox.ini +++ b/sdks/python/tox.ini @@ -17,7 +17,7 @@ [tox] # new environments will be excluded by default unless explicitly added to envlist. -envlist = py27,py27-{gcp,cython,lint},py3-lint,docs +envlist = py27,py27-{gcp,cython2,lint},py3-lint,docs toxworkdir = {toxinidir}/target/.tox [pycodestyle] @@ -27,12 +27,8 @@ select = E3 # Shared environment options. [testenv] -# Set [] options for pip install, e.g., pip install apache-beam[test]. +# Set [] options for pip installation of apache-beam tarball. extras = test -# Always recreate the virtual environment. -recreate = True -# Pass these environment variables to the test environment. -passenv = TRAVIS* # Don't warn that these commands aren't installed. whitelist_externals = find @@ -40,12 +36,15 @@ whitelist_externals = [testenv:py27] commands = + python --version + pip --version {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test {toxinidir}/run_tox_cleanup.sh -[testenv:py27-cython] +# This environment will fail if named "py27-cython". +[testenv:py27-cython2] # cython tests are only expected to work in linux (2.x and 3.x) # If we want to add other platforms in the future, it should be: # `platform = linux2|darwin|...` @@ -54,6 +53,8 @@ platform = linux2 deps = cython==0.26.1 commands = + python --version + pip --version {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test @@ -62,6 +63,8 @@ commands = [testenv:py27-gcp] extras = test,gcp commands = + python --version + pip --version {toxinidir}/run_tox_cleanup.sh python apache_beam/examples/complete/autocomplete_test.py python setup.py test @@ -76,6 +79,7 @@ deps = flake8==3.5.0 commands = python --version + pip --version time {toxinidir}/run_pylint.sh [testenv:py3-lint] @@ -89,17 +93,17 @@ setenv = BEAM_EXPERIMENTAL_PY3=1 commands = python --version + pip --version time {toxinidir}/run_mini_py3lint.sh [testenv:docs] -extras = docs +extras = test,gcp,docs deps = Sphinx==1.6.5 sphinx_rtd_theme==0.2.4 commands = python --version pip --version - time pip install -e .[test,gcp,docs] time {toxinidir}/generate_pydoc.sh [testenv:cover] @@ -107,6 +111,8 @@ commands = deps = coverage==4.4.1 commands = + python --version + pip --version {toxinidir}/run_tox_cleanup.sh # Clean up previously collected data. coverage erase From 2438aba555dc06e16f940b453e2e09f948196ce6 Mon Sep 17 00:00:00 2001 From: Udi Meiri Date: Fri, 23 Mar 2018 09:42:56 -0700 Subject: [PATCH 3/3] Fix py3-lint error and build.gradle. --- sdks/python/apache_beam/typehints/typehints.py | 1 - sdks/python/build.gradle | 2 +- sdks/python/tox.ini | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sdks/python/apache_beam/typehints/typehints.py b/sdks/python/apache_beam/typehints/typehints.py index 543641526f56..af9e1fef70b2 100644 --- a/sdks/python/apache_beam/typehints/typehints.py +++ b/sdks/python/apache_beam/typehints/typehints.py @@ -1127,7 +1127,6 @@ def coerce_to_kv_type(element_type, label=None): Union[tuple(t.tuple_types[1] for t in union_types)]] else: # TODO: Possibly handle other valid types. - print "element_type", element_type raise ValueError( "Input to %r must be compatible with KV[Any, Any]. " "Found %s." % (label, element_type)) diff --git a/sdks/python/build.gradle b/sdks/python/build.gradle index 50bf6bf1413d..2c6637a53732 100644 --- a/sdks/python/build.gradle +++ b/sdks/python/build.gradle @@ -74,7 +74,7 @@ check.dependsOn lint task testCython (dependsOn: ['setupTest', 'testPython', 'testGcp']) { doLast { exec { - commandLine 'tox', '-e', 'py27-cython', '-c', 'tox.ini' + commandLine 'tox', '-e', 'py27-cython2', '-c', 'tox.ini' } } } diff --git a/sdks/python/tox.ini b/sdks/python/tox.ini index 431d9ba19f48..add9bbfb2cc0 100644 --- a/sdks/python/tox.ini +++ b/sdks/python/tox.ini @@ -43,7 +43,7 @@ commands = python setup.py test {toxinidir}/run_tox_cleanup.sh -# This environment will fail if named "py27-cython". +# This environment will fail in Jenkins if named "py27-cython". [testenv:py27-cython2] # cython tests are only expected to work in linux (2.x and 3.x) # If we want to add other platforms in the future, it should be: