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/20 00:08:31 UTC
[GitHub] [airflow] xinbinhuang opened a new pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
xinbinhuang opened a new pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860
Make sure you have checked _all_ steps below.
### Jira
- [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
- https://issues.apache.org/jira/projects/AIRFLOW/issues/AIRFLOW-6311
### Description
- Remove workaround for python2 `newstr` from settings.py
- Change default python interpreter for dataflow to python3, given that support is mostly finished. and they are also about to sunset python2 in 2020, ( thought the timeline has not been set yet)
- Dataflow python3 support roadmap: https://issues.apache.org/jira/browse/BEAM-1251#comment-16979381]
- Python2 sunset schedule: https://issues.apache.org/jira/browse/BEAM-8371
- Apache-beam sign (without detailed timeline) on https://python3statement.org/
- It depends on when will this be merged in. We may also drop the `py_interpreter` all together
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang commented on issue #6860: [AIRFLOW-6311]
Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568197295
Thanks! @potiuk
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#discussion_r360350915
##########
File path: airflow/gcp/operators/dataflow.py
##########
@@ -393,7 +393,7 @@ def __init__(
py_options: Optional[List[str]] = None,
dataflow_default_options: Optional[dict] = None,
options: Optional[dict] = None,
- py_interpreter: str = "python2",
+ py_interpreter: str = "python3",
Review comment:
This should be done in a separate PR (to create an entry in the changelog) and we should add a note in the UPDATING.MD file.
https://github.com/apache/airflow/pull/5602
Here is the discussion where changing this parameter to Python3 was discussed, although now the situation may be different.
@jaketf @lgajowy WDYT?
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang commented on a change in pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#discussion_r360506409
##########
File path: airflow/gcp/operators/dataflow.py
##########
@@ -393,7 +393,7 @@ def __init__(
py_options: Optional[List[str]] = None,
dataflow_default_options: Optional[dict] = None,
options: Optional[dict] = None,
- py_interpreter: str = "python2",
+ py_interpreter: str = "python3",
Review comment:
I will create a separate PR to adjust the change in Dataflow, and update the changelog
----------------------------------------------------------------
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
[GitHub] [airflow] ashb commented on issue #6860: [AIRFLOW-6311] Remove
python2 from codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-567903723
This is mostly about changing the default interpreter in GCP operators more than anything (as we've already removed most of the Py2 specific code elsewhere) so this PR title would be better updated to reflect that (PR titles go in to the changelog)
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#discussion_r360350915
##########
File path: airflow/gcp/operators/dataflow.py
##########
@@ -393,7 +393,7 @@ def __init__(
py_options: Optional[List[str]] = None,
dataflow_default_options: Optional[dict] = None,
options: Optional[dict] = None,
- py_interpreter: str = "python2",
+ py_interpreter: str = "python3",
Review comment:
This should be done in a separate PR (to create an entry in the changelog) and we should add a note in the UPDATING.MD file.
https://github.com/apache/airflow/pull/5602
Here is the discussion where changing this parameter to Python3 was discussed, although now the situation may be different.
@jaketf @lgajowy @kkucharc WDYT?
----------------------------------------------------------------
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
[GitHub] [airflow] ashb commented on a change in pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#discussion_r360347142
##########
File path: airflow/gcp/operators/dataflow.py
##########
@@ -393,7 +393,7 @@ def __init__(
py_options: Optional[List[str]] = None,
dataflow_default_options: Optional[dict] = None,
options: Optional[dict] = None,
- py_interpreter: str = "python2",
+ py_interpreter: str = "python3",
Review comment:
I don't disagree with this change, but this is stretching the definition of "in this code base". The entirety of Airflow could be Py3 only and this could stay as python2 and it would work.
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang commented on issue #6860: [AIRFLOW-6311]
Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568042680
Rebased & Remove changes for Dataflow. Will create a separate PR later and link to this one
----------------------------------------------------------------
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
[GitHub] [airflow] codecov-io commented on issue #6860: [AIRFLOW-6311]
Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568168933
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=h1) Report
> Merging [#6860](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/b7c11e449e528f2f8b6f030160d3eefd643e6c2f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6860 +/- ##
==========================================
- Coverage 84.56% 84.28% -0.29%
==========================================
Files 680 680
Lines 38454 38453 -1
==========================================
- Hits 32520 32409 -111
- Misses 5934 6044 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==) | `93.66% <ø> (-0.05%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6860/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=footer). Last update [b7c11e4...5c76741](https://codecov.io/gh/apache/airflow/pull/6860?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [airflow] potiuk merged pull request #6860: [AIRFLOW-6311] Remove
python2 from codebase
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang edited a comment on issue #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang edited a comment on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568024770
> some unit tests are failing
Sorry, I was making the changes on another PC that I thought have pre-commit hooks installed while did not. I will double-check that.
> This is mostly about changing the default interpreter in GCP operators more than anything (as we've already removed most of the Py2 specific code elsewhere) so this PR title would be better updated to reflect that (PR titles go in to the changelog)
@potiuk @ashb I agree with your opinions. I will create another PR to just include the dataflow changes. while this remains simple as removing those 2lines in the `settings.py`
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang commented on a change in pull request #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#discussion_r361822543
##########
File path: airflow/gcp/operators/dataflow.py
##########
@@ -393,7 +393,7 @@ def __init__(
py_options: Optional[List[str]] = None,
dataflow_default_options: Optional[dict] = None,
options: Optional[dict] = None,
- py_interpreter: str = "python2",
+ py_interpreter: str = "python3",
Review comment:
See the separate PR #6945 https://github.com/apache/airflow/pull/6945
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang commented on issue #6860: [AIRFLOW-6311]
Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568024770
> This is mostly about changing the default interpreter in GCP operators more than anything (as we've already removed most of the Py2 specific code elsewhere) so this PR title would be better updated to reflect that (PR titles go in to the changelog)
@potiuk @ashb I agree with your opinions. I will create another PR to just include the dataflow changes. while this remains simple as removing those 2lines in the `settings.py`
----------------------------------------------------------------
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
[GitHub] [airflow] potiuk commented on issue #6860: [AIRFLOW-6311] Remove
python2 from codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568181556
I merged and updated the commit subject slightly.
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang edited a comment on issue #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang edited a comment on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568024770
> some unit tests are failing
Sorry, I was making the changes on another PC that I thought have pre-commit hooks installed while did not. I will double-check that.
> This is mostly about changing the default interpreter in GCP operators more than anything (as we've already removed most of the Py2 specific code elsewhere) so this PR title would be better updated to reflect that (PR titles go in to the changelog)
@potiuk @ashb I agree with your opinions. I will create another PR to just include the dataflow changes. while this remains simple as removing those 2lines in the `settings.py`
----------------------------------------------------------------
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
[GitHub] [airflow] xinbinhuang edited a comment on issue #6860:
[AIRFLOW-6311] Remove python2 from codebase
Posted by GitBox <gi...@apache.org>.
xinbinhuang edited a comment on issue #6860: [AIRFLOW-6311] Remove python2 from codebase
URL: https://github.com/apache/airflow/pull/6860#issuecomment-568024770
> some unit tests are failing
Sorry,~I was making the changes on another PC that I thought have pre-commit hooks installed while did not~ . I will double-check that.
> This is mostly about changing the default interpreter in GCP operators more than anything (as we've already removed most of the Py2 specific code elsewhere) so this PR title would be better updated to reflect that (PR titles go in to the changelog)
@potiuk @ashb I agree with your opinions. I will create another PR to just include the dataflow changes. while this remains simple as removing those 2lines in the `settings.py`
----------------------------------------------------------------
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