[AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes #5786

potiuk · 2019-08-12T14:24:07Z

This is the first set of new static checks to add (for python files)

As described in another PR - I split it per file type. There will be about 7 or 8 such changes.

This one is by far the biggest because it is for python files:

It makes licence files consistent accross all python files (there were subtle differences, mistakes partially copied licenses etc). Now we have single LICENSE.txt file from which we automatically add licence to python file if it is missing. The same LICENSE.txt file will be used for other file types.
It adds coding pragma for all files if they are missing
It checks if there are no debug statements left in python code
It fixes pylint errors in all the files touched by either LICENSE.txt change or pragma - this is actually what causes so many changes (but only in files that were changed because one of the two other changes: pragma/license)

We also have a script that can refresh pylint_todo.txt (used in this case)

There are quite a number of pylint fixes but they were all planned anyway.

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-5170

Description

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

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.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

Passes flake8

potiuk · 2019-08-12T14:26:00Z

@dimberman @BasPH @mik-laj -> next step after adding pre-commit checks. This check adds automatically enccoding pragma for all .py files and fixes all related pylint problems.

@BasPH -> I also added a script to refresh pylint_todo.txt automatically (and remove all files that have been fixed so far)

potiuk · 2019-08-15T04:28:36Z

@ashb @dimberman @Fokko -> this is the first additional set of checks (for python files) added after merging the pylint/mypy/flake checks in pre-commit. It will make our python code much more consistent (and fixes/disables a lot of pylint errors). We also have a script that can refresh pylint_todo.txt

airflow/contrib/hooks/bigquery_hook.py

airflow/contrib/hooks/grpc_hook.py

airflow/contrib/operators/aws_sqs_publish_operator.py

tests/kubernetes/kubernetes_request_factory/test_pod_request_factory.py

potiuk · 2019-08-16T02:17:26Z

All reported so far resolved.

feluelle

@potiuk Nice work as always. If you have time please look over my comments. :)

.pre-commit-config.yaml

CONTRIBUTING.md

airflow/contrib/hooks/databricks_hook.py

airflow/contrib/hooks/qubole_hook.py

dev/airflow-jira

tests/contrib/utils/gcp_authenticator.py

tests/kubernetes/kubernetes_request_factory/test_pod_request_factory.py

tests/operators/test_docker_operator.py

tests/utils/log/elasticmock/__init__.py

potiuk · 2019-09-11T22:02:52Z

Anyone :)

airflow/kubernetes/volume_mount.py

docs/exts/exampleinclude.py

scripts/ci/_utils.sh

scripts/ci/in_container/_in_container_utils.sh

tests/utils/log/elasticmock/fake_elasticsearch.py

potiuk · 2019-09-12T16:26:31Z

Should be all fixed now @feluelle

feluelle

Sorry, I missed some..

After this is done and green you can merge it, imo :)

airflow/kubernetes/volume.py

potiuk · 2019-09-16T00:36:48Z

@feluelle -> I think it will be green soon:)

feluelle · 2019-09-16T08:42:14Z

What happened @potiuk ? There are far less changes in the commit than before.

potiuk · 2019-09-16T15:19:39Z

Ups. I will take a look . Luckily Git does not forget things too easily

potiuk · 2019-09-16T20:16:17Z

@feluelle - reflog to the rescue. All should be good now :). Waiting for Travis...

potiuk · 2019-09-16T22:26:04Z

All green @feluelle !

feluelle

@potiuk I unresolved the conversations where I think you missed a commit.

…iles

potiuk · 2019-09-17T09:55:17Z

Right. I thought I squashed it together :(.

feluelle

We made it @potiuk ! 🎉

potiuk · 2019-09-17T12:30:16Z

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

…iles (#5786)

…iles (#5786) (cherry picked from commit 4780105)

potiuk requested review from dimberman, BasPH and mik-laj August 12, 2019 14:24

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch 3 times, most recently from 2ad7517 to f8761b6 Compare August 13, 2019 11:28

potiuk changed the title ~~[AIRFLOW-5170] Fix encoding pragma and related pylint fixes (depends on AIRFLOW-5161]~~ [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes Aug 15, 2019

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch 2 times, most recently from 54ae759 to 2cd9734 Compare August 15, 2019 04:00

potiuk requested review from ashb and Fokko August 15, 2019 04:15

Fokko reviewed Aug 15, 2019

View reviewed changes

airflow/contrib/hooks/bigquery_hook.py Outdated Show resolved Hide resolved

Fokko reviewed Aug 15, 2019

View reviewed changes

airflow/contrib/hooks/grpc_hook.py Outdated Show resolved Hide resolved

mik-laj reviewed Aug 15, 2019

View reviewed changes

airflow/contrib/operators/aws_sqs_publish_operator.py Outdated Show resolved Hide resolved

mik-laj reviewed Aug 15, 2019

View reviewed changes

tests/kubernetes/kubernetes_request_factory/test_pod_request_factory.py Outdated Show resolved Hide resolved

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from 2cd9734 to e2b016c Compare August 16, 2019 01:38

mik-laj added the area:dev-env CI, pre-commit, pylint and other changes that do not change the behavior of the final code label Aug 16, 2019

feluelle requested changes Aug 16, 2019

View reviewed changes

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch 6 times, most recently from 29bc9cc to c4a40c1 Compare August 18, 2019 23:53

potiuk changed the title ~~[AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes~~ [AIRFLOW-5170] Consistent licences for python files and related pylint fixes Aug 19, 2019

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from c4a40c1 to 55a3963 Compare August 19, 2019 13:17

potiuk changed the title ~~[AIRFLOW-5170] Consistent licences for python files and related pylint fixes~~ [AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes Aug 19, 2019

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from cd57c40 to 7059f9f Compare September 11, 2019 21:59

feluelle requested changes Sep 12, 2019

View reviewed changes

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from e299e12 to 6f01e2b Compare September 12, 2019 16:26

feluelle requested changes Sep 12, 2019

View reviewed changes

airflow/kubernetes/volume.py Show resolved Hide resolved

airflow/kubernetes/volume.py Outdated Show resolved Hide resolved

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from 6f01e2b to 7923879 Compare September 16, 2019 00:33

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from 7923879 to db4526b Compare September 16, 2019 20:14

feluelle requested changes Sep 17, 2019

View reviewed changes

[AIRFLOW-5256] Related pylint changes for common licences in python f…

eee6687

…iles

potiuk force-pushed the add-encoding-pragma-hook-with-pylint-fixes branch from db4526b to eee6687 Compare September 17, 2019 09:54

feluelle approved these changes Sep 17, 2019

View reviewed changes

feluelle merged commit 4780105 into apache:master Sep 17, 2019

potiuk added a commit that referenced this pull request Sep 17, 2019

[AIRFLOW-5256] Related pylint changes for common licences in python f…

b037ba8

…iles (#5786)

potiuk added a commit that referenced this pull request Sep 17, 2019

[AIRFLOW-5256] Related pylint changes for common licences in python f…

1043c85

…iles (#5786) (cherry picked from commit 4780105)

potiuk added a commit that referenced this pull request Sep 18, 2019

[AIRFLOW-5256] Related pylint changes for common licences in python f…

f8be014

…iles (#5786) (cherry picked from commit 4780105)

potiuk deleted the add-encoding-pragma-hook-with-pylint-fixes branch September 19, 2019 22:43

ashb pushed a commit that referenced this pull request Sep 24, 2019

[AIRFLOW-5256] Related pylint changes for common licences in python f…

5cba7db

…iles (#5786) (cherry picked from commit 4780105)

egibbm mentioned this pull request Jun 17, 2020

[AIRFLOW-9347] Fix QuboleHook unable to add list to tags #9349

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes #5786

[AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes #5786

potiuk commented Aug 12, 2019 •

edited

Loading

potiuk commented Aug 12, 2019

potiuk commented Aug 15, 2019

potiuk commented Aug 16, 2019

feluelle left a comment

potiuk commented Sep 11, 2019

potiuk commented Sep 12, 2019

feluelle left a comment

potiuk commented Sep 16, 2019

feluelle commented Sep 16, 2019

potiuk commented Sep 16, 2019

potiuk commented Sep 16, 2019

potiuk commented Sep 16, 2019

feluelle left a comment

potiuk commented Sep 17, 2019

feluelle left a comment

potiuk commented Sep 17, 2019

[AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes #5786

[AIRFLOW-5170] [AIRFLOW-5256] Consistent licences for python files and related pylint fixes #5786

Conversation

potiuk commented Aug 12, 2019 • edited Loading

Jira

Description

Tests

Commits

Documentation

Code Quality

potiuk commented Aug 12, 2019

potiuk commented Aug 15, 2019

potiuk commented Aug 16, 2019

feluelle left a comment

Choose a reason for hiding this comment

potiuk commented Sep 11, 2019

potiuk commented Sep 12, 2019

feluelle left a comment

Choose a reason for hiding this comment

potiuk commented Sep 16, 2019

feluelle commented Sep 16, 2019

potiuk commented Sep 16, 2019

potiuk commented Sep 16, 2019

potiuk commented Sep 16, 2019

feluelle left a comment

Choose a reason for hiding this comment

potiuk commented Sep 17, 2019

feluelle left a comment

Choose a reason for hiding this comment

potiuk commented Sep 17, 2019

potiuk commented Aug 12, 2019 •

edited

Loading