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

Update providers path in pre-commit configuration #46575

Closed
wants to merge 1 commit into from

Conversation

vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Feb 7, 2025

While working on a pre-commit I realized that many pre-commits are now misconfigured because the path of the providers changed with the restructure project (#46045). Fixing these.


^ 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.

@vincbeck
Copy link
Contributor Author

vincbeck commented Feb 7, 2025

@potiuk You might want to take a look at this one

@vincbeck vincbeck force-pushed the vincbeck/pre-commits branch from 76a14c0 to 4571ef4 Compare February 7, 2025 22:19
@vincbeck vincbeck force-pushed the vincbeck/pre-commits branch from 4571ef4 to cc499b1 Compare February 7, 2025 23:08
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Thanks for the change. I assume this is correct with the exception that for the moment the old path must be kept until all providers are migrated.
As well as you are hitting a bug as the dependencies are now missing in provider info... for migrated providers the depedency must be loaded from pyproject.toml :-( So a bit additional complexity is needed.

@potiuk
Copy link
Member

potiuk commented Feb 8, 2025

@potiuk You might want to take a look at this one

Oh absolutely .. I knew we will have to review and update those (and possibly fix some things that we missed) - it was just sometimes a bit too complex to get it all right with new and old provider structures.

I'd say what @jscheffl wrote is good idea - let's fix things and keep on rebasing those for the next few days (I hope) and merge right after we merge amazon and microsoft.azure ones - then I have the "cleanup" step to do where I will be removing a lot of the code tha handles both new and old providers, and I can make sure to also review the pre-commits to make sure they are "correct". i saw at least few places where pre-commits are "somewhat" testing things now - possibly skipping some of the stuff we wanted to catch so some fixes will be inevitable.

@vincbeck
Copy link
Contributor Author

Happy to see it was on your radar :) I'll close this one then

@vincbeck vincbeck closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants