Skip to content

Commit

Permalink
[AIRFLOW-5285] Pylint pre-commit filters out pylint_todo files (#5884)
Browse files Browse the repository at this point in the history
(cherry picked from commit d24db82)
  • Loading branch information
potiuk authored and kaxil committed Aug 30, 2019
1 parent 4e28ee2 commit ac2ae48
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 17 deletions.
16 changes: 14 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -142,10 +156,8 @@ repos:
language: system
entry: ./scripts/ci/ci_mypy.sh
files: \.py$
require_serial: true
- id: flake8
name: Run flake8
language: system
entry: ./scripts/ci/ci_flake8.sh
files: \.py$
require_serial: true
14 changes: 10 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ env:
global:
- BUILD_ID=${TRAVIS_BUILD_ID}
- AIRFLOW_CONTAINER_MOUNT_HOST_VOLUMES="false"
- AIRFLOW_CI_SILENT="false"
- AIRFLOW_CI_SILENT="true"
- CI="true"
python: "3.6"
stages:
Expand Down Expand Up @@ -58,10 +58,16 @@ jobs:
env: BACKEND=postgres ENV=kubernetes KUBERNETES_VERSION=v1.9.0 KUBERNETES_MODE=git_mode
python: "2.7"
stage: test
- name: "Static checks"
- 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_licence.sh
- name: "Check licence compliance for Apache"
stage: pre-test
script: ./scripts/ci/ci_check_license.sh
- name: "Pylint checks"
stage: pre-test
script: ./scripts/ci/ci_run_all_static_tests_pylint.sh
- name: "Build documentation"
stage: pre-test
script: ./scripts/ci/ci_docs.sh
services:
Expand Down
7 changes: 1 addition & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,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
Expand Down Expand Up @@ -682,11 +682,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`

Expand Down
13 changes: 13 additions & 0 deletions scripts/ci/_utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -602,3 +602,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
}
2 changes: 1 addition & 1 deletion scripts/ci/ci_before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions scripts/ci/ci_pylint_main.sh → scripts/ci/ci_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,4 @@ script_start

rebuild_image_if_needed_for_static_checks

run_pylint_main "$@"

script_end
4 changes: 3 additions & 1 deletion scripts/ci/ci_run_all_static_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ script_start

rebuild_image_if_needed_for_static_checks

run_pylint_tests "$@"
SKIP=check-apache-license pre-commit run --all-files --show-diff-on-failure

script_end

0 comments on commit ac2ae48

Please sign in to comment.