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 2020/01/08 09:45:05 UTC
[GitHub] [airflow] sarutak opened a new pull request #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
sarutak opened a new pull request #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100
cattrs 0.9 with Python 3.8 causes following error.
```
$ airflow
Traceback (most recent call last):
File "/Users/sarutak/airflow-env/bin/airflow", line 27, in <module>
from airflow.bin.cli import CLIFactory
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/airflow/__init__.py", line 40, in <module>
from airflow.models.dag import DAG
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/airflow/models/__init__.py", line 21, in <module>
from airflow.models.baseoperator import BaseOperator, BaseOperatorLink # noqa: F401
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/airflow/models/baseoperator.py", line 41, in <module>
from airflow.lineage import apply_lineage, prepare_lineage
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/airflow/lineage/__init__.py", line 28, in <module>
from cattr import structure, unstructure
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/cattr/__init__.py", line 2, in <module>
from .converters import Converter, UnstructureStrategy
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/cattr/converters.py", line 3, in <module>
from ._compat import (
File "/Users/sarutak/airflow-env/lib/python3.8/site-packages/cattr/_compat.py", line 86, in <module>
from typing import _Union
ImportError: cannot import name '_Union' from 'typing' (/opt/python/3.8.0/lib/python3.8/typing.py)
```
This issue is resolved in cattrs 1.0.
https://github.com/Tinche/cattrs/pull/73
So let's update it.
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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 #7100: [AIRFLOW-6508] Update
the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611370819
@kaxil -> could we merge this in rc5 as well? This is also a requirement-only change as per rc5 sqlalchemy ? I think it is pretty safe.
----------------------------------------------------------------
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] danieltahara commented on issue #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
danieltahara commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-607484310
Can you issue a patch version bump? This was merged Jan 8 but I still can't use it :(
----------------------------------------------------------------
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 #7100: [AIRFLOW-6508] Update
the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100
----------------------------------------------------------------
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] kaxil commented on issue #7100: [AIRFLOW-6508] Update the
version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611650401
To add to what Jarek said, we support Python 2 on Airflow 1.10.* branches, while we don't in Master so, makes cherry-picking process a bit harder for now but provides a good balance between development with new versions will backporting imp features.
----------------------------------------------------------------
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 #7100: [AIRFLOW-6508] Update
the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611372638
On the other hand... If this is only for python 3.8 compatibility, then maybe better to do it in 1.10.1.-> then we could add support to python 3.8 in master and test it in 1.10.11 as well. This is a major version change for cattrs, so there is a risk some obscure problems might occur.
@danieltahara -> we have not yet officially got Python 3.8 compatibility yet not even in master: https://github.com/apache/airflow/blob/master/README.md#master-version-200dev , and there is high chance other things will break with python 3.8 so I think It's better to wait for it until we officially add (and run all tests) for python 3.8. In the meantime 3.6 and 3.7 are also very good choices.
----------------------------------------------------------------
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 #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-572004796
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7100?src=pr&el=h1) Report
> Merging [#7100](https://codecov.io/gh/apache/airflow/pull/7100?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fe20ef6147f157b7c963ad46e13aff7c16458632?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7100?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7100 +/- ##
==========================================
- Coverage 85.15% 84.87% -0.29%
==========================================
Files 680 680
Lines 38824 38824
==========================================
- Hits 33061 32952 -109
- Misses 5763 5872 +109
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7100?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7100/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/7100/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/7100/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/7100/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/7100/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/contrib/operators/ssh\_operator.py](https://codecov.io/gh/apache/airflow/pull/7100/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zc2hfb3BlcmF0b3IucHk=) | `84.61% <0%> (+1.28%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7100?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/7100?src=pr&el=footer). Last update [fe20ef6...ca05904](https://codecov.io/gh/apache/airflow/pull/7100?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] danieltahara commented on issue #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
danieltahara commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611240116
How is this still not in 1.10rcs? @potiuk @feluelle
----------------------------------------------------------------
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] boring-cyborg[bot] commented on issue #7100:
[AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-572036306
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
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] danieltahara commented on issue #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
danieltahara commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611638895
Hi all,
Thanks for the responses. I think I'm finding it confusing that a commit would get merged into master and not used. If it doesn't break 3.6 and 3.7 tests (which it doesn't, since it got merged), and 3.8 is build your own adventure anyway (b/c it's not officially supported), then I don't see the harm in including it in the release.
I understand you have a very specific release branch flow, but the idea a commit can sit for 4 months on master seems like overhead for y'all (cherrypicking, mental overhead of remembering NOT to include the commit) and confusion for the public.
That said, I completely respect that outcome and really appreciate the work you do maintaining the project. Thanks!
----------------------------------------------------------------
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 edited a comment on issue #7100: [AIRFLOW-6508]
Update the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611372638
On the other hand... If this is only for python 3.8 compatibility, then maybe better to do it in 1.10.11.-> then we could add support to python 3.8 in master and test it in 1.10.11 as well. This is a major version change for cattrs, so there is a risk some obscure problems might occur.
@danieltahara -> we have not yet officially got Python 3.8 compatibility yet not even in master: https://github.com/apache/airflow/blob/master/README.md#master-version-200dev , and there is high chance other things will break with python 3.8 so I think It's better to wait for it until we officially add (and run all tests) for python 3.8. In the meantime 3.6 and 3.7 are also very good choices.
----------------------------------------------------------------
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 #7100: [AIRFLOW-6508] Update
the version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611645878
Jut to explain - The flow is only because we are now in a process of working on backwards incompatible 2.0 release but in the meantime we release bugfixes and even some features in 1.10 line. It is an overhead indeed but we have to be mindful to our users who want to still have bugs solved and some minor features added and 2.0 version is going to be a bit more difficult to upgrade for them (and it can take months or years because of corporate limitations).
And we do not have to remember of not cherry-picking BTW - we only cherry-pick what we think is worth it. Most of the changes from master branch are not cherry-picked to 1.10.
Surprisingly - it works quite well so far.
----------------------------------------------------------------
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] kaxil commented on issue #7100: [AIRFLOW-6508] Update the
version of cattrs from 0.9 to 1.0
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7100: [AIRFLOW-6508] Update the version of cattrs from 0.9 to 1.0
URL: https://github.com/apache/airflow/pull/7100#issuecomment-611412800
Yes, we don't support Python 3.8 at the moment. Last I check cattrs was still buggy for 1.0.0 & Py3.8
So it will definitely have to wait for next release.
----------------------------------------------------------------
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