-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Fix primitive parameter type handling in dynamic CLI generation #52768
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
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for your first PR @mohiuddin-khan-shiam! Amazing work! The Boolean part was in my TODO as a bug. I have tested and it works. It is amazing to see you fixed the bug and you also enhanced/streamlined the mapping parsing.
It looks great! Could you please rebase your branch? We can merge afterwards
Done. |
Thanks for the rebase, but I think you merged the changes rather than rebase! Could you please also run static checks (pre-commit hooks) in your localand push? it is failing on the CI for ruff format and if we merge, it will break main. :) Sorry for double work! |
8e3a4c7
to
a8829dc
Compare
Thanks for the review! I've:
The PR should now be clean and ready for another look. Let me know if you need anything else! |
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
Congrats @mohiuddin-khan-shiam! Thanks for your work! |
Welcome |
Description
Problem
Primitive parameters discovered via OpenAPI introspection were always registered
with
argparse
as str because the code used type(parameter_type) whereparameter_type
itself was already a string. This caused:BooleanOptionalAction
,Solution
(
"int"
,"float"
,"bool"
, …) to their real Python counterparts.booleans with
BooleanOptionalAction
.datetime
to supportdatetime.datetime
mapping.Impact
CLI arguments now receive the correct Python types at parse-time, preventing
type-related runtime errors and ensuring the generated payloads match the
API schema.