-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Clean up top-level SDK imports in Core #59817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f7e34b6 to
60888ea
Compare
18c153f to
8bab3e1
Compare
There are still quite several left, mostly in CLI things. These are the easier ones.
8bab3e1 to
5580df1
Compare
|
Should not we use "compat" also for type checking? I think not having compat will make mypy and other typing checks confused if our users will use mypy for checking their dags in Airflow 2 - we had a number of issues in the past from our users when they had some problems with it. As far as I understand how MyPy and other type checkers work, is that they actually parse even installed packages - so if someone installs our provider and runs mypy checks on their code using that provider, the So for me - the decision to not use compat for TYPE_CHECKING branches has some negative effects for Airlfow 2 users - and I don't see what we are going to gain by doing it (We will have to anyhow convert those into sdk imports in bulk when we go to apache-airflow>=3.1.0 state for providers). |
|
Type checking can have compatibility, the specific pattern I changed here does not affect that. |
Agreed! Indeed if someone uses Context from one of those, they deserve to have an error that they should fix :) |
Some more cleanups
airflow.utils.contextmodule has been cleaned up. Old names are redirected toairflow.sdkwith a warning.from airflow.utils.context import Contextfor typing (mostly in providers) are all unconditionally changed tofrom airflow.sdk import Contextsince they should only matter for our Mypy checks in CI.Contextin provider tests.Most of the Core modules are now SDK-free (aside from typing). Exceptions are
The two are on old as mentioned in #52141. I plan to tackle the third next. That’s a bit complicated.