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

Add more type hints to the code base #30503

Merged
merged 16 commits into from
Apr 7, 2023
Merged

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Apr 6, 2023

This is mainly driven by eliminating session=None usages. And since using NEW_SESSION makes many previously untyped functions become typed and checked by Mypy, a cascade of minor type hinting issues are surfaced and fixed in this PR.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:core-operators Operators, Sensors and hooks within Core Airflow provider:cncf-kubernetes Kubernetes provider related issues area:logging area:Scheduler including HA (high availability) scheduler area:secrets area:serialization area:webserver Webserver related Issues labels Apr 6, 2023
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's really useful. This will make some of my refactorings for AIP-44 show a bit more things that need fixing, but I was actually counting on those to complete the AIP-44, and that would be great if those are merged before.

@uranusjr uranusjr marked this pull request as draft April 6, 2023 12:20
@potiuk
Copy link
Member

potiuk commented Apr 6, 2023

(pending fixing the static checks/tests of course).

@uranusjr
Copy link
Member Author

uranusjr commented Apr 6, 2023

Some test failures in views. Converting to draft for now since I don’t have time to investigate at the moment.

@potiuk
Copy link
Member

potiuk commented Apr 6, 2023

Some test failures in views. Converting to draft for now since I don’t have time to investigate at the moment.

I can take a look and fix those if needs be.

@uranusjr uranusjr changed the title Add som type hints to the code base Add more type hints to the code base Apr 6, 2023
uranusjr added 4 commits April 6, 2023 21:36
Also fix a bug where create_or_update_pool silently fails when an empty
name is given. An error is raised instead now.
uranusjr added 12 commits April 6, 2023 21:36
This triggers an existing typing bug that pickle_id is incorrectly typed
as str in executors, while it should be int in practice. This is fixed
to keep things straight.
This uncovers a couple of incorrect type hints in the base
SecurityManager (in fab_security), which are also fixed.
This slightly improves how view functions are typechecked and should
prevent some trivial bugs.
@uranusjr uranusjr force-pushed the type-hint-session branch from de23977 to 219a4fa Compare April 6, 2023 13:36
@uranusjr
Copy link
Member Author

uranusjr commented Apr 6, 2023

OK seems fixed…

@uranusjr uranusjr marked this pull request as ready for review April 6, 2023 17:51
@potiuk potiuk merged commit 1a85446 into apache:main Apr 7, 2023
@uranusjr uranusjr deleted the type-hint-session branch April 10, 2023 06:25
@ephraimbuddy ephraimbuddy added the type:improvement Changelog: Improvements label Apr 11, 2023
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 area:CLI area:core-operators Operators, Sensors and hooks within Core Airflow area:logging area:Scheduler including HA (high availability) scheduler area:secrets area:serialization area:webserver Webserver related Issues provider:cncf-kubernetes Kubernetes provider related issues type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants