Repository: incubator-airflow Updated Branches: refs/heads/master 8e7b0abed -> b9c82c040
[AIRFLOW-1870] Enable flake8 tests Flake8 tests now run for diffs Closes #2829 from bolkedebruin/use_flake8 Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/b9c82c04 Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/b9c82c04 Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/b9c82c04 Branch: refs/heads/master Commit: b9c82c0400017b24c2670392d8b850b2fb0de8eb Parents: 8e7b0ab Author: Bolke de Bruin <[email protected]> Authored: Thu Nov 30 15:57:17 2017 +0100 Committer: Bolke de Bruin <[email protected]> Committed: Thu Nov 30 15:57:17 2017 +0100 ---------------------------------------------------------------------- .github/PULL_REQUEST_TEMPLATE.md | 6 +- .travis.yml | 43 +++------ scripts/ci/flake8_diff.sh | 164 ++++++++++++++++++++++++++++++++++ scripts/ci/run_tests.sh | 10 ++- tox.ini | 43 +++++---- 5 files changed, 216 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/b9c82c04/.github/PULL_REQUEST_TEMPLATE.md ---------------------------------------------------------------------- diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 63e3b02..6103caa 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,7 +1,4 @@ -Dear Airflow maintainers, - -Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below! - +Make sure you have checked _all_ steps below. ### JIRA - [ ] My PR addresses the following [Airflow JIRA](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR" @@ -25,3 +22,4 @@ Please accept this PR. I understand that it will not be reviewed until I have ch 5. Body wraps at 72 characters 6. Body explains "what" and "why", not "how" +- [ ] Passes `git diff upstream/master -u -- "*.py" | flake8 --diff` http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/b9c82c04/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index d3cd216..fb4ef21 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,44 +47,29 @@ env: # does not work with python 3 - BOTO_CONFIG=/tmp/bogusvalue matrix: - - TOX_ENV=py27-cdh-airflow_backend_mysql - - TOX_ENV=py27-cdh-airflow_backend_sqlite - - TOX_ENV=py27-cdh-airflow_backend_postgres -# - TOX_ENV=py27-hdp-airflow_backend_mysql -# - TOX_ENV=py27-hdp-airflow_backend_sqlite -# - TOX_ENV=py27-hdp-airflow_backend_postgres - - TOX_ENV=py34-cdh-airflow_backend_mysql - - TOX_ENV=py34-cdh-airflow_backend_sqlite - - TOX_ENV=py34-cdh-airflow_backend_postgres -# - TOX_ENV=py34-hdp-airflow_backend_mysql -# - TOX_ENV=py34-hdp-airflow_backend_sqlite -# - TOX_ENV=py34-hdp-airflow_backend_postgres + - TOX_ENV=py27-backend_mysql + - TOX_ENV=py27-backend_sqlite + - TOX_ENV=py27-backend_postgres + - TOX_ENV=py34-backend_mysql + - TOX_ENV=py34-backend_sqlite + - TOX_ENV=py34-backend_postgres + - TOX_ENV=flake8 matrix: exclude: - python: "3.4" - env: TOX_ENV=py27-cdh-airflow_backend_mysql + env: TOX_ENV=py27-backend_mysql - python: "3.4" - env: TOX_ENV=py27-cdh-airflow_backend_sqlite + env: TOX_ENV=py27-backend_sqlite - python: "3.4" - env: TOX_ENV=py27-cdh-airflow_backend_postgres - - python: "3.4" - env: TOX_ENV=py27-hdp-airflow_backend_mysql - - python: "3.4" - env: TOX_ENV=py27-hdp-airflow_backend_sqlite - - python: "3.4" - env: TOX_ENV=py27-hdp-airflow_backend_postgres - - python: "2.7" - env: TOX_ENV=py34-cdh-airflow_backend_mysql - - python: "2.7" - env: TOX_ENV=py34-cdh-airflow_backend_sqlite + env: TOX_ENV=py27-backend_postgres - python: "2.7" - env: TOX_ENV=py34-cdh-airflow_backend_postgres + env: TOX_ENV=py34-backend_mysql - python: "2.7" - env: TOX_ENV=py34-hdp-airflow_backend_mysql + env: TOX_ENV=py34-backend_sqlite - python: "2.7" - env: TOX_ENV=py34-hdp-airflow_backend_sqlite + env: TOX_ENV=py34-backend_postgres - python: "2.7" - env: TOX_ENV=py34-hdp-airflow_backend_postgres + env: TOX_ENV=flake8 cache: directories: - $HOME/.wheelhouse/ http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/b9c82c04/scripts/ci/flake8_diff.sh ---------------------------------------------------------------------- diff --git a/scripts/ci/flake8_diff.sh b/scripts/ci/flake8_diff.sh new file mode 100755 index 0000000..376be9b --- /dev/null +++ b/scripts/ci/flake8_diff.sh @@ -0,0 +1,164 @@ +#!/usr/bin/env bash + +# Copyright (c) 2007â2017 The scikit-learn developers. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# 3. Neither the name of the Scikit-learn Developers nor the names of +# its contributors may be used to endorse or promote products +# derived from this software without specific prior written +# permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +# This script is used in Travis to check that PRs do not add obvious +# flake8 violations. It relies on two things: +# - find common ancestor between branch and +# apache/incubator-airflow remote +# - run flake8 --diff on the diff between the branch and the common +# ancestor +# +# Additional features: +# - the line numbers in Travis match the local branch on the PR +# author machine. +# - ./build_tools/travis/flake8_diff.sh can be run locally for quick +# turn-around + +set -e +# pipefail is necessary to propagate exit codes +set -o pipefail + +PROJECT=apache/incubator-airflow +PROJECT_URL=https://github.com/$PROJECT.git + +# Find the remote with the project name (upstream in most cases) +REMOTE=$(git remote -v | grep $PROJECT | cut -f1 | head -1 || echo '') + +# Add a temporary remote if needed. For example this is necessary when +# Travis is configured to run in a fork. In this case 'origin' is the +# fork and not the reference repo we want to diff against. +if [[ -z "$REMOTE" ]]; then + TMP_REMOTE=tmp_reference_upstream + REMOTE=$TMP_REMOTE + git remote add $REMOTE $PROJECT_URL +fi + +echo "Remotes:" +echo '--------------------------------------------------------------------------------' +git remote --verbose + +# Travis does the git clone with a limited depth (50 at the time of +# writing). This may not be enough to find the common ancestor with +# $REMOTE/master so we unshallow the git checkout +if [[ -a .git/shallow ]]; then + echo -e '\nTrying to unshallow the repo:' + echo '--------------------------------------------------------------------------------' + git fetch --unshallow +fi + +if [[ "$TRAVIS" == "true" ]]; then + if [[ "$TRAVIS_PULL_REQUEST" == "false" ]] + then + # In main repo, using TRAVIS_COMMIT_RANGE to test the commits + # that were pushed into a branch + if [[ "$PROJECT" == "$TRAVIS_REPO_SLUG" ]]; then + if [[ -z "$TRAVIS_COMMIT_RANGE" ]]; then + echo "New branch, no commit range from Travis so passing this test by convention" + exit 0 + fi + COMMIT_RANGE=$TRAVIS_COMMIT_RANGE + fi + else + # We want to fetch the code as it is in the PR branch and not + # the result of the merge into master. This way line numbers + # reported by Travis will match with the local code. + LOCAL_BRANCH_REF=travis_pr_$TRAVIS_PULL_REQUEST + # In Travis the PR target is always origin + git fetch origin pull/$TRAVIS_PULL_REQUEST/head:refs/$LOCAL_BRANCH_REF + fi +fi + +# If not using the commit range from Travis we need to find the common +# ancestor between $LOCAL_BRANCH_REF and $REMOTE/master +if [[ -z "$COMMIT_RANGE" ]]; then + if [[ -z "$LOCAL_BRANCH_REF" ]]; then + LOCAL_BRANCH_REF=$(git rev-parse --abbrev-ref HEAD) + fi + echo -e "\nLast 2 commits in $LOCAL_BRANCH_REF:" + echo '--------------------------------------------------------------------------------' + git --no-pager log -2 $LOCAL_BRANCH_REF + + REMOTE_MASTER_REF="$REMOTE/master" + # Make sure that $REMOTE_MASTER_REF is a valid reference + echo -e "\nFetching $REMOTE_MASTER_REF" + echo '--------------------------------------------------------------------------------' + git fetch $REMOTE master:refs/remotes/$REMOTE_MASTER_REF + LOCAL_BRANCH_SHORT_HASH=$(git rev-parse --short $LOCAL_BRANCH_REF) + REMOTE_MASTER_SHORT_HASH=$(git rev-parse --short $REMOTE_MASTER_REF) + + COMMIT=$(git merge-base $LOCAL_BRANCH_REF $REMOTE_MASTER_REF) || \ + echo "No common ancestor found for $(git show $LOCAL_BRANCH_REF -q) and $(git show $REMOTE_MASTER_REF -q)" + + if [ -z "$COMMIT" ]; then + exit 1 + fi + + COMMIT_SHORT_HASH=$(git rev-parse --short $COMMIT) + + echo -e "\nCommon ancestor between $LOCAL_BRANCH_REF ($LOCAL_BRANCH_SHORT_HASH)"\ + "and $REMOTE_MASTER_REF ($REMOTE_MASTER_SHORT_HASH) is $COMMIT_SHORT_HASH:" + echo '--------------------------------------------------------------------------------' + git --no-pager show --no-patch $COMMIT_SHORT_HASH + + COMMIT_RANGE="$COMMIT_SHORT_HASH..$LOCAL_BRANCH_SHORT_HASH" + + if [[ -n "$TMP_REMOTE" ]]; then + git remote remove $TMP_REMOTE + fi + +else + echo "Got the commit range from Travis: $COMMIT_RANGE" +fi + +echo -e '\nRunning flake8 on the diff in the range' "$COMMIT_RANGE" \ + "($(git rev-list $COMMIT_RANGE | wc -l) commit(s)):" +echo '--------------------------------------------------------------------------------' + +MODIFIED_FILES="$(git diff --name-only $COMMIT_RANGE || echo "no_match")" + +check_files() { + files="$1" + shift + options="$*" + if [ -n "$files" ]; then + # Conservative approach: diff without context (--unified=0) so that code + # that was not changed does not create failures + git diff --unified=0 $COMMIT_RANGE -- $files | flake8 --diff --show-source $options + fi +} + +if [[ "$MODIFIED_FILES" == "no_match" ]]; then + echo "No file outside ignored locations has been modified" +else + + check_files "$(echo "$MODIFIED_FILES" | grep -v ^examples)" + check_files "$(echo "$MODIFIED_FILES" | grep ^examples)" \ + --config ./examples/.flake8 +fi +echo -e "No problem detected by flake8\n" http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/b9c82c04/scripts/ci/run_tests.sh ---------------------------------------------------------------------- diff --git a/scripts/ci/run_tests.sh b/scripts/ci/run_tests.sh index 0ae22ef..1253686 100755 --- a/scripts/ci/run_tests.sh +++ b/scripts/ci/run_tests.sh @@ -38,5 +38,11 @@ if [ "${TRAVIS}" ]; then kinit -kt ${KRB5_KTNAME} airflow fi -echo Backend: $AIRFLOW__CORE__SQL_ALCHEMY_CONN -./run_unit_tests.sh +if [[ "$RUN_FLAKE8" == "true" ]]; then + ./flake8_diff.sh +fi + +if [[ "$SKIP_TESTS" != "true" ]]; then + echo Backend: $AIRFLOW__CORE__SQL_ALCHEMY_CONN + ./run_unit_tests.sh +fi http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/b9c82c04/tox.ini ---------------------------------------------------------------------- diff --git a/tox.ini b/tox.ini index ae7f5f5..cd73079 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ # limitations under the License. [tox] -envlist = {py27,py34}-{cdh,hdp}-airflow_backend_{mysql,sqlite,postgres} +envlist = flake8,{py27,py34}-backend_{mysql,sqlite,postgres} skipsdist=True [global] @@ -21,29 +21,31 @@ find_links = {homedir}/.wheelhouse {homedir}/.pip-cache +[flake8] +max-line-length = 90 +ignore = E731 + [testenv] deps = wheel coveralls + basepython = py27: python2.7 py34: python3.4 + setenv = COVERALLS_REPO_TOKEN=ic8IH7CrUrtweVbmY3VZQ7ncEGe1XJA5E - cdh: HADOOP_DISTRO=cdh - cdh: HADOOP_HOME=/tmp/hadoop-cdh - cdh: HADOOP_OPTS=-D/tmp/krb5.conf - cdh: MINICLUSTER_HOME=/tmp/minicluster-1.1-SNAPSHOT - cdh: HIVE_HOME=/tmp/hive - hdp: HADOOP_DISTRO=hdp - hdp: HADOOP_HOME=/tmp/hadoop-hdp - hdp: HADOOP_OPTS=-D/tmp/krb5.conf - hdp: MINICLUSTER_HOME=/tmp/minicluster-1.1-SNAPSHOT - hdp: HIVE_HOME=/tmp/hive - airflow_backend_mysql: AIRFLOW__CORE__SQL_ALCHEMY_CONN=mysql://root@localhost/airflow - airflow_backend_postgres: AIRFLOW__CORE__SQL_ALCHEMY_CONN=postgresql+psycopg2://postgres@localhost/airflow - airflow_backend_sqlite: AIRFLOW__CORE__SQL_ALCHEMY_CONN=sqlite:///{homedir}/airflow/airflow.db - airflow_backend_sqlite: AIRFLOW__CORE__EXECUTOR=SequentialExecutor + HADOOP_DISTRO=cdh + HADOOP_HOME=/tmp/hadoop-cdh + HADOOP_OPTS=-D/tmp/krb5.conf + MINICLUSTER_HOME=/tmp/minicluster-1.1-SNAPSHOT + HIVE_HOME=/tmp/hive + backend_mysql: AIRFLOW__CORE__SQL_ALCHEMY_CONN=mysql://root@localhost/airflow + backend_postgres: AIRFLOW__CORE__SQL_ALCHEMY_CONN=postgresql+psycopg2://postgres@localhost/airflow + backend_sqlite: AIRFLOW__CORE__SQL_ALCHEMY_CONN=sqlite:///{homedir}/airflow/airflow.db + backend_sqlite: AIRFLOW__CORE__EXECUTOR=SequentialExecutor + passenv = HOME JAVA_HOME @@ -58,6 +60,7 @@ passenv = BOTO_CONFIG KRB5_CONFIG KRB5_KTNAME + commands = pip wheel -w {homedir}/.wheelhouse -f {homedir}/.wheelhouse -r scripts/ci/requirements.txt pip install --find-links={homedir}/.wheelhouse --no-index -r scripts/ci/requirements.txt @@ -69,3 +72,13 @@ commands = {toxinidir}/scripts/ci/run_tests.sh [] {toxinidir}/scripts/ci/check-license.sh coveralls + +[testenv:flake8] +basepython = python3 + +deps = + flake8 + +commands = + {toxinidir}/scripts/ci/flake8_diff.sh +
