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 2023/01/10 21:01:17 UTC
[GitHub] [airflow] josh-fell opened a new pull request, #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
josh-fell opened a new pull request, #28842:
URL: https://github.com/apache/airflow/pull/28842
Now that all providers have a minimum Airflow version of 2.3.0, importing `airflow.utils.context.Context` no longer needs to be done behind TYPE_CHECKING. The `Context` module was made available starting with Airflow 2.2.3.
There are also a few instances which the `context` typing in `execute()` methods was added or updated to reflect the proper typing as well.
Happy to break this up if it's easier to review. Let me know!
<!--
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] josh-fell commented on pull request #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on PR #28842:
URL: https://github.com/apache/airflow/pull/28842#issuecomment-1377908220
@potiuk I noticed that the Common SQL provider doesn't specify having a minimum Airflow version in its `provider.yaml` file, but I went ahead and pushed some updates anyway. Let me know if you those should be removed and I'll back them out. (Or if the `provider.yaml` file should be updated with the Airflow dependency I can whip that up too).
--
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] josh-fell commented on pull request #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on PR #28842:
URL: https://github.com/apache/airflow/pull/28842#issuecomment-1382023560
> Hmmmm, so the advantage of keeping it behind TYPE_CHECKING is that it's one less import at runtime.
>
> In practice this will already be imported, but still, it's an import we don't need to do, so I'd say we keep the TYPE_CHECKING if the only reason for the import is a type annotation
That's a very valid point and thinking about it, I agree. Not sure why it didn't cross my mind. Been out of practice from the last 3 months of leave. But, you have inspired another idea.
I'll close this monster.
--
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] josh-fell closed pull request #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
Posted by GitBox <gi...@apache.org>.
josh-fell closed pull request #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
URL: https://github.com/apache/airflow/pull/28842
--
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 #28842: Remove TYPE_CHECKING for `airflow.utils.context.Context` in providers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28842:
URL: https://github.com/apache/airflow/pull/28842#issuecomment-1378051698
Yes. you can add the same limits as for the other providers. I think common.sql was added around the same time when we added the min-version and that's why it did not have the limit set. It did not really matter, because common.sql is only ever used by other providers (and the limit on airlfow version would be used freom those providers)- but it can be added without any problems/changes/.
--
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