Btw. The only reason I even attempted it, was how 'safe' it is. Those are exclusively changes to typing system - which is only really relevant for development time rather than runtime. There were literally 2 problematic changes that I found (and skipped pyupgrade from automatically bumping the typing)
* Serialization test where we are checking the type of dict at runtime (this is the only place we do it and only to check for specific Dict type * GCS hook has 'list' method name that clashes with the list built-in type and there we have to continue using List (and this one we are not even going to cherry pick - all the provider's changes (about 70% of cbanges roughly) we are not even going to attempt to cherry pick at all. J niedz., 11 wrz 2022, 14:33 użytkownik Jarek Potiuk <[email protected]> napisał: > Yeah. It was a bit late. But this was the main reasoning - if we merge > them quickly AND cherry-pick those to 2.4 branch we have a very good chance > to avoid any accidental merge of something that was merged since the 2.4 > branch cut-off - there were literally a handful of those PRs merged and it > will be immediately visible (this is also one reason why I split it into > many small PRs to detect and fix any kind of problems and remove just > merged code while cherry-picking). > > This was the reasoning behind doing it fast when I recalled that we hold > off from it in the middle of 2.3 development. > > J,. > > > On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <[email protected]> wrote: > >> One comment on the timing: but doing it now you make cherry picks for >> 2.4.x harder, of which were about to do a lot of. The best time to do such >> a change is just _before_ cutting the next main branch. >> >> On 10 September 2022 21:38:12 BST, Jarek Potiuk <[email protected]> >> wrote: >>> >>> Hey Everyone, >>> >>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much >>> fully automated PRs I prepared, but a comment from TP made me think that >>> maybe I went too much ahead and maybe there are other ideas on implementing >>> the change (or even maybe not implementing it at all). >>> >>> More context: >>> >>> I have just attempted what I THOUGHT was generally accepted and good >>> idea, but comment from TP made me think maybe I went ahead of everyone >>> else, so I am reaching here to see if this was a good idea (And I am ready >>> to back-out and do it differently - and follow another way if someone can >>> propose it). >>> >>> I just (literally while watching Poland <-> Brazil Volleybal World >>> Championship semi-finals) prepared 48 separate PRs to migrate to >>> __future__.annotations which (I thought) was considered a good idea and >>> something we wanted to defer to after v2-4-test branch cut-off because of >>> the concerns that it might make cherry-picking more complex. It was 9X% >>> fully automated conversion - by search/replace and adding `from __future__ >>> import annotations` and letting upgrade do the job (as discussed before). >>> I also split the one huge PR into 48 (!) much smaller PRs - basically >>> one per each "airflow" core package, few big providers separately, tests >>> separately (as they are far less risky than "core code" change). >>> >>> We already added __future__ annotation in a number of other places >>> before - without a "broad" agreement and consensus/voting (different people >>> did in various places - including myself and others) so I thought this is >>> not something we need a broader voting/consensus on. >>> >>> Again - that was 95% automated change where I contributed mostly the >>> thinking "how to split '' theI PR and all the rest was literally "git >>> commit add <package>" (and let pyupgrade and our other pre-commits like >>> isort and others) do the job. >>> >>> It never occured to me that this might be a problem for anyone. My >>> understanding of the problem was: >>> * we want to do it >>> * we were afraid to do it before because it would make cherry-picking to >>> 2-3 branch more problematic >>> * we wanted to do it right "when" we cut-off the 2.4 branch (for the >>> same reason as above - we wanted to avoid cherry-picking problems for 2.4.* >>> changes) >>> >>> So I wanted to make this change as fast as humanly possible - and raise >>> this multiple PRs so we can review/approve (and I'd cherry-pick them) as >>> soon as possible to minimise the cherry-picking problem. >>> >>> But maybe I was too fast and it's not as straightforward as I thought it >>> was? (actually it never occured to me that there might be a slightest >>> problem with it after the earlier discussion). I literally thought about it >>> as a mechanical change that we want to introduce and that doing that in >>> small chunks (as discussed in the original PR) was the best approach. The >>> comment from TP >>> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849 >>> - made me think that I have misjudged it. >>> >>> I wanted to write that email asking all of the committers here to help >>> to review and merge all those PRs as fast as possible (again - those are >>> purely mechanical changes, there are literally a few - literally less than >>> 5) manual changes I had to make to fix some mypy problems. >>> All the PRs are here: >>> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations >>> >>> But then I ask for something else then: >>> >>> * do you think this is fine we add __future__ annotations in general >>> * do you think the way I proposed it is somewhat too In-human (i >>> honestly feel it is an in-human change - because it is fully automated by >>> our tools :D). But is it wrong approach to have 48 PRs automatically >>> prepared by our tools to get a "consistent" approach >>> * did I do it too fast/too early ? >>> * any ideas on how we can do it differently to avoid the >>> "cherry-picking" problems (mentioned by others in the original message)? >>> >>> I am really concerned to ask if this is something I have done wrongly or >>> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should >>> approach it differently and ask for consensus on such a flurry of PRS? Or >>> maybe the timing is wrong. >>> >>> I am happy to follow in any way there - I can either keep the PRs and >>> rebase them until the time is good (it takes about two minutes to rebase >>> every one of them and apply the automation again) if the way I proposed it >>> was because I misunderstood the "sentiment" about the change - or follow >>> any other way that will help us to implement the change - or even abandon >>> it completely if we will agree it's not a good idea. I am also happy to >>> apply any "general comments" there - now that the PR's are there, applying >>> any automation even on all of them to improve it, is rather simple. >>> >>> Looking forward to any comments. I am really curious how others look at >>> it - regardless of the outcome - always eager to learn new things :) >>> >>> J. >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski < >>> [email protected]> wrote: >>> >>>> Great idea. +1 from me. Thanks for raising this up Jarek and thanks to >>>> Pierre for great isort tip :) >>>> >>>> BR >>>> Tobiasz >>>> >>>
