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, >>> > >>> >> > >>> > >>> >> >>> > >>> > >>> > >>> >>> > >> >>> > >>> >>
