425 строки
18 KiB
ReStructuredText
425 строки
18 KiB
ReStructuredText
.. 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.
|
|
|
|
.. contents:: :local:
|
|
|
|
Static Code Checks
|
|
==================
|
|
|
|
The static code checks in Airflow are used to verify that the code meets certain quality standards.
|
|
All the static code checks can be run through pre-commit hooks.
|
|
|
|
Some of the static checks in pre-commits require Breeze Docker images to be installed locally.
|
|
The pre-commit hooks perform all the necessary installation when you run them
|
|
for the first time. See the table below to identify which pre-commit checks require the Breeze Docker images.
|
|
|
|
Sometimes your image is outdated and needs to be rebuilt because some dependencies have been changed.
|
|
In such cases, the Docker-based pre-commit will inform you that you should rebuild the image.
|
|
|
|
You can also run some static code checks via `Breeze <BREEZE.rst#aout-airflow-breeze>`_ environment
|
|
using available bash scripts.
|
|
|
|
Pre-commit Hooks
|
|
----------------
|
|
|
|
Pre-commit hooks help speed up your local development cycle and place less burden on the CI infrastructure.
|
|
Consider installing the pre-commit hooks as a necessary prerequisite.
|
|
|
|
|
|
This table lists pre-commit hooks used by Airflow and indicates which hooks
|
|
require Breeze Docker images to be installed locally:
|
|
|
|
=================================== ================================================================ ============
|
|
**Hooks** **Description** **Breeze**
|
|
=================================== ================================================================ ============
|
|
``base-operator`` Checks that BaseOperator is imported properly
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``build`` Builds image for check-apache-licence, mypy, pylint, flake8. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``check-apache-license`` Checks compatibility with Apache License requirements. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``check-executables-have-shebangs`` Checks that executables have shebang.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``check-hooks-apply`` Checks which hooks are applicable to the repository.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``check-merge-conflict`` Checks if a merge conflict is committed.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``check-xml`` Checks XML files with xmllint.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``consistent-pylint`` Consistent usage of pylint enable/disable with space.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``debug-statements`` Detects accidenatally committed debug statements.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``detect-private-key`` Detects if private key is added to the repository.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``doctoc`` Refreshes the table of contents for md files.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``end-of-file-fixer`` Makes sure that there is an empty line at the end.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``flake8`` Runs flake8. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``forbid-tabs`` Fails if tabs are used in the project.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``insert-license`` Adds licenses for most file types.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``isort`` Sorts imports in python files.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``lint-dockerfile`` Lints a dockerfile.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``mixed-line-ending`` Detects if mixed line ending is used (\r vs. \r\n).
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``mypy`` Runs mypy. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``pydevd`` Check for accidentally commited pydevd statements.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``pylint`` Runs pylint for main code. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``pylint-tests`` Runs pylint for tests. *
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``python-no-log-warn`` Checks if there are no deprecate log warn.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``rst-backticks`` Checks if RST files use double backticks for code.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``setup-order`` Checks for an order of dependencies in setup.py
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``shellcheck`` Checks shell files with shellcheck.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``stylelint`` Checks CSS files with stylelint.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``update-breeze-file`` Update output of breeze command in BREEZE.rst.
|
|
----------------------------------- ---------------------------------------------------------------- ------------
|
|
``yamllint`` Checks yaml files with yamllint.
|
|
=================================== ================================================================ ============
|
|
|
|
The pre-commit hooks only check the files you are currently working on and make
|
|
them fast. Yet, these checks use exactly the same environment as the CI tests
|
|
use. So, you can be sure your modifications will also work for CI if they pass
|
|
pre-commit hooks.
|
|
|
|
We have integrated the fantastic `pre-commit <https://pre-commit.com>`__ framework
|
|
in our development workflow. To install and use it, you need Python 3.6 locally.
|
|
|
|
It is the best to use pre-commit hooks when you have your local virtualenv for
|
|
Airflow activated since then pre-commit hooks and other dependencies are
|
|
automatically installed. You can also install the pre-commit hooks manually
|
|
using ``pip install``.
|
|
|
|
The pre-commit hooks require the Docker Engine to be configured as the static
|
|
checks are executed in the Docker environment. You should build the images
|
|
locally before installing pre-commit checks as described in `BREEZE.rst <BREEZE.rst>`__.
|
|
In case you do not have your local images built, the
|
|
pre-commit hooks fail and provide instructions on what needs to be done.
|
|
|
|
Prerequisites for Pre-commit Hooks
|
|
..................................
|
|
|
|
The pre-commit hooks use several external linters that need to be installed before pre-commit is run.
|
|
|
|
Each of the checks installs its own environment, so you do not need to install those, but there are some
|
|
checks that require locally installed binaries. On Linux, you typically install
|
|
them with ``sudo apt install``, on macOS - with ``brew install``.
|
|
|
|
The current list of prerequisites is limited to ``xmllint``:
|
|
|
|
- on Linux, install via ``sudo apt install libxml2-utils``;
|
|
|
|
- on macOS, install via ``brew install libxml2``.
|
|
|
|
Enabling Pre-commit Hooks
|
|
.........................
|
|
|
|
To turn on pre-commit checks for ``commit`` operations in git, enter:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit install
|
|
|
|
|
|
To install the checks also for ``pre-push`` operations, enter:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit install -t pre-push
|
|
|
|
|
|
For details on advanced usage of the install method, use:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit install --help
|
|
|
|
|
|
Using Pre-commit Hooks
|
|
......................
|
|
|
|
After installation, pre-commit hooks are run automatically when you commit the
|
|
code. But you can run pre-commit hooks manually as needed.
|
|
|
|
- Run all checks on your staged files by using:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit run
|
|
|
|
|
|
- Run only mypy check on your staged files by using:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit run mypy
|
|
|
|
|
|
- Run only mypy checks on all files by using:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit run mypy --all-files
|
|
|
|
|
|
- Run all checks on all files by using:
|
|
|
|
.. code-block:: bash
|
|
|
|
pre-commit run --all-files
|
|
|
|
|
|
- Skip one or more of the checks by specifying a comma-separated list of
|
|
checks to skip in the SKIP variable:
|
|
|
|
.. code-block:: bash
|
|
|
|
SKIP=pylint,mypy pre-commit run --all-files
|
|
|
|
|
|
You can always skip running the tests by providing ``--no-verify`` flag to the
|
|
``git commit`` command.
|
|
|
|
To check other usage types of the pre-commit framework, see `Pre-commit website <https://pre-commit.com/>`__.
|
|
|
|
Pylint Static Code Checks
|
|
-------------------------
|
|
|
|
We are in the process of fixing the code flagged with pylint checks for the whole Airflow project.
|
|
This is a huge task so we implemented an incremental approach for the process.
|
|
Currently most of the code is excluded from pylint checks via scripts/ci/pylint_todo.txt.
|
|
We have an open JIRA issue AIRFLOW-4364 which has a number of sub-tasks for each of
|
|
the modules that should be made compatible. Fixing problems identified with pylint is one of
|
|
straightforward and easy tasks to do (but time-consuming), so if you are a first-time
|
|
contributor to Airflow, you can choose one of the sub-tasks as your first issue to fix.
|
|
|
|
To fix a pylint issue, do the following:
|
|
|
|
1. Remove module/modules from the
|
|
`scripts/ci/pylint_todo.txt <scripts/ci/pylint_todo.txt>`__.
|
|
|
|
2. Run `scripts/ci/ci_pylint_main.sh <scripts/ci/ci_pylint_main.sh>`__ and
|
|
`scripts/ci/ci_pylint_tests.sh <scripts/ci/ci_pylint_tests.sh>`__.
|
|
|
|
3. Fix all the issues reported by pylint.
|
|
|
|
4. Re-run `scripts/ci/ci_pylint_main.sh <scripts/ci/ci_pylint_main.sh>`__ and
|
|
`scripts/ci/ci_pylint_tests.sh <scripts/ci/ci_pylint_tests.sh>`__.
|
|
|
|
5. If you see "success", submit a PR following
|
|
`Pull Request guidelines <#pull-request-guidelines>`__.
|
|
|
|
|
|
These are guidelines for fixing errors reported by pylint:
|
|
|
|
- Fix the errors rather than disable pylint checks. Often you can easily
|
|
refactor the code (IntelliJ/PyCharm might be helpful when extracting methods
|
|
in complex code or moving methods around).
|
|
|
|
- If disabling a particular problem, make sure to disable only that error by
|
|
using the symbolic name of the error as reported by pylint.
|
|
|
|
.. code-block:: python
|
|
|
|
import airflow.* # pylint: disable=wildcard-import
|
|
|
|
|
|
- If there is a single line where you need to disable a particular error,
|
|
consider adding a comment to the line that causes the problem. For example:
|
|
|
|
.. code-block:: python
|
|
|
|
def MakeSummary(pcoll, metric_fn, metric_keys): # pylint: disable=invalid-name
|
|
|
|
|
|
- For multiple lines/block of code, to disable an error, you can surround the
|
|
block with ``pylint:disable/pylint:enable`` comment lines. For example:
|
|
|
|
.. code-block:: python
|
|
|
|
# pylint: disable=too-few-public-methods
|
|
class LoginForm(Form):
|
|
"""Form for the user"""
|
|
username = StringField('Username', [InputRequired()])
|
|
password = PasswordField('Password', [InputRequired()])
|
|
# pylint: enable=too-few-public-methods
|
|
|
|
|
|
Running Static Code Checks via Breeze
|
|
-------------------------------------
|
|
|
|
The static code checks can be launched using the Breeze environment.
|
|
|
|
You run the static code checks via ``./breeze static-check`` or ``./breeze static-check-all-files`` commands.
|
|
The former ones run appropriate checks only for files changed and staged locally, the latter ones
|
|
run checks on all files.
|
|
|
|
Note that it may take a lot of time to run checks for all files with pylint on macOS due to a slow
|
|
filesystem for macOS Docker. As a workaround, you can add their arguments after ``--`` as extra arguments.
|
|
You cannot pass the ``--files`` flag if you use the ``./breeze static-check-all-files`` command.
|
|
|
|
You can see the list of available static checks either via ``--help`` flag or by using the autocomplete
|
|
option. Note that the ``all`` static check runs all configured static checks. Also since pylint tests take
|
|
a lot of time, you can run a special ``all-but-pylint`` check that skips pylint checks.
|
|
|
|
Run the ``mypy`` check for the currently staged changes:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check mypy
|
|
|
|
Run the ``mypy`` check for all files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check-all-files mypy
|
|
|
|
Run the ``flake8`` check for the ``tests.core.py`` file with verbose output:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check flake8 -- --files tests/core.py --verbose
|
|
|
|
Run the ``flake8`` check for the ``tests.core`` package with verbose output:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check mypy -- --files tests/hooks/test_druid_hook.py
|
|
|
|
Run all tests for the currently staged files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check all
|
|
|
|
Run all tests for all files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check-all-files all
|
|
|
|
Run all tests but pylint for all files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check-all-files all-but-pylint
|
|
|
|
Run pylint checks for all changed files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check pylint
|
|
|
|
Run pylint checks for selected files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check pylint -- --files airflow/configuration.py
|
|
|
|
|
|
Run pylint checks for all files:
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check-all-files pylint
|
|
|
|
|
|
The ``license`` check is run via a separate script and a separate Docker image containing the
|
|
Apache RAT verification tool that checks for Apache-compatibility of licenses within the codebase.
|
|
It does not take pre-commit parameters as extra arguments.
|
|
|
|
.. code-block:: bash
|
|
|
|
./breeze static-check-all-files licenses
|
|
|
|
Running Static Code Checks via Scripts from the Host
|
|
....................................................
|
|
|
|
You can trigger the static checks from the host environment, without entering the Docker container. To do
|
|
this, run the following scripts (the same is done in Travis CI):
|
|
|
|
* `<scripts/ci/ci_check_license.sh>`_ - checks the licenses.
|
|
* `<scripts/ci/ci_docs.sh>`_ - checks that documentation can be built without warnings.
|
|
* `<scripts/ci/ci_flake8.sh>`_ - runs Flake8 source code style enforcement tool.
|
|
* `<scripts/ci/ci_lint_dockerfile.sh>`_ - runs lint checker for the dockerfiles.
|
|
* `<scripts/ci/ci_mypy.sh>`_ - runs a check for mypy type annotation consistency.
|
|
* `<scripts/ci/ci_pylint_main.sh>`_ - runs pylint static code checker for main files.
|
|
* `<scripts/ci/ci_pylint_tests.sh>`_ - runs pylint static code checker for tests.
|
|
|
|
The scripts may ask you to rebuild the images, if needed.
|
|
|
|
You can force rebuilding the images by deleting the ``.build`` directory. This directory keeps cached
|
|
information about the images already built and you can safely delete it if you want to start from scratch.
|
|
|
|
After documentation is built, the HTML results are available in the ``docs/_build/html``
|
|
folder. This folder is mounted from the host so you can access those files on your host as well.
|
|
|
|
Running Static Code Checks in the Docker Container
|
|
..................................................
|
|
|
|
If you are already in the Breeze Docker environment (by running the ``./breeze`` command),
|
|
you can also run the same static checks via run_scripts:
|
|
|
|
* Mypy: ``./scripts/ci/in_container/run_mypy.sh airflow tests``
|
|
* Pylint for main files: ``./scripts/ci/in_container/run_pylint_main.sh``
|
|
* Pylint for test files: ``./scripts/ci/in_container/run_pylint_tests.sh``
|
|
* Flake8: ``./scripts/ci/in_container/run_flake8.sh``
|
|
* License check: ``./scripts/ci/in_container/run_check_licence.sh``
|
|
* Documentation: ``./scripts/ci/in_container/run_docs_build.sh``
|
|
|
|
Running Static Code Checks for Selected Files
|
|
.............................................
|
|
|
|
In all static check scripts, both in the container and host versions, you can also pass a module/file path as
|
|
parameters of the scripts to only check selected modules or files. For example:
|
|
|
|
In the Docker container:
|
|
|
|
.. code-block::
|
|
|
|
./scripts/ci/in_container/run_pylint.sh ./airflow/example_dags/
|
|
|
|
or
|
|
|
|
.. code-block::
|
|
|
|
./scripts/ci/in_container/run_pylint.sh ./airflow/example_dags/test_utils.py
|
|
|
|
On the host:
|
|
|
|
.. code-block::
|
|
|
|
./scripts/ci/ci_pylint.sh ./airflow/example_dags/
|
|
|
|
.. code-block::
|
|
|
|
./scripts/ci/ci_pylint.sh ./airflow/example_dags/test_utils.py
|