And merged! that was fast. Thanks Bugra, Ash, Jens ! On Sun, Feb 16, 2025 at 11:40 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> https://github.com/apache/airflow/pull/46800 -> there you go :) . Happy > reviewing of 1570+ files changed. > > BTW. It's way easier to review it with `git diff` than with Github UI :) > > On Sun, Feb 16, 2025 at 11:32 AM Jarek Potiuk <ja...@potiuk.com> wrote: > >> Naaa. that one is super easy to implement and automate and it's better to >> have one quick "surgical" cut to rename all of those :), I am about to send >> PR with all of those changes in a moment. >> >> On Sun, Feb 16, 2025 at 11:26 AM Buğra Öztürk <ozturkbugr...@gmail.com> >> wrote: >> >>> Amazing!:) Maybe we can divide and conquer this on structure changes and >>> pre-commit rather than leaving you with all :) >>> >>> On Sun, 16 Feb 2025, 10:45 Jarek Potiuk, <ja...@potiuk.com> wrote: >>> >>> > Yeah "unit" seems to be the way to go :) . I will make another giant PR >>> > (but this time it should be even easier to review) to convert to it :) >>> > >>> > On Sun, Feb 16, 2025 at 10:41 AM Buğra Öztürk <ozturkbugr...@gmail.com >>> > >>> > wrote: >>> > >>> > > Thanks for all the effort on this! It looks great! >>> > > - The approach looks great! >>> > > - Heavy plus one on this. The pre-commit should be one of the ground >>> > > pillars of any standardisation. Having an extra file structure check >>> > could >>> > > be included too on top of `from` to eliminate new providers to >>> follow the >>> > > consistent structure. >>> > > - I like the unit too over provider_tests. It is a great idea to >>> > eliminate >>> > > ambiguity. The other parts seem pretty pretty good. >>> > > >>> > > Along with the latest improvements, these will be great additions! >>> > > >>> > > >>> > > On Sun, Feb 16, 2025 at 4:15 AM Rahul Vats <rah.sharm...@gmail.com> >>> > wrote: >>> > > >>> > > > Thanks for the detailed explanation and for driving this effort—it >>> > looks >>> > > > like a solid improvement. >>> > > > >>> > > > 1. >>> > > > >>> > > > I love the approach & new structure. I can't think of a better >>> > > approach. >>> > > > 2. >>> > > > >>> > > > I agree that adding a pre-commit check to enforce consistent >>> imports >>> > > is >>> > > > a good idea. It will help maintain the new structure and prevent >>> > > > regressions. >>> > > > 3. >>> > > > >>> > > > For the naming, I’d prefer using unit instead of provider_tests >>> to >>> > > keep >>> > > > it more intuitive and consistent with system and integration. >>> This >>> > > makes >>> > > > the structure self-explanatory and easier for new contributors. >>> > > > >>> > > > >>> > > > Regards, >>> > > > Rahul Vats >>> > > > >>> > > > On Sun, 16 Feb 2025 at 03:50, Jarek Potiuk <ja...@potiuk.com> >>> wrote: >>> > > > >>> > > > > > 2) no real opinion, seems useful. Let’s see if we can do it >>> with a >>> > > ruff >>> > > > > rule first? It _might_ be possible. (It is possible to extend the >>> > ruff >>> > > > > rules on a per subproject basis, check out >>> task_sdk/pyproject.toml) >>> > > > > >>> > > > > I am not sure if we need ruff rule for it, as we have not yet >>> enabled >>> > > any >>> > > > > `ruff` airflow rules in airflow development (it is targeted so >>> far >>> > for >>> > > > > users writing DAGs} >>> > > > > >>> > > > > I think it's more "airflow internal" rule not something that can >>> be >>> > > used >>> > > > > for general audience (which is pretty-much what ruff rules are >>> about) >>> > > > > >>> > > > > Unfortunately - the current way ruff works does not allow local >>> > > > > "plugin" functionality to add new rules that would only be >>> applicable >>> > > to >>> > > > > the current project you are in. In the absence of that feature, >>> > having >>> > > a >>> > > > > python pre-commit seem the next-best thing. >>> > > > > >>> > > > > J. >>> > > > > >>> > > > > >>> > > > > On Sat, Feb 15, 2025 at 10:50 PM Ash Berlin-Taylor < >>> a...@apache.org> >>> > > > wrote: >>> > > > > >>> > > > > > 100%. It’s on my list to tackle very soon, it can’t get put off >>> > much >>> > > > > > longer. >>> > > > > > >>> > > > > > I have some thoughts (around back compat mostly) but they >>> aren’t >>> > > > > collected >>> > > > > > yet, and I won’t derail this thread. >>> > > > > > >>> > > > > > > On 15 Feb 2025, at 21:43, Jarek Potiuk <ja...@potiuk.com> >>> wrote: >>> > > > > > > >>> > > > > > > cc: @ashb @uranusjr @kaxil -> something for consideration in >>> our >>> > > > > > > discussions on how to repackage airlfow soon. I keep on >>> > explaining >>> > > > why >>> > > > > > > running code in airflow.__init__.py is a bad idea and >>> advocating >>> > > for >>> > > > > > > removal of it and replace it with explicit initialization, >>> yet >>> > that >>> > > > > topic >>> > > > > > > have not been discussed yet, but I will plan to start a >>> > discussion >>> > > > > about >>> > > > > > it >>> > > > > > > soon once we approach the packaging subject. I am not sure >>> what's >>> > > > your >>> > > > > > > thinking is about this - I know you spent consirderable >>> amount of >>> > > > time >>> > > > > on >>> > > > > > > doing all the "lazy initalization" dance all over the places, >>> > and I >>> > > > > think >>> > > > > > > it adds a lot of complexity to our code and only partially >>> solves >>> > > the >>> > > > > > > cicular imports problem. But I know @ashb has very strong >>> feeling >>> > > > about >>> > > > > > > being able to do "from airlfow import Dag" - which more or >>> less >>> > > > > requires >>> > > > > > > all this complexity. I just don't think it's worth it. >>> > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > > >>> > > -- >>> > > Bugra Ozturk >>> > > >>> > >>> >>
