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 2022/09/10 14:43:59 UTC
[GitHub] [airflow] potiuk opened a new pull request, #26290: Convert airflow/api to __future__.annotations
potiuk opened a new pull request, #26290:
URL: https://github.com/apache/airflow/pull/26290
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #26290: Apply PEP-563 (Postponed Evaluation of Annotations) to core airflow
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26290:
URL: https://github.com/apache/airflow/pull/26290#issuecomment-1246325183
The error looks like python-attrs/cattrs#279. Translation: It does not work, no way around 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #26290: Apply PEP-563 (Postponed Evaluation of Annotations) to core airflow
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26290:
URL: https://github.com/apache/airflow/pull/26290
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #26290: Apply PEP-563 (Postponed Evaluation of Annotations) to core airflow
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26290:
URL: https://github.com/apache/airflow/pull/26290#issuecomment-1246014807
Hey @ashb @uranusjr (also @kaxil - I know you were looking at cattrs/atts before) -> I think, before merging that one I need one more opinion/comment/advice. I have not merged it yet as I was trying to figure out why we had exactly ONE test failing after I merged all the PRs.
I had this one test failing after I merged all the PEP-563 changes (strangely enough it was not failing in the PR where "baseoperator" changes were separated into a separate PR).
Example failure here: https://github.com/apache/airflow/actions/runs/3047099374/jobs/4911094396#step:9:8804
`airflow.exceptions.SerializationError: Failed to serialize DAG 'test-bigquery-operators': 'type' object is not subscriptable`
I did some bisecting and pin-pointed the exact line which is the culprit:
https://github.com/apache/airflow/pull/26290/files#diff-848f325ace55b3504e8052fecdb53c0f295c891b67a6d90e9341cbe79cc545fbL1748
It is super-reproducible with :
```
pytest tests/serialization/test_dag_serialization.py::TestStringifiedDAGs::test_extra_serialized_field_and_operator_links
```
It's as triggered by replacing those lines:
```
@attr.s(auto_attribs=True)
class BaseOperatorLink(metaclass=ABCMeta):
"""Abstract base class that defines how we get an operator link."""
operators: ClassVar[List[Type[BaseOperator]]] = []
```
with those:
```
@attr.s(auto_attribs=True)
class BaseOperatorLink(metaclass=ABCMeta):
"""Abstract base class that defines how we get an operator link."""
operators: ClassVar[list[type[BaseOperator]]] = []
```
This is what `pyupgrade` does. All other changes in "baseoperator" are fine (and I left them in in the current version).
Basically seems that changing from the "classic" typing to PEP-563 breaks some of the "attrs" magic in this SINGLE case.
I looked for similar issues and it seems very similar to https://github.com/python-attrs/cattrs/issues/80, but it's not the same. And it does look like `attrs` is not able to handle the type correctly, when it is defined with PEP-563.
For now I just reverted that single line (and List/Type) from PyUpgrade and added the baseoperator to be skipped from pyugprade. The test is passing locally.
But I wonder If this is something that you might have a better idea how to solve ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #26290: Apply PEP-563 (Postponed Evaluation of Annotations) to core airflow
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26290:
URL: https://github.com/apache/airflow/pull/26290#issuecomment-1246324016
BTW. Tests are passing. I will merge that one (with better commend in pyupgrade disable) and I think we can deal with this afterwards.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org