[AIRFLOW-3695] Replace Flake8 by Pylint

[BasPH]

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-3695
  • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.

Description

• Here are some details about my PR, including screenshots of any UI changes:

Following the dev mailing list about AIP-6, I replaced Flake8 by Pylint. I choose Pylint because it's a widely used tool, very configurable and is already used here and there in Airflow itself.

On top of the default Pylint configuration (see .pylintrc), I set some sensible defaults:

disable=comprehension-escape,
        pointless-statement,
        invalid-name,
        duplicate-code,
        abstract-method,
        bad-continuation

ignored-argument-names=_.*|^ignored_|^unused_|args|kwargs (added args & kwargs)

max-args=15

min-public-methods=1

Pylint is much stricter than Flake8 and I'm sure it will lead to some discussion about the configuration in the beginning. However I'm convinced in the long run it will lead to much more consistent code. This PR adds and applies Pylint only on changed lines compared to master with the help of diff-cover. I see it as an intermediate solution, until the entire Airflow codebase is Pylint-compliant and we can simply run Pylint on complete changed files.

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
  • All the public functions and the classes in the PR contain docstrings that explain what it does

Code Quality

• Passes Pylint

[feluelle]

Hey @BasPH, I like your proposal to use pylint to have cleaner code because like you said pylint has more strict rules by default but I actually would be even more strict in that sense that we would add it instead of replacing it and use both flake8 + pylint because flake8 has some checks that even pylint does not have I believe. At work we use both for our custom airflow plugins and it is working great.

[BasPH]

Whoops Pylint made the sensible choice to drop Python2 support with Pylint2 which I'm installing and now the tests are failing. Will check tonight.

[BasPH]

@feluelle I thought Flake8 checks only PEP8 + logic, and PyLint does that too + code smells + more. Will investigate more how they overlap tonight.

[feluelle]

@BasPH Thank you. Maybe I am wrong and we only need pylint. That would be great to know :)

[feng-tao]

I think we should send out an email with a title [VOTE] to get three binding votes to close the loop for this AIP. But I am +1 on having this.

[diraol]

Just adding here that it would be good to check Prospector too.

It seems to "replace" both pylint and flake8 and also add some other features. Actually it is more an integration testing tool than a replacement, but still worth looking. =)

[max-sixty]

I haven't been involved as an airflow developer, but saw this on the mailing list and thought it would be helpful to comment given my experience with code formatters on half-a-dozen python projects:

I've generally had negative experiences with pylint, and I'm not aware of projects having much success with it, relative to flake8 or increasingly black (which is a larger change, and doesn't operate on diffs).

Are there any large python projects using pylint?
Projects successfully using flake8 include pandas, sqlalchemy, xarray, django

[BasPH]

Closed issue in favour of #5238. We will apply both Flake8 and Pylint in the near future.

[potiuk]

Yep. Good idea to keep both.. Both have slightly different scope and can be configured to work in parallel.