Skip to content
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

Improve modules import in Airflow core by some of them into a type-checking block #33755

Merged
merged 9 commits into from
Aug 28, 2023

Conversation

hussein-awala
Copy link
Member


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label Aug 26, 2023
@hussein-awala
Copy link
Member Author

I think it's ready, I fixed the wrong import in utils.session, and I reverted the change which update the lazy import logic in the models module.

I checked the files one by one, and all looks good.

@potiuk
Copy link
Member

potiuk commented Aug 27, 2023

This is a huge and potentially conflicting change (thought the conflicts will be easy to solve because those are only import changes. I am a little torn on this one- should we split it inte separate per "package/subpackage" maybe to limit the cherry-picking radius blast ?

@hussein-awala
Copy link
Member Author

I agree, I had the same concern, but I preferred to hear from the reviewers before making this change.

@hussein-awala hussein-awala marked this pull request as ready for review August 28, 2023 10:16
@hussein-awala
Copy link
Member Author

I think we can keep the remaining files in a single PR

@potiuk
Copy link
Member

potiuk commented Aug 28, 2023

I think we can keep the remaining files in a single PR

LGTM

@hussein-awala hussein-awala merged commit b82ce61 into apache:main Aug 28, 2023
51 checks passed
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.2 milestone Oct 3, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 3, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 29, 2023
…ecking block (#33755)

* Improve modules import in Airflow core by some of them into a type-checking block

* Restore lazy import in models and fix import in utils.sessions

* fix unit tests

* fix unit tests

* fix static checks

(cherry picked from commit b82ce61)
ephraimbuddy pushed a commit that referenced this pull request Oct 30, 2023
…ecking block (#33755)

* Improve modules import in Airflow core by some of them into a type-checking block

* Restore lazy import in models and fix import in utils.sessions

* fix unit tests

* fix unit tests

* fix static checks

(cherry picked from commit b82ce61)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants