You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2019/12/02 15:21:16 UTC
[GitHub] [airflow] potiuk commented on a change in pull request #6585: More
GSOD improvements
potiuk commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352654863
##########
File path: CONTRIBUTING.rst
##########
@@ -267,348 +264,34 @@ Limitations:
They are optimized for repeatability of tests, maintainability and speed of building rather
than production performance. The production images are not yet officially published.
-Pylint Checks
-=============
-
-We are in the process of fixing 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
-
-
-Pre-commit Hooks
-================
-
-Pre-commit hooks help speed up your local development cycle, either in the local virtualenv or Breeze,
-and place less burden on the CI infrastructure. Consider installing the pre-commit
-hooks as a necessary prerequisite.
-
-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:
-
-- ``xmllint``:
- on Linux, install via ``sudo apt install xmllint``;
- on macOS, install via ``brew install xmllint``
-
-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 Docker Images for Pre-commit Hooks
-----------------------------------------
-
-Before running the pre-commit hooks, you must first build the Docker images as
-described in `BREEZE.rst <BREEZE.rst>`__.
-
-Sometimes your image is outdated and needs to be rebuilt because some
-dependencies have been changed. In such case the Docker-based pre-commit will
-inform you that you should rebuild the image.
-
-Supported Pre-commit Hooks
---------------------------
-
-In Airflow, we have the following checks (The checks with stare in Breeze require `BREEZE.rst <BREEZE.rst>`__
-image built 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. *
------------------------------------ ---------------------------------------------------------------- ------------
-``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.
------------------------------------ ---------------------------------------------------------------- ------------
-``update-breeze-file`` Update output of breeze command in BREEZE.rst.
------------------------------------ ---------------------------------------------------------------- ------------
-``yamllint`` Checks yaml files with yamllint.
-=================================== ================================================================ ============
-
-
-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/>`__.
-
-Importing Airflow core objects
-==============================
-
-When you implement core features or DAGs you might need to import some of the core objects or modules.
-Since Apache Airflow can be used both as application (by internal classes) and as library (by DAGs), there are
-different ways those core objects and packages are imported.
-
-Airflow imports some of the core objects directly to 'airflow' package so that they can be used from there.
-
-Those criteria were assumed for choosing what import path to use:
-
-* If you work on a core feature inside Apache Airflow, you should import the objects directly from the
- package where the object is defined - this minimises the risk of cyclic imports.
-* If you import the objects from any of 'providers' classes, you should import the objects from
- 'airflow' or 'airflow.models', It is very important for back-porting operators/hooks/sensors
- to Airflow 1.10.* (AIP-21)
-* If you import objects from within a DAG you write, you should import them from 'airflow' or
- 'airflow.models' package where stable location of such import is important.
-
-Those checks enforced for the most important and repeated objects via pre-commit hooks as described below.
-
-BaseOperator
-------------
-
-The BaseOperator should be imported:
-* as ``from airflow.models import BaseOperator`` in external DAG/operator
-* as ``from airflow.models.baseoperator import BaseOperator`` in Airflow core to avoid cyclic imports
-
-
-Travis CI Testing Framework
-===========================
-
-Airflow test suite is based on Travis CI framework as running all of the tests
-locally requires significant setup. You can set up Travis CI in your fork of
-Airflow by following the `Travis
-CI Getting Started
-guide <https://docs.travis-ci.com/user/getting-started/>`__.
+Static code checks
+==================
+We check our code quality via static code checks. See
+`STATIC_CODE_CHECKS.rst <STATIC_CODE_CHECKS.rst>`_ for details.
-There are two different options available for running Travis CI, and they are
-set up on GitHub as separate components:
+Your code must pass all the static code checks in Travis CI in order to be eligible for Code Review.
+The easiest way to make sure your code is good before pushing is to use pre-commit checks locally
+as described in the static code checks documentation.
-- **Travis CI GitHub App** (new version)
-- **Travis CI GitHub Services** (legacy version)
+Test Infrastructure
+===================
-Using Travis CI GitHub App (new version)
-----------------------------------------
-
-- Once `installed <https://github.com/apps/travis-ci/installations/new/permissions?target_id=47426163>`__,
- configure the Travis CI GitHub App at
- `Configure Travis CI <https://github.com/settings/installations>`__.
-
-- Set repository access to either "All repositories" for convenience, or "Only
- select repositories" and choose ``USERNAME/airflow`` in the drop-down menu.
-
-- Access Travis CI for your fork at `<https://travis-ci.com/USERNAME/airflow>`__.
-
-Using Travis CI GitHub Services (legacy version)
-------------------------------------------------
-
-**NOTE:** The apache/airflow project is still using the legacy version.
+We support the following types of tests:
-Travis CI GitHub Services version uses an Authorized OAuth App.
-
-1. Once installed, configure the Travis CI Authorized OAuth App at
- `Travis CI OAuth APP <https://github.com/settings/connections/applications/88c5b97de2dbfc50f3ac>`__.
-
-2. If you are a GitHub admin, click the **Grant** button next to your
- organization; otherwise, click the **Request** button. For the Travis CI
- Authorized OAuth App, you may have to grant access to the forked
- ``ORGANIZATION/airflow`` repo even though it is public.
-
-3. Access Travis CI for your fork at
- `<https://travis-ci.org/ORGANIZATION/airflow>`_.
-
-Creating New Projects in Travis CI
-----------------------------------
+* **Unit tests** are Python ``nose`` tests launched with ``run-tests``.
Review comment:
Not yet. We will merge it after.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services