-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Verify decorators parameter in __init__.pyi matches operator #51048
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
base: main
Are you sure you want to change the base?
Conversation
I like the idea, but this feels like it belongs to a pre-commit check rather than a unit test. |
yes that makes sense. let me add this as part of precommit test. |
87a9768
to
444ba87
Compare
@uranusjr There are some complexities in moving to pre-commit. As unit tests, they run within the virtualenv and are context-aware, running without issue. But to run as part of a pre-commit hook, setting up the path has to be done. even when done,
|
@uranusjr when you have time, could you help me handle the above issue? |
@amoghrajesh @potiuk @uranusjr If any of you get time, could you help me in understanding if this can be better handled? |
@Bowrna I also think its not a great idea to add this in unit tests but in precommit checks to catch things earlier and repeatedly. Did you try using the inspect library and matching signatures.
Tried with the console and it works pretty consistently. |
5945451
to
19f3114
Compare
c3ec851
to
a2a98fd
Compare
6d67af2
to
28243ff
Compare
278eac1
to
bc25384
Compare
@uranusjr could you review this PR and share your feedback? |
20aaca2
to
d7ffbc6
Compare
@uranusjr the fix you suggested is added into this. thank you. when you get, please verify this PR and share your feedback. |
@uranusjr do i have any update regarding this PR? |
d7ffbc6
to
e01b252
Compare
e01b252
to
b00474b
Compare
related: #48448
closes: #48448
Add tests to all existed decorators to make sure the parameter list they accept matches the operator.
In this PR, I have added tests to verify the match using the libcst library. i have included tests for few decorators and once i get reviews if this way of verifying is fine, i will add the other decorator case too. cc: @eladkal
^ 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 airflow-core/newsfragments.