Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove pylint #16682

Merged
merged 1 commit into from Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 0 additions & 2 deletions .dockerignore
Expand Up @@ -51,8 +51,6 @@
!.rat-excludes
!.flake8
!.dockerignore
!pylintrc
!pylintrc-tests
!pytest.ini
!CHANGELOG.txt
!LICENSE
Expand Down
2 changes: 1 addition & 1 deletion .github/boring-cyborg.yml
Expand Up @@ -210,7 +210,7 @@ firstPRWelcomeComment: >

Here are some useful points:

- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits](
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits](
https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks)
will help you with that.

Expand Down
73 changes: 9 additions & 64 deletions .github/workflows/ci.yml
Expand Up @@ -310,7 +310,7 @@ jobs:
needs: [build-info, ci-images]
env:
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn) }}
SKIP: "pylint,identity"
SKIP: "identity"
MOUNT_SELECTED_LOCAL_SOURCES: "true"
PYTHON_MAJOR_MINOR_VERSION: ${{needs.build-info.outputs.defaultPythonVersion}}
GITHUB_REGISTRY: ${{ needs.ci-images.outputs.githubRegistry }}
Expand Down Expand Up @@ -340,20 +340,20 @@ jobs:
${{ hashFiles('setup.py', 'setup.cfg') }}"
restore-keys: "\
pre-commit-local-installation-${{steps.host-python-version.outputs.host-python-version}}-"
- name: "Cache pre-commit envs: no-pylint"
- name: "Cache pre-commit envs"
uses: actions/cache@v2
with:
path: ~/.cache/pre-commit
key: "pre-commit-no-pylint-${{steps.host-python-version.outputs.host-python-version}}-\
key: "pre-commit-${{steps.host-python-version.outputs.host-python-version}}-\
${{ hashFiles('.pre-commit-config.yaml') }}"
restore-keys: pre-commit-no-pylint-${{steps.host-python-version.outputs.host-python-version}}
restore-keys: pre-commit-${{steps.host-python-version.outputs.host-python-version}}

- name: "Cache eslint"
uses: actions/cache@v2
with:
path: 'airflow/ui/node_modules'
key: ${{ runner.os }}-ui-node-modules-${{ hashFiles('airflow/ui/**/yarn.lock') }}
- name: "Static checks: except pylint"
- name: "Static checks"
run: ./scripts/ci/static_checks/run_static_checks.sh
env:
VERBOSE: false
Expand All @@ -368,7 +368,7 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
needs: [build-info]
env:
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn) }}
SKIP: "build,mypy,flake8,pylint,bats-in-container-tests,identity"
SKIP: "build,mypy,flake8,bats-in-container-tests,identity"
MOUNT_SELECTED_LOCAL_SOURCES: "true"
PYTHON_MAJOR_MINOR_VERSION: ${{needs.build-info.outputs.defaultPythonVersion}}
if: needs.build-info.outputs.basic-checks-only == 'true'
Expand Down Expand Up @@ -400,68 +400,17 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
${{ hashFiles('setup.py', 'setup.cfg') }}"
restore-keys: "\
pre-commit-local-installation-${{steps.host-python-version.outputs.host-python-version}}-"
- name: "Cache pre-commit envs: no-pylint"
- name: "Cache pre-commit envs"
uses: actions/cache@v2
with:
path: ~/.cache/pre-commit
key: "pre-commit-no-pylint-${{steps.host-python-version.outputs.host-python-version}}-\
key: "pre-commit-basic-${{steps.host-python-version.outputs.host-python-version}}-\
${{ hashFiles('.pre-commit-config.yaml') }}"
restore-keys: pre-commit-no-pylint-${{steps.host-python-version.outputs.host-python-version}}
restore-keys: pre-commit-basic-${{steps.host-python-version.outputs.host-python-version}}
- name: "Static checks: basic checks only"
run: ./scripts/ci/static_checks/run_basic_static_checks.sh "${{ github.sha }}"
env:
VERBOSE: false

static-checks-pylint:
timeout-minutes: 60
name: "Pylint"
runs-on: ${{ fromJson(needs.build-info.outputs.runsOn) }}
needs: [build-info, ci-images]
if: needs.build-info.outputs.basic-checks-only == 'false'
env:
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn) }}
# We want to make sure we have latest sources as only in_container scripts are added
# to the image but we want to static-check all of them
MOUNT_SELECTED_LOCAL_SOURCES: "true"
PYTHON_MAJOR_MINOR_VERSION: ${{needs.build-info.outputs.defaultPythonVersion}}
GITHUB_REGISTRY: ${{ needs.ci-images.outputs.githubRegistry }}
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v2
with:
persist-credentials: false
- name: "Setup python"
uses: actions/setup-python@v2
with:
python-version: ${{needs.build-info.outputs.defaultPythonVersion}}
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Prepare CI image ${{env.PYTHON_MAJOR_MINOR_VERSION}}:${{ env.GITHUB_REGISTRY_PULL_IMAGE_TAG }}"
run: ./scripts/ci/images/ci_prepare_ci_image_on_ci.sh
- name: "Get Python version"
run: "echo \"::set-output name=host-python-version::$(python -c
'import platform; print(platform.python_version())')\""
id: host-python-version
- name: "Cache pre-commit local-installation"
uses: actions/cache@v2
with:
path: ~/.local
key: "pre-commit-local-installation-${{steps.host-python-version.outputs.host-python-version}}-\
${{ hashFiles('setup.py', 'setup.cfg') }}"
restore-keys: "\
pre-commit-local-installation-${{steps.host-python-version.outputs.host-python-version}}-"
- name: "Cache pre-commit envs - pylint"
uses: actions/cache@v2
with:
path: ~/.cache/pre-commit
key: "pre-commit-pylint-${{steps.host-python-version.outputs.host-python-version}}-\
${{ hashFiles('.pre-commit-config.yaml') }}"
restore-keys: pre-commit-pylint-${{steps.host-python-version.outputs.host-python-version}}
- name: "Static checks: pylint"
run: ./scripts/ci/static_checks/run_static_checks.sh pylint
env:
VERBOSE: false

docs:
timeout-minutes: 45
name: "Build docs"
Expand Down Expand Up @@ -1178,7 +1127,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
needs:
- build-info
- static-checks
- static-checks-pylint
- tests-sqlite
- tests-postgres
- tests-mysql
Expand Down Expand Up @@ -1241,7 +1189,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
needs:
- build-info
- static-checks
- static-checks-pylint
- tests-sqlite
- tests-postgres
- tests-mysql
Expand Down Expand Up @@ -1288,7 +1235,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- ci-images
- prod-images
- static-checks
- static-checks-pylint
- tests-sqlite
- tests-mysql
- tests-mssql
Expand Down Expand Up @@ -1357,7 +1303,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- docs
- build-info
- static-checks
- static-checks-pylint
- tests-sqlite
- tests-postgres
- tests-mysql
Expand Down
51 changes: 15 additions & 36 deletions .pre-commit-config.yaml
Expand Up @@ -232,7 +232,7 @@ repos:
name: Run pydocstyle
args:
- --convention=pep257
- --add-ignore=D100,D102,D104,D105,D107,D205,D400,D401
- --add-ignore=D100,D102,D103,D104,D105,D107,D205,D400,D401
exclude: |
(?x)
^tests/.*\.py$|
Expand All @@ -243,6 +243,14 @@ repos:
.*example_dags/.*|
^chart/.*\.py$|
^airflow/_vendor/
additional_dependencies: ['toml']
- repo: https://github.com/asottile/yesqa
rev: v1.2.3
hooks:
- id: yesqa
exclude: |
(?x)
^airflow/_vendor/
- repo: local
hooks:
- id: lint-openapi
Expand Down Expand Up @@ -353,14 +361,7 @@ repos:
^docs/apache-airflow-providers-apache-cassandra/connections/cassandra\.rst$|
^docs/apache-airflow-providers-apache-hive/commits\.rst$|
git|
^pylintrc |
^CHANGELOG\.txt$
- id: consistent-pylint
language: pygrep
name: Check for inconsistent pylint disable/enable without space
entry: "pylint:disable|pylint:enable"
pass_filenames: true
files: \.py$
- id: base-operator
language: pygrep
name: Check BaseOperator[Link] core imports
Expand Down Expand Up @@ -644,12 +645,6 @@ repos:
files: ^airflow/www/static/js/
entry: scripts/ci/static_checks/www_lint.sh
pass_filenames: false
- id: bats-in-container-tests
name: Run in container bats tests
language: system
entry: ./scripts/ci/pre_commit/pre_commit_in_container_bats_test.sh
files: ^tests/bats/in_container/.*\.bats$|^scripts/in_container/.*sh
pass_filenames: false
potiuk marked this conversation as resolved.
Show resolved Hide resolved
## ADD MOST PRE-COMMITS ABOVE THAT LINE
# The below pre-commits are those requiring CI image to be built
- id: build
Expand All @@ -658,6 +653,12 @@ repos:
language: system
always_run: true
pass_filenames: false
- id: bats-in-container-tests
name: Run in container bats tests
language: system
entry: ./scripts/ci/pre_commit/pre_commit_in_container_bats_test.sh
files: ^tests/bats/in_container/.*\.bats$|^scripts/in_container/.*sh
pass_filenames: false
- id: mypy
name: Run mypy
language: system
Expand All @@ -677,28 +678,6 @@ repos:
files: ^docs/.*\.py$
exclude: rtd-deprecation
require_serial: false
- id: pylint
name: Run pylint for main code
language: system
entry: ./scripts/ci/pre_commit/pre_commit_pylint.sh
files: \.py$
exclude: ^scripts/.*\.py$|^dev|^provider_packages|^chart|^tests|^kubernetes_tests|^airflow/_vendor/
pass_filenames: true
require_serial: true
- id: pylint
name: Run pylint for tests
language: system
entry: env PYLINTRC=pylintrc-tests ./scripts/ci/pre_commit/pre_commit_pylint.sh
files: ^tests/.*\.py$
pass_filenames: true
require_serial: true
- id: pylint
name: Run pylint for helm chart tests
language: system
entry: env PYLINTRC=pylintrc-tests ./scripts/ci/pre_commit/pre_commit_pylint.sh
files: ^chart/.*\.py$
pass_filenames: true
require_serial: true
- id: flake8
name: Run flake8
language: system
Expand Down
3 changes: 0 additions & 3 deletions .rat-excludes
Expand Up @@ -83,9 +83,6 @@ PROVIDER_CHANGES*.md
manifests/*
redirects.txt

# Temporary list of files to make compatible with Pylint
pylint_todo.txt

# Locally mounted files
.*egg-info/*
.bash_history
Expand Down
31 changes: 15 additions & 16 deletions BREEZE.rst
Expand Up @@ -2240,23 +2240,22 @@ This is the current syntax for `./breeze <./breeze>`_:
Run selected static checks for currently changed files. You should specify static check that
you would like to run or 'all' to run all checks. One of:

all all-but-pylint airflow-config-yaml airflow-providers-available
airflow-provider-yaml-files-ok base-operator bats-tests bats-in-container-tests
black blacken-docs boring-cyborg build build-providers-dependencies
check-apache-license check-builtin-literals check-executables-have-shebangs
check-hooks-apply check-integrations check-merge-conflict check-xml
consistent-pylint daysago-import-check debug-statements detect-private-key doctoc
dont-use-safe-filter end-of-file-fixer fix-encoding-pragma flake8 flynt forbid-tabs
helm-lint identity incorrect-use-of-LoggingMixin insert-license isort json-schema
language-matters lint-dockerfile lint-openapi markdownlint mermaid mixed-line-ending
mypy mypy-helm no-providers-in-core-examples no-relative-imports
pre-commit-descriptions pre-commit-hook-names pretty-format-json
all airflow-config-yaml airflow-providers-available airflow-provider-yaml-files-ok
base-operator bats-tests bats-in-container-tests black blacken-docs boring-cyborg
build build-providers-dependencies check-apache-license check-builtin-literals
check-executables-have-shebangs check-hooks-apply check-integrations
check-merge-conflict check-xml daysago-import-check debug-statements
detect-private-key doctoc dont-use-safe-filter end-of-file-fixer fix-encoding-pragma
flake8 flynt forbid-tabs helm-lint identity incorrect-use-of-LoggingMixin
insert-license isort json-schema language-matters lint-dockerfile lint-openapi
markdownlint mermaid mixed-line-ending mypy mypy-helm no-providers-in-core-examples
no-relative-imports pre-commit-descriptions pre-commit-hook-names pretty-format-json
provide-create-sessions providers-changelogs providers-init-file provider-yamls
pydevd pydocstyle pylint pylint-tests python-no-log-warn pyupgrade
restrict-start_date rst-backticks setup-order setup-extra-packages shellcheck
sort-in-the-wild sort-spelling-wordlist stylelint trailing-whitespace ui-lint
update-breeze-file update-extras update-local-yml-file update-setup-cfg-file
verify-db-migrations-documented version-sync www-lint yamllint
pydevd pydocstyle python-no-log-warn pyupgrade restrict-start_date rst-backticks
setup-order setup-extra-packages shellcheck sort-in-the-wild sort-spelling-wordlist
stylelint trailing-whitespace ui-lint update-breeze-file update-extras
update-local-yml-file update-setup-cfg-file verify-db-migrations-documented
version-sync www-lint yamllint yesqa

You can pass extra arguments including options to the pre-commit framework as
<EXTRA_ARGS> passed after --. For example:
Expand Down
4 changes: 1 addition & 3 deletions CI.rst
Expand Up @@ -653,9 +653,7 @@ This workflow is a regular workflow that performs all checks of Airflow code.
+---------------------------+----------------------------------------------+-------+-------+------+
| CI Images | Waits for CI Images (3) | Yes | Yes | Yes |
+---------------------------+----------------------------------------------+-------+-------+------+
| Static checks | Performs static checks without pylint | Yes | Yes | Yes |
+---------------------------+----------------------------------------------+-------+-------+------+
| Static checks: pylint | Performs pylint static checks | Yes | Yes | Yes |
| Static checks | Performs static checks | Yes | Yes | Yes |
+---------------------------+----------------------------------------------+-------+-------+------+
| Build docs | Builds documentation | Yes | Yes | Yes |
+---------------------------+----------------------------------------------+-------+-------+------+
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.rst
Expand Up @@ -923,7 +923,7 @@ as described in the static code checks documentation.
Coding style and best practices
===============================

Most of our coding style rules are enforced programmatically by flake8 and pylint (which are run automatically
Most of our coding style rules are enforced programmatically by flake8 and mypy (which are run automatically
on every pull request), but there are some rules that are not yet automated and are more Airflow specific or
semantic than style

Expand Down
14 changes: 2 additions & 12 deletions CONTRIBUTORS_QUICK_START.rst
Expand Up @@ -632,8 +632,7 @@ All Tests are inside ./tests directory.
entrypoint_exec.sh* run_install_and_test_provider_packages.sh*
_in_container_script_init.sh* run_mypy.sh*
prod/ run_prepare_provider_packages.sh*
refresh_pylint_todo.sh* run_prepare_provider_documentation.sh*
run_ci_tests.sh* run_pylint.sh*
run_ci_tests.sh* run_prepare_provider_documentation.sh*
run_clear_tmp.sh* run_system_tests.sh*
run_docs_build.sh* run_tmux_welcome.sh*
run_extract_tests.sh* stop_tmux_airflow.sh*
Expand Down Expand Up @@ -830,8 +829,7 @@ To avoid burden on CI infrastructure and to save time, Pre-commit hooks can be r
entrypoint_exec.sh* run_install_and_test_provider_packages.sh*
_in_container_script_init.sh* run_mypy.sh*
prod/ run_prepare_provider_packages.sh*
refresh_pylint_todo.sh* run_prepare_provider_documentation.sh*
run_ci_tests.sh* run_pylint.sh*
run_ci_tests.sh* run_prepare_provider_documentation.sh*
run_clear_tmp.sh* run_system_tests.sh*
run_docs_build.sh* run_tmux_welcome.sh*
run_extract_tests.sh* stop_tmux_airflow.sh*
Expand Down Expand Up @@ -875,14 +873,6 @@ To avoid burden on CI infrastructure and to save time, Pre-commit hooks can be r
<a href="https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#pre-commit-hooks" target="_blank">
Pre-commit Hooks</a>

- |Pylint Static Code Checks|

.. |Pylint Static Code Checks| raw:: html

<a href="https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#pylint-static-code-checks"
target="_blank">Pylint Static Code Checks</a>


- |Running Static Code Checks via Breeze|

.. |Running Static Code Checks via Breeze| raw:: html
Expand Down
6 changes: 3 additions & 3 deletions PULL_REQUEST_WORKFLOW.rst
Expand Up @@ -109,7 +109,7 @@ We have the following test types (separated by packages in which they are):

We also have several special kinds of tests that are not separated by packages but they are marked with
pytest markers. They can be found in any of those packages and they can be selected by the appropriate
pylint custom command line options. See `TESTING.rst <TESTING.rst>`_ for details but those are:
pytest custom command line options. See `TESTING.rst <TESTING.rst>`_ for details but those are:

* Integration - tests that require external integration images running in docker-compose
* Quarantined - tests that are flaky and need to be fixed
Expand Down Expand Up @@ -175,11 +175,11 @@ The logic implemented for the changes works as follows:
Quarantined tests are described in `TESTING.rst <TESTING.rst>`_

11) There is a special case of static checks. In case the above logic determines that the CI image
needs to be build, we run long and more comprehensive version of static checks - including Pylint,
needs to be build, we run long and more comprehensive version of static checks - including
Mypy, Flake8. And those tests are run on all files, no matter how many files changed.
In case the image is not built, we run only simpler set of changes - the longer static checks
that require CI image are skipped, and we only run the tests on the files that changed in the incoming
commit - unlike pylint/flake8/mypy, those static checks are per-file based and they should not miss any
commit - unlike flake8/mypy, those static checks are per-file based and they should not miss any
important change.

Similarly to selective tests we also run selective security scans. In Pull requests,
Expand Down