[AIRFLOW-4227] Use python3-style type annotations. #5030

jmcarp · 2019-04-02T20:19:09Z

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-4227
- 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.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

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

BasPH · 2019-04-02T21:25:25Z

run_unit_tests.sh

@@ -34,9 +34,9 @@ nose_args=$@
 # Generate the `airflow` executable if needed
 which airflow > /dev/null || python setup.py develop

-echo "Initializing the DB"


BasPH · 2019-04-02T21:29:01Z

Note: there is also this issue https://issues.apache.org/jira/browse/AIRFLOW-4205, part of a collection of issues on removing Python 2.

jmcarp · 2019-04-03T02:48:52Z

Thanks @BasPH. Also, will we support python3.5 in the future? If so, we can use function annotations but not variable annotations.

ashb · 2019-04-03T09:08:02Z

We should probably support Python 3.5.2 which is what Ubuntu ships

jmcarp · 2019-04-03T18:28:20Z

Thanks @ashb, I backed out variable annotations, and tests are passing on 3.5.x.

mik-laj · 2019-04-10T06:17:24Z

What do you think about replacing collections.namedtuple with typing.NamedTuple?
https://docs.python.org/3/library/typing.html#typing.NamedTuple

jmcarp · 2019-04-10T22:39:18Z

Added some typed namedtuples. Since we're supporting python3.5, we can't use variable annotations, so I used the backwards-compatible syntax.

Fokko · 2019-04-12T07:56:17Z

Can you rebase onto master?

jmcarp · 2019-04-12T16:06:33Z

Looks like #5090 will fix the kubernetes tests here once it's ready.

codecov-io · 2019-04-20T20:20:02Z

Codecov Report

Merging #5030 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5030      +/-   ##
==========================================
+ Coverage   78.67%   78.68%   +<.01%     
==========================================
  Files         470      470              
  Lines       30013    30020       +7     
==========================================
+ Hits        23613    23620       +7     
  Misses       6400     6400

Impacted Files	Coverage Δ
airflow/operators/check_operator.py	`92.64% <ø> (ø)`	⬆️
airflow/models/baseoperator.py	`93.92% <ø> (ø)`	⬆️
airflow/models/variable.py	`94.73% <ø> (ø)`	⬆️
airflow/models/dag.py	`93.51% <ø> (ø)`	⬆️
airflow/operators/python_operator.py	`95.8% <ø> (ø)`	⬆️
.../kubernetes_request_factory/pod_request_factory.py	`100% <100%> (ø)`	⬆️
airflow/plugins_manager.py	`86.79% <100%> (ø)`	⬆️
airflow/models/dagbag.py	`92.27% <100%> (ø)`	⬆️
airflow/hooks/base_hook.py	`92.15% <100%> (+0.49%)`	⬆️
airflow/contrib/utils/gcp_field_sanitizer.py	`91.17% <100%> (ø)`	⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24b9c7c...192f3f9. Read the comment docs.

jmcarp · 2019-04-24T04:39:46Z

This should be ready to go now that #5090 is finished. cc @Fokko @kaxil

Fokko

Looking good @jmcarp

One more comment, apart from that LGTM. Makes the code much more readable.

Fokko · 2019-04-30T09:21:20Z

tests/plugins/test_plugin.py

@@ -99,7 +99,7 @@ class AirflowTestPlugin(AirflowPlugin):
    flask_blueprints = [bp]
    appbuilder_views = [v_appbuilder_package]
    appbuilder_menu_items = [appbuilder_mitem]
-    stat_name_handler = staticmethod(stat_name_dummy_handler)
+    stat_name_handler = stat_name_dummy_handler


This change seems unrelated?

It could be because mypy was complaining about this otherwise?

That's right @ashb, and mypy might be right to complain about staticmethod here, since stat_name_dummy_handler is a function and not a method, and already has the expected signature.

feng-tao · 2019-05-02T04:21:30Z

We could revisit this pr given we drop PY2?

mik-laj · 2019-05-06T00:03:45Z

02:01 $ grep -RE 'namedtuple\(' . --include \*.py
./tests/models/test_connection.py:ConnectionParts = namedtuple("ConnectionParts", ["conn_type", "login", "password", "host", "port", "schema"])
./tests/contrib/hooks/test_azure_container_instance_hook.py:        named_instance = namedtuple("InstanceView", instance_view.keys())(*instance_view.values())
./tests/contrib/hooks/test_azure_container_instance_hook.py:        named_instance = namedtuple("Events", instance_view.keys())(*instance_view.values())
./tests/contrib/hooks/test_wasb_hook.py:        Blob = namedtuple('Blob', ['name'])

It seems to me that all classes have not been changed. WDYT?

jmcarp · 2019-05-07T17:37:35Z

@mik-laj: I hadn't changed all the named tuples in tests, but now I've gone through and dropped all uses of collections.namedtuple. Thanks for checking.

jmcarp · 2019-06-03T04:04:58Z

Looks like this was reimplemented in #5327. Thanks @BasPH.

jmcarp force-pushed the py3-type-annotations branch from de12238 to 00b663b Compare April 2, 2019 20:43

BasPH reviewed Apr 2, 2019

View reviewed changes

jmcarp force-pushed the py3-type-annotations branch from 00b663b to 26fbbd1 Compare April 3, 2019 00:22

feng-tao added the defer-until-v1-10-3 label Apr 3, 2019

jmcarp force-pushed the py3-type-annotations branch 2 times, most recently from 3741b1c to 59f0a90 Compare April 3, 2019 16:25

jmcarp force-pushed the py3-type-annotations branch from 59f0a90 to 532c6fa Compare April 10, 2019 03:49

jmcarp force-pushed the py3-type-annotations branch from 532c6fa to 2344d4c Compare April 10, 2019 20:30

jmcarp force-pushed the py3-type-annotations branch 2 times, most recently from a0fa71a to 942d40c Compare April 12, 2019 15:13

jmcarp force-pushed the py3-type-annotations branch from 942d40c to 8471971 Compare April 20, 2019 19:39

kaxil removed defer-until-v1-10-3 labels Apr 22, 2019

Fokko reviewed Apr 30, 2019

View reviewed changes

kaxil mentioned this pull request Apr 30, 2019

[AIRFLOW-4343] Show warning in UI if scheduler is not running #5127

Merged

6 tasks

jmcarp force-pushed the py3-type-annotations branch 2 times, most recently from 2ccd6aa to d34804e Compare May 5, 2019 14:24

jmcarp force-pushed the py3-type-annotations branch from d34804e to dc0de57 Compare May 7, 2019 02:52

jmcarp force-pushed the py3-type-annotations branch 2 times, most recently from 736b616 to 7491815 Compare May 7, 2019 03:59

[AIRFLOW-4227] Use python3-style type annotations.

192f3f9

jmcarp force-pushed the py3-type-annotations branch from 7491815 to 192f3f9 Compare May 7, 2019 04:03

jmcarp closed this Jun 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-4227] Use python3-style type annotations. #5030

[AIRFLOW-4227] Use python3-style type annotations. #5030

jmcarp commented Apr 2, 2019

BasPH Apr 2, 2019

BasPH commented Apr 2, 2019

jmcarp commented Apr 3, 2019

ashb commented Apr 3, 2019

jmcarp commented Apr 3, 2019

mik-laj commented Apr 10, 2019 •

edited

Loading

jmcarp commented Apr 10, 2019

Fokko commented Apr 12, 2019

jmcarp commented Apr 12, 2019 •

edited

Loading

codecov-io commented Apr 20, 2019 •

edited

Loading

jmcarp commented Apr 24, 2019

Fokko left a comment

Fokko Apr 30, 2019

ashb Apr 30, 2019

jmcarp May 5, 2019

feng-tao commented May 2, 2019

mik-laj commented May 6, 2019

jmcarp commented May 7, 2019

jmcarp commented Jun 3, 2019

[AIRFLOW-4227] Use python3-style type annotations. #5030

[AIRFLOW-4227] Use python3-style type annotations. #5030

Conversation

jmcarp commented Apr 2, 2019

Jira

Description

Tests

Commits

Documentation

Code Quality

BasPH Apr 2, 2019

Choose a reason for hiding this comment

BasPH commented Apr 2, 2019

jmcarp commented Apr 3, 2019

ashb commented Apr 3, 2019

jmcarp commented Apr 3, 2019

mik-laj commented Apr 10, 2019 • edited Loading

jmcarp commented Apr 10, 2019

Fokko commented Apr 12, 2019

jmcarp commented Apr 12, 2019 • edited Loading

codecov-io commented Apr 20, 2019 • edited Loading

Codecov Report

jmcarp commented Apr 24, 2019

Fokko left a comment

Choose a reason for hiding this comment

Fokko Apr 30, 2019

Choose a reason for hiding this comment

ashb Apr 30, 2019

Choose a reason for hiding this comment

jmcarp May 5, 2019

Choose a reason for hiding this comment

feng-tao commented May 2, 2019

mik-laj commented May 6, 2019

jmcarp commented May 7, 2019

jmcarp commented Jun 3, 2019

mik-laj commented Apr 10, 2019 •

edited

Loading

jmcarp commented Apr 12, 2019 •

edited

Loading

codecov-io commented Apr 20, 2019 •

edited

Loading