>But I would personally love to see more "yes, but" than "no".
Saying "would love to hear what you think" in the original message followed by terming "negative" is not the way to collaborate. On Thu, 1 May 2025 at 15:01, Kaxil Naik <kaxiln...@gmail.com> wrote: > "-1" was backed with rationale discussion and no one has said it was a bad > idea but you have said it is "negative". > > On Thu, 1 May 2025 at 14:59, Jarek Potiuk <ja...@potiuk.com> wrote: > >> > That said, I want to push back on the framing of some feedback as >> “negative.” I really appreciate the folks who raised concerns, those >> perspectives are vital to making the project stronger and more inclusive. >> >> I think what I wanted to say is that I think we should all exercise >> empathy. It's super easy to say "-1" but it's harder to say "It's a good >> idea but ..". Psychological effect of the "-1" is that it "cuts the wings" >> of the person who has some idea and wants to follow it through. The >> psychological effect of "cool idea, but" is that it - more often than not >> - >> make the person more energised and inspired to look for a better solution. >> >> And yes I know different people have different communication style, and >> there are cultural differences and all that. But I would personally love >> to >> see more "yes, but" than "no". >> >> That's all I want to say.. >> >> J/ >> >> >> >> >> >> On Thu, May 1, 2025 at 11:23 AM Kaxil Naik <kaxiln...@gmail.com> wrote: >> >> > Thanks for the energy and initiative here. >> > >> > That said, I want to push back on the framing of some feedback as >> > “negative.” I really appreciate the folks who raised concerns, those >> > perspectives are vital to making the project stronger and more >> inclusive. >> > >> > Let’s make sure we continue to welcome both enthusiasm and critique. As >> > the ASF >> > Code of Conduct <https://www.apache.org/foundation/policies/conduct> >> > reminds us, thoughtful disagreement and diverse viewpoints are not only >> > expected but encouraged. Referring to concerns as “negative”, even >> > unintentionally, can discourage people from speaking up, which we want >> to >> > avoid. >> > >> > We all want what’s best for Airflow, and part of that is making sure >> > everyone feels safe to contribute, especially when something doesn’t >> feel >> > right. >> > >> > >Rather than focusing on the negative side, try to figure out a way to >> make >> > it work :) >> > >> > Back to the proposal from Jens and you - Yes that (a comment allowing >> to >> > still merge/auto-merge the PR) alleviates my concern. >> > >> > Regards, >> > Kaxil >> > >> > >> > >> > On Thu, 1 May 2025 at 12:54, Jarek Potiuk <ja...@potiuk.com> wrote: >> > >> > > I really like Jens's line of thought. Rather than focusing on the >> > negative >> > > side, try to figure out a way to make it work :). That's very >> inspiring. >> > > >> > > I think the original slack proposal of Jens was rather brittle, but it >> > made >> > > me think that we can actually implement what we need rather quickly. >> > > >> > > Following the `Dependabot` style @rebase - we could implement - very >> > easily >> > > - there are ready actions for that and other projects like apache beam >> > are >> > > using similar workflows - the comment workflow, where maintainer who >> > wants >> > > to merge (or auto-merge) such "failing" PR would just comment it >> > > appropriately. >> > > >> > > What we really want is to make the "finalize" check succeed, so if >> the PR >> > > misses the "Finalize" step, a committer would just have to comment it >> > with >> > > "@bypass-finalize` or `@finalize` or similar. And what it would do, it >> > > would use Github API to make the "Finalize" check succeed. There is an >> > API >> > > for that. >> > > >> > > That has a few nice properties: >> > > >> > > * it uses the same GitHub UI we use, no need to have an "escape hatch" >> > and >> > > separate remote >> > > * it follows the pattern we already agreed to some time ago and >> generally >> > > follow -> we are supposed (as maintainers) to explain when we are >> merging >> > > such failing PR to show that this is a deliberate action - I usually >> for >> > > example always add "fixed already in main" or "Fixed already by #..." >> > > comment - you really need a comment in this case explaining that it >> was >> > not >> > > accidental "merge". >> > > * so if we have such workflow, it would "force" maintainer to make a >> > > deliberate decision "yes I want to merge it despite failing" and make >> a >> > > comment in the PR >> > > * it would only set the "finalize" check to succeed, so still you >> would >> > not >> > > be able to merge it without having an approval - this would only >> bypass >> > the >> > > "finalize" check >> > > >> > > WDYT? Would that address all concerns with auto-merge ? Does it make >> > sense >> > > and is it "easy enough" to follow? Or are there any other concerns? >> > > >> > > >> > > >> > > >> > > On Wed, Apr 30, 2025 at 10:36 PM Pavankumar Gopidesu < >> > > gopidesupa...@gmail.com> wrote: >> > > >> > > > I am also in line with Jens. The auto-merge feature works well in >> > keeping >> > > > the CI pipeline green before merging. >> > > > However, there are situations where we need to merge changes >> quickly. >> > > > Instead of using an >> > > > escape route (which I’m not a fan of, as it carries risks if >> something >> > > goes >> > > > wrong—even though we can revert, it’s still a concern), >> > > > we could explore ways to dynamically bypass certain checks when >> > necessary >> > > > or maybe some help from INFRA? >> > > > >> > > > Pavan >> > > > >> > > > On Wed, Apr 30, 2025 at 9:22 PM Jens Scheffler >> > > <j_scheff...@gmx.de.invalid >> > > > > >> > > > wrote: >> > > > >> > > > > As there was a call for more opinions. Here I am :-D >> > > > > >> > > > > I understand both positions. As I like AutoMerge very much I am >> not >> > > > > giving up :-D I'd like to keep it. >> > > > > >> > > > > I think there is still an option in between. Maybe need to draft a >> > bit >> > > > > of thoughts but I think we could build something still around the >> > > > > limitations allowing automation. >> > > > > >> > > > > Added a minor thought to the slack chat. But if community rules >> > propose >> > > > > to push to devchennel let me know. >> > > > > >> > > > > Jens >> > > > > >> > > > > On 30.04.25 11:04, Jarek Potiuk wrote: >> > > > > > Yeah, And I would encourage everyone to try it and provide >> feedback >> > > > while >> > > > > > it is enabled. >> > > > > > >> > > > > > So far we identified a few things (and fix them) that made it >> > > > borderline >> > > > > > unusable (bug in workflow for UI-only changes, GitHub starting >> to >> > > > 429-us >> > > > > > with too many requests, and the mysterious "hanging" of the >> latest >> > > > > > botocore/celery (?) "special tests" -> all that is already >> > addressed >> > > in >> > > > > > main, and those issues should not happen (hopefully), so I'd >> say we >> > > can >> > > > > now >> > > > > > "truly" see how it might work. >> > > > > > >> > > > > > And one comment from my side - indeed, I find it nice actually, >> but >> > > > it's >> > > > > > definitely not a "deal breaker"- so if others find it too >> > > disruptive, I >> > > > > am >> > > > > > definitely not going to die on this hill, I just thought it does >> > > > improve >> > > > > > the workflow in the way that it allows for mostly "fire and >> forget" >> > > > when >> > > > > > you approve the workflow, one thing that it definitely improves >> is >> > > that >> > > > > you >> > > > > > do not have to remember about coming back to merge a request >> when >> > CI >> > > > > > succeeds. >> > > > > > >> > > > > > J. >> > > > > > >> > > > > > On Wed, Apr 30, 2025 at 10:58 AM Kaxil Naik < >> kaxiln...@gmail.com> >> > > > wrote: >> > > > > > >> > > > > >> Forgot to note an additional point in Summary: If we find >> anything >> > > > > blocking >> > > > > >> us in that period, we will merge >> > > > > >> https://github.com/apache/airflow/pull/50009 to disable >> > auto-merge. >> > > > > >> >> > > > > >> On Wed, 30 Apr 2025 at 14:26, Kaxil Naik <kaxiln...@gmail.com> >> > > wrote: >> > > > > >> >> > > > > >>> Jarek & I discussed it on Slack in #internal-airflow-ci-cd. >> > Summary >> > > > > >> below: >> > > > > >>> I have a PR to disable it: >> > > > > https://github.com/apache/airflow/pull/50009. >> > > > > >>> However, given that many countries will be on holiday on May 1 >> > due >> > > to >> > > > > >>> Labour Day, and some teething issues were fixed yesterday, we >> > will >> > > > let >> > > > > it >> > > > > >>> run for a few more days so other committers and contributors >> can >> > > get >> > > > a >> > > > > >>> chance to try it out and share their experience after the >> > > > > >> experiment/trial >> > > > > >>> is concluded. >> > > > > >>> >> > > > > >>> >> > > > > >>> >> > > > > >>> On Wed, 30 Apr 2025 at 13:59, Kaxil Naik <kaxiln...@gmail.com >> > >> > > > wrote: >> > > > > >>> >> > > > > >>>> Whoops yeah. >> > > > > >>>> >> > > > > >>>>> Yep. Because it did not have all conversations resolved. We >> > also >> > > > have >> > > > > >>>> "require resolved conversation" as one of the branch >> protection >> > > > > >>>> conditions. >> > > > > >>>> I resolved the conversation and it got merged automatically. >> > > > > >>>> >> > > > > >>>> Let's adapt it when ready though, I don't see any urgency of >> > > getting >> > > > > >>>> enabling auto-merge or getting it contributed immediately to >> asf >> > > > INFRA >> > > > > >> when >> > > > > >>>> it isn't critical. It is about priortization >> > > > > >>>> >> > > > > >>>> I'd say - if that is really bothering us - let's shape the >> > feature >> > > > > >> rather >> > > > > >>>>> than outright reject it. >> > > > > >>>> >> > > > > >>>> On Wed, 30 Apr 2025 at 12:37, Jarek Potiuk <ja...@potiuk.com >> > >> > > > wrote: >> > > > > >>>> >> > > > > >>>>> On Wed, Apr 30, 2025 at 7:55 AM Kaxil Naik < >> > kaxiln...@gmail.com> >> > > > > >> wrote: >> > > > > >>>>>> To the point that the original PR is still not merged even >> > > after I >> > > > > >> had >> > > > > >>>>>> re-triggered the failed tests yesterday: >> > > > > >>>>>> https://github.com/apache/airflow/pull/49727 >> > > > > >>>>>> >> > > > > >>>>>> >> > > > > >>>>> Yep. Because it did not have all conversations resolved. We >> > also >> > > > have >> > > > > >>>>> "require resolved conversation" as one of the branch >> protection >> > > > > >>>>> conditions. >> > > > > >>>>> I resolved the conversation and it got merged automatically. >> > > > > >>>>> >> > > > > >>>>> >> > > > > >>>>>> On Wed, 30 Apr 2025 at 11:20, Kaxil Naik < >> kaxiln...@gmail.com >> > > >> > > > > >> wrote: >> > > > > >>>>>>> The gitbox escape hatch isn't it though -- if we are to >> allow >> > > > that >> > > > > >>>>> why >> > > > > >>>>>> not >> > > > > >>>>>>> just allow people to merge it directly from github that >> to go >> > > via >> > > > > >> an >> > > > > >>>>>>> "escape hatch". >> > > > > >>>>> Generally speaking GitHub has this option. Currently >> "admins" >> > > have >> > > > a >> > > > > >>>>> possibility of overriding branch protection (via UI). And it >> > > would >> > > > be >> > > > > >>>>> possible - if INFRA will allow it - to possibly add an >> > .asf.yaml >> > > > > >> feature >> > > > > >>>>> to >> > > > > >>>>> also allow branch protection override for all committers or >> a >> > > > subset >> > > > > of >> > > > > >>>>> the >> > > > > >>>>> committers (PMC Members ? ). This is more of a limitation of >> > the >> > > > > >> current >> > > > > >>>>> implementation of permissions than a missing feature. If we >> all >> > > > feel >> > > > > >> that >> > > > > >>>>> the gitbox escape hatch is not enough, we can likely even >> > > > contribute >> > > > > >>>>> such a >> > > > > >>>>> feature to .asf.yaml - if INFRA will be ok with the option. >> > It's >> > > > very >> > > > > >>>>> easy >> > > > > >>>>> to contribute to - INFRA made it possible, we have a new >> > > framework: >> > > > > >>>>> https://github.com/apache/infrastructure-asfyaml - we can >> even >> > > > > >> implement >> > > > > >>>>> "airflow-only" .asf.yaml feature, that will not be initially >> > > > > available >> > > > > >> to >> > > > > >>>>> other ASF projects and later we can promote it to be >> available >> > to >> > > > > >>>>> everyone. >> > > > > >>>>> >> > > > > >>>>> I'd say - if that is really bothering us - let's shape the >> > > feature >> > > > > >> rather >> > > > > >>>>> than outright reject it. >> > > > > >>>>> >> > > > > >>>>> >> > > > > >>>>>>> I am -1 on this auto-merge feature >> > > > > >>>>>>> >> > > > > >>>>> Understood :). But let's give it a bit more time as well and >> > > maybe >> > > > > >>>>> improve >> > > > > >>>>> it (see above) - unless we really feel we are blocked now - >> > then >> > > it >> > > > > >>>>> should >> > > > > >>>>> be as easy as merging an .asf.yaml change to disable it. >> > > > > >>>>> >> > > > > >>>>> >> > > > > >>>>>>> On Wed, 30 Apr 2025 at 11:18, Kaxil Naik < >> > kaxiln...@gmail.com> >> > > > > >>>>> wrote: >> > > > > >>>>>>>> That’s not a single person, it impacts the committers and >> > the >> > > PR >> > > > > >>>>> author >> > > > > >>>>>>>> involved too. I don’t see how team productivity soars >> here. >> > > > > >>>>>>>> >> > > > > >>>>>>>> On Wed, 30 Apr 2025 at 02:39, Jarek Potiuk < >> > ja...@potiuk.com> >> > > > > >>>>> wrote: >> > > > > >>>>>>>>> But yes, I also miss the previous "merge because I think >> > it's >> > > > > >> safe" >> > > > > >>>>>>>>> workflow. >> > > > > >>>>>>>>> >> > > > > >>>>>>>>> I badly miss it. Personally, It hurts my productivity. >> > > > > >>>>>>>>> >> > > > > >>>>>>>>> But I think the "require status check" to be green is >> great >> > > for >> > > > > >>>>> "team >> > > > > >>>>>>>>> productivity". Usually when single person is impacted >> more >> > > than >> > > > > >>>>> team in >> > > > > >>>>>>>>> general, it's worse for the person impacted, but team >> > > > > >> productivity >> > > > > >>>>>> soars. >> > > > > >>>>>>>>> J. >> > > > > >>>>>>>>> >> > > > > >>>>>>>>> >> > > > > >>>>>>>>> On Tue, Apr 29, 2025 at 11:03 PM Jarek Potiuk < >> > > > ja...@potiuk.com> >> > > > > >>>>>> wrote: >> > > > > >>>>>>>>>> Just to add comment: >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> a) there was some instability of "celery/boto" hanging >> > tests >> > > > > >>>>> today >> > > > > >>>>>>>>> that is >> > > > > >>>>>>>>>> rather difficult to address - but we worked around it >> by >> > > just >> > > > > >>>>>> removing >> > > > > >>>>>>>>>> "special-tests" from pre-requisite of merging >> > > > > >>>>>>>>>> b) GitHub today (like literally today!) started to be >> > picky >> > > on >> > > > > >>>>> "too >> > > > > >>>>>>>>> many >> > > > > >>>>>>>>>> requests" - I addressed it today for both helm chart >> and >> > our >> > > > > >>>>> release >> > > > > >>>>>>>>> tests >> > > > > >>>>>>>>>> (we are using bearer-token to authenticate and increase >> > the >> > > > > >>>>> limit - >> > > > > >>>>>>>>> and we >> > > > > >>>>>>>>>> added cache on downloading json schema that was >> downloaded >> > > > > >>>>> "per-test" >> > > > > >>>>>>>>>> c) in cases like the one mentioned above with >> intermittent >> > > > > >>>>> failures - >> > > > > >>>>>>>>>> simple "rerun failed jobs" (assuming it will succeed >> after >> > > > > >>>>> rerun) - >> > > > > >>>>>> is >> > > > > >>>>>>>>>> essentially equivalent of "merge" (unless it fails >> again >> > > which >> > > > > >>>>> for me >> > > > > >>>>>>>>> is a >> > > > > >>>>>>>>>> signal of "DO NOT MERGE") >> > > > > >>>>>>>>>> d) we always have the "gitbox" escape hatch - that >> allows >> > > any >> > > > > >>>>>>>>> committer to >> > > > > >>>>>>>>>> push the fix directly, bypassing the limits: >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> This is a simple thing for committers: >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> git add remote gitbox >> > > > > >>>>>> https://gitbox.apache.org/repos/asf/airflow.git >> > > > > >>>>>>>>>> git fetch gitbox >> > > > > >>>>>>>>>> git commit --amend ("add #PR number") >> > > > > >>>>>>>>>> git push gitbox BRANCH_NAME:main (you need to provide >> your >> > > > > >>>>> apache id >> > > > > >>>>>>>>> and >> > > > > >>>>>>>>>> password) >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> This is a nice escape hatch that we can use as >> > "exceptional >> > > > > >>>>>> workflow" - >> > > > > >>>>>>>>>> and it works - I did it quite a few times over the last >> > few >> > > > > >>>>> days. Not >> > > > > >>>>>>>>> UI >> > > > > >>>>>>>>>> controlled, but IMHO exceptional workflow should be - >> > well >> > > - >> > > > > >>>>>>>>> exceptional. >> > > > > >>>>>>>>>> J. >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> >> > > > > >>>>>>>>>> On Tue, Apr 29, 2025 at 8:52 PM Kaxil Naik < >> > > > > >> kaxiln...@gmail.com> >> > > > > >>>>>>>>> wrote: >> > > > > >>>>>>>>>>> Similar experience as Elad, I am in favor of >> disabling it >> > > > tbh. >> > > > > >>>>> For >> > > > > >>>>>>>>>>> example, >> > > > > >>>>>>>>>>> https://github.com/apache/airflow/pull/49727 has a >> > failing >> > > > > >>>>> test as >> > > > > >>>>>>>>> below >> > > > > >>>>>>>>>>> -- >> > > > > >>>>>>>>>>> which is not an issue, and test passes locally so I >> would >> > > > want >> > > > > >>>>> to >> > > > > >>>>>>>>> merge it >> > > > > >>>>>>>>>>> but I can't. >> > > > > >>>>>>>>>>> >> > > > > >>>>>>>>>>> FAILED >> > > > > >>>>>>>>>>> >> > > > > >>>>>>>>>>> >> > > > > >> >> > > > > >> > > > >> > > >> > >> helm-tests/tests/helm_tests/airflow_aux/test_basic_helm_chart.py::TestBaseChartTest::test_priority_classes >> > > > > >>>>>>>>>>> - requests.exceptions.HTTPError: 429 Client Error: Too >> > Many >> > > > > >>>>> Requests >> > > > > >>>>>>>>> for >> > > > > >>>>>>>>>>> url: >> > > > > >>>>>>>>>>> >> > > > > >>>>>>>>>>> >> > > > > >> >> > > > > >> > > > >> > > >> > >> https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.29.1-standalone-strict/priorityclass-scheduling-v1.json >> > > > > >>>>>>>>>>> On Tue, 29 Apr 2025 at 18:29, Jarek Potiuk < >> > > ja...@potiuk.com >> > > > > >> > > > > >>>>>> wrote: >> > > > > >>>>>>>>>>>> On Tue, Apr 29, 2025 at 1:46 PM Elad Kalif < >> > > > > >>>>> elad...@apache.org> >> > > > > >>>>>>>>> wrote: >> > > > > >>>>>>>>>>>>> Thanks for that Jarek! >> > > > > >>>>>>>>>>>>> I find the lack of ability to merge PRs fast very >> > > limiting >> > > > > >>>>> but >> > > > > >>>>>> it >> > > > > >>>>>>>>>>> might >> > > > > >>>>>>>>>>>> be >> > > > > >>>>>>>>>>>>> just something to get used to. >> > > > > >>>>>>>>>>>>> >> > > > > >>>>>>>>>>>> Indeed. I also see it, but also I got a few manually >> > > pushed >> > > > > >>>>> "must >> > > > > >>>>>>>>> fix >> > > > > >>>>>>>>>>>> quickly" to gitbox, and actually I find it really >> nice - >> > > > > >>>>> because >> > > > > >>>>>> it >> > > > > >>>>>>>>> is >> > > > > >>>>>>>>>>>> still possible, but it require some extra effort and >> > > > > >>>>> deliberate >> > > > > >>>>>> "ok >> > > > > >>>>>>>>> that >> > > > > >>>>>>>>>>>> one really should be pushed to unblock everyone" - as >> > long >> > > > > >> as >> > > > > >>>>> we >> > > > > >>>>>> all >> > > > > >>>>>>>>>>>> (especially those people that are active in the >> > > > > >>>>>>>>>>>> #internal-airfow-ci-cd channel) know how to do it and >> > can >> > > > > >> fix >> > > > > >>>>>> things >> > > > > >>>>>>>>>>>> quickly, this is actually a nice way to make it into >> > > > > >>>>> "exceptional" >> > > > > >>>>>>>>>>> workflow >> > > > > >>>>>>>>>>>> - which will push us more in making sure airflow >> main is >> > > > > >>>>> really >> > > > > >>>>>>>>> "green" >> > > > > >>>>>>>>>>> - >> > > > > >>>>>>>>>>>> which ultimately is our goal to make it as green as >> > > possible >> > > > > >>>>> all >> > > > > >>>>>> the >> > > > > >>>>>>>>>>> time. >> > > > > >>>>>>>>>>>> What **might help with that** (and also keeping the >> > > "enable >> > > > > >>>>> auto >> > > > > >>>>>>>>> merge" >> > > > > >>>>>>>>>>>> might motivate it more to) is to: >> > > > > >>>>>>>>>>>> >> > > > > >>>>>>>>>>>> * speed up the builds - we MUST prioritise now ARC >> (K8S >> > > > > >>>>>> self-hosted >> > > > > >>>>>>>>>>>> runners) to make our builds simply faster - I >> started a >> > > > > >>>>> discussion >> > > > > >>>>>>>>> and a >> > > > > >>>>>>>>>>>> small group of people who can work together to >> complete >> > it >> > > > > >>>>> after >> > > > > >>>>>>>>>>> Hussein's >> > > > > >>>>>>>>>>>> POC) >> > > > > >>>>>>>>>>>> * speed up the image release - with ARM runners >> (which >> > we >> > > > > >>>>> might be >> > > > > >>>>>>>>> able >> > > > > >>>>>>>>>>> to >> > > > > >>>>>>>>>>>> do independently as recently I think we have >> > > > > >>>>> hypervisor-enabled >> > > > > >>>>>> ARM >> > > > > >>>>>>>>>>> images >> > > > > >>>>>>>>>>>> available as public runners as github made it >> generally >> > > > > >>>>>> available). >> > > > > >>>>>>>>>>>> * speed up the doc builds for airflow-site - we >> (mainly >> > > > > >>>>> Pavan) are >> > > > > >>>>>>>>>>> close to >> > > > > >>>>>>>>>>>> complete offloading of the historical release docs >> to S3 >> > > > > >> and I >> > > > > >>>>>> hope >> > > > > >>>>>>>>> it >> > > > > >>>>>>>>>>> will >> > > > > >>>>>>>>>>>> cut down a lot on doc publishing workflows. >> > > > > >>>>>>>>>>>> >> > > > > >>>>>>>>>>>> J, >> > > > > >>>>>>>>>>>> >> > > > > >> > > > > >> --------------------------------------------------------------------- >> > > > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org >> > > > > For additional commands, e-mail: dev-h...@airflow.apache.org >> > > > > >> > > > > >> > > > >> > > >> > >> >
