[AIRFLOW-5285] Pylint pre-commit filters out pylint_todo files (apach…

…e#5884)

* [AIRFLOW-5285] Pylint pre-commit filters out pylint_todo files

.pre-commit-config.yaml

@@ -20,6 +20,20 @@ default_language_version:
   # force all unspecified python hooks to run python3
   python: python3
 repos:
+  - repo: local
+    hooks:
+      - id: build
+        name: Check if image build is needed
+        entry: ./scripts/ci/ci_build.sh
+        language: system
+        always_run: true
+        pass_filenames: false
+      - id: check-apache-license
+        name: Check if licences are OK for Apache
+        entry: ./scripts/ci/ci_check_license.sh
+        language: system
+        always_run: true
+        pass_filenames: false
   - repo: https://github.com/Lucas-C/pre-commit-hooks
     rev: v1.1.6
     hooks:
@@ -142,23 +156,20 @@ repos:
         language: system
         entry: ./scripts/ci/ci_mypy.sh
         files: \.py$
-        require_serial: true
+        exclude: ^airflow/_vendor/.*$
       - id: pylint
         name: Run pylint for main sources
         language: system
         entry: ./scripts/ci/ci_pylint_main.sh
         files: \.py$
-        exclude: ^tests/.*\.py$
-        require_serial: true
+        exclude: ^tests/.*\.py$|^airflow/_vendor/.*$
       - id: pylint
         name: Run pylint for tests
         language: system
         entry: ./scripts/ci/ci_pylint_tests.sh
         files: ^tests/.*\.py$
-        require_serial: true
       - id: flake8
         name: Run flake8
         language: system
         entry: ./scripts/ci/ci_flake8.sh
         files: \.py$
-        require_serial: true

.travis.yml

@@ -50,18 +50,18 @@ jobs:
       env: BACKEND=postgres ENV=kubernetes KUBERNETES_VERSION=v1.13.0 KUBERNETES_MODE=git_mode
       python: "3.6"
       stage: test
-    - name: "Static checks except pylint and docs"
+    - name: "Static checks (no pylint, no licence check)"
       stage: pre-test
-      script: ./scripts/ci/ci_run_all_static_tests.sh
-    - name: Check docs
+      script: ./scripts/ci/ci_run_all_static_tests_except_pylint_licence.sh
+    - name: "Check licence compliance for Apache"
       stage: pre-test
-      script: ./scripts/ci/ci_docs.sh
-    - name: Pylint main
+      script: ./scripts/ci/ci_check_license.sh
+    - name: "Pylint checks"
       stage: pre-test
-      script: ./scripts/ci/ci_pylint_main.sh
-    - name: Pylint tests
+      script: ./scripts/ci/ci_run_all_static_tests_pylint.sh
+    - name: "Build documentation"
       stage: pre-test
-      script: ./scripts/ci/ci_pylint_tests.sh
+      script: ./scripts/ci/ci_docs.sh
 services:
   - docker
 before_install:

CONTRIBUTING.md

@@ -687,7 +687,7 @@ In Linux you typically install them with `sudo apt install` on MacOS with `brew
 
 The current list of prerequisites:
 
-* xmllint: Linux - install via `sudo apt install xmllint`, MacOS - install via`brew install xmllint`
+* xmllint: Linux - install via `sudo apt install xmllint`, MacOS - install via `brew install xmllint`
 * yamllint: install via `pip install yamllint`
 
 ## Pre-commit hooks installed
@@ -729,11 +729,6 @@ run pre-commit hooks manually as needed.
 *You can run all checks manually on all files by running:*
 `SKIP=pylint pre-commit run --all-files`
 
-Note this might be very slow for individual tests with pylint because of passing individual files. It is
-recommended to run `/scripts/ci/ci_pylint_main.sh` (for the main application files) or
-`/scripts/ci/ci_pylint_tests.sh` (for tests) for pylint check.
-You can also adding SKIP=pylint variable (as in the example above) if you run pre-commit hooks with --all-files switch.
-
 *You can skip one or more of the checks by specifying comma-separated list of checks to skip in SKIP variable:*
 `SKIP=pylint,mypy pre-commit run --all-files`
 

airflow/gcp/operators/vision.py

@@ -1,4 +1,4 @@
-# -*- coding: utf-8 -*-
+# -*- coding: utf-8 -*-  # pylint: disable=too-many-lines
 #
 # Licensed to the Apache Software Foundation (ASF) under one
 # or more contributor license agreements.  See the NOTICE file
@@ -20,7 +20,6 @@
 This module contains a Google Cloud Vision operator.
 """
 
-# pylint: disable=too-many-lines
 
 from copy import deepcopy
 from typing import Union, List, Dict, Any, Sequence, Tuple, Optional

scripts/ci/_utils.sh

@@ -660,3 +660,16 @@ function run_docker_lint() {
         echo
     fi
 }
+
+function filter_out_files_from_pylint_todo_list() {
+  FILTERED_FILES=()
+  set +e
+  for FILE in "$@"
+  do
+      if ! grep -x "./${FILE}" <"${AIRFLOW_SOURCES}/scripts/ci/pylint_todo.txt" >/dev/null; then
+          FILTERED_FILES+=("${FILE}")
+      fi
+  done
+  set -e
+  export FILTERED_FILES
+}

scripts/ci/ci_before_install.sh

@@ -34,7 +34,7 @@ docker system prune --all --force
 
 if [[ ${TRAVIS_JOB_NAME} == "Tests"* ]]; then
     rebuild_image_if_needed_for_tests
-elif [[ ${TRAVIS_JOB_NAME} == "Check license headers"  ]]; then
+elif [[ ${TRAVIS_JOB_NAME} == "Check lic"* ]]; then
     rebuild_image_if_needed_for_checklicence
 else
     rebuild_image_if_needed_for_static_checks

scripts/ci/ci_build.sh

@@ -0,0 +1,37 @@
+#!/usr/bin/env 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.
+
+set -euo pipefail
+
+MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+export AIRFLOW_CI_SILENT=${AIRFLOW_CI_SILENT:="true"}
+export ASSUME_QUIT_TO_ALL_QUESTIONS=${ASSUME_QUIT_TO_ALL_QUESTIONS:="true"}
+
+# shellcheck source=scripts/ci/_utils.sh
+. "${MY_DIR}/_utils.sh"
+
+basic_sanity_checks
+
+force_python_3_5
+
+script_start
+
+rebuild_image_if_needed_for_static_checks
+
+script_end

scripts/ci/ci_pylint_main.sh

@@ -34,6 +34,16 @@ script_start
 
 rebuild_image_if_needed_for_static_checks
 
-run_pylint_main "$@"
+if [[ "${#@}" != "0" ]]; then
+    filter_out_files_from_pylint_todo_list "$@"
+
+    if [[ "${#FILTERED_FILES[@]}" == "0" ]]; then
+        echo "Filtered out all files. Skipping pylint."
+    else
+        run_pylint_main "${FILTERED_FILES[@]}"
+    fi
+else
+    run_pylint_main
+fi
 
 script_end

scripts/ci/ci_pylint_tests.sh

@@ -34,6 +34,16 @@ script_start
 
 rebuild_image_if_needed_for_static_checks
 
-run_pylint_tests "$@"
+if [[ "${#@}" != "0" ]]; then
+    filter_out_files_from_pylint_todo_list "$@"
+
+    if [[ "${#FILTERED_FILES[@]}" == "0" ]]; then
+        echo "Filtered out all files. Skipping pylint."
+    else
+        run_pylint_tests "${FILTERED_FILES[@]}"
+    fi
+else
+    run_pylint_tests
+fi
 
 script_end

scripts/ci/ci_run_all_static_tests.sh

@@ -34,6 +34,8 @@ script_start
 
 rebuild_image_if_needed_for_static_checks
 
-SKIP=pylint pre-commit run --all-files --show-diff-on-failure
+rebuild_image_if_needed_for_checklicence
+
+pre-commit run --all-files --show-diff-on-failure
 
 script_end

scripts/ci/ci_run_all_static_tests_except_pylint.sh

@@ -0,0 +1,41 @@
+#!/usr/bin/env 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.
+
+set -euo pipefail
+
+MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+export AIRFLOW_CI_SILENT=${AIRFLOW_CI_SILENT:="true"}
+export ASSUME_QUIT_TO_ALL_QUESTIONS=${ASSUME_QUIT_TO_ALL_QUESTIONS:="true"}
+
+# shellcheck source=scripts/ci/_utils.sh
+. "${MY_DIR}/_utils.sh"
+
+basic_sanity_checks
+
+force_python_3_5
+
+script_start
+
+rebuild_image_if_needed_for_static_checks
+
+rebuild_image_if_needed_for_checklicence
+
+SKIP=pylint pre-commit run --all-files --show-diff-on-failure
+
+script_end

scripts/ci/ci_run_all_static_tests_except_pylint_licence.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env 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.
+
+set -euo pipefail
+
+MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+export AIRFLOW_CI_SILENT=${AIRFLOW_CI_SILENT:="true"}
+export ASSUME_QUIT_TO_ALL_QUESTIONS=${ASSUME_QUIT_TO_ALL_QUESTIONS:="true"}
+
+# shellcheck source=scripts/ci/_utils.sh
+. "${MY_DIR}/_utils.sh"
+
+basic_sanity_checks
+
+force_python_3_5
+
+script_start
+
+rebuild_image_if_needed_for_static_checks
+
+SKIP=pylint,check-apache-license pre-commit run --all-files --show-diff-on-failure
+
+script_end

scripts/ci/ci_run_all_static_tests_pylint.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env 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.
+
+set -euo pipefail
+
+MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+export AIRFLOW_CI_SILENT=${AIRFLOW_CI_SILENT:="true"}
+export ASSUME_QUIT_TO_ALL_QUESTIONS=${ASSUME_QUIT_TO_ALL_QUESTIONS:="true"}
+
+# shellcheck source=scripts/ci/_utils.sh
+. "${MY_DIR}/_utils.sh"
+
+basic_sanity_checks
+
+force_python_3_5
+
+script_start
+
+rebuild_image_if_needed_for_static_checks
+
+pre-commit run pylint --all-files --show-diff-on-failure
+
+script_end