> That was a temporary issue - it is green now. I needed to figure out how to name checks properly, and it turned out that "Tests / Finalize tests / Summarize Warnings" is named (for whatever reason) as "Finalize tests / Summarize Warnings" when run as part of the composite "umbrella" workflow "Tests". Simply - the first level path is stripped in this case.
Oh ok! > So I struggled a bit with guessing how it should be done. But I figured it - out - your PR is green now, and if you see it as a required check - re-running the finalize warning or just rebasing your PR should do the job. It was only really problematic for PRs that were completing in a last hour or two where I tried to figure out what check I should put in.... And Haha, I see. Unfortunately that was the case with one of my PRs :) Gave me the initial bad preview. My initial experience was shadowing the benefits that this would offer to the mental load of maintainers. It would be a great tool overall as Jens mentioned too. However, I am going to try it out again and express any concerns once i encounter them. IAC, it's worth experimenting with, given that our recent experiments like backport tool, dosubot have all stayed :D Thanks & Regards, Amogh Desai On Sat, Apr 26, 2025 at 9:35 PM Jens Scheffler <j_scheff...@gmx.de.invalid> wrote: > Hi all, > > thanks for tuning on auto-merge. > > Agree that there might be a residual risk that pipeline goes green "too > early" and approval is there... but I think we need to use carefully. > > I just did my first auto merge and carefully watched... all looks good. > If it helps in only 50% of cases to reduce mental load I am totally > fine. Might not be perfect in all situations. But in cases where (1) I > trust the author that while I am not watching nothing surprising is > committed and turning green and (2) pipeline is stable. Otherwise there > is always the "resert PR" option. > > Jens > > P.S.: Ups, found one dependency issue which I attempt to fix in #49827 > > On 26.04.25 15:45, Jarek Potiuk wrote: > > On Sat, Apr 26, 2025 at 3:02 PM Amogh Desai <amoghdesai....@gmail.com> > > wrote: > > > >> I am not sure if I like this feature a lot with my recent experience > with > >> this PR: https://github.com/apache/airflow/pull/49692. > >> > >> On the PR status, it mentions "finalize tests" is waiting to report > status > >> but when i describe the tests, I see that task has completed. > > > >> It is quite confusing to me with this experience at least. But, maybe I > >> need some getting used to. > >> > > That was a temporary issue - it is green now. I needed to figure out how > to > > name checks properly, and it turned out that "Tests / Finalize tests / > > Summarize Warnings" is named (for whatever reason) as "Finalize tests / > > Summarize Warnings" when run as part of the composite "umbrella" workflow > > "Tests". Simply - the first level path is stripped in this case. > > > > And yes it took me some time because indeed - it inconsistently shows in > > the UI and it depends on timing: > > > > * When you add it first as required, it is displayed as "Finalize tests > / > > Summarize Warnings" but when the job starts running this required check > > magically turns into "Tests / Finalize tests / Summarize Warnings" > > * Also this required check disappears when you remove it (and PR can be > > merged then) but when you add it to existing PR, it will not get green > > until you rerun the job (or whole PR) - so if your "Summarize Warnings" > > was added before I properly added the check - you will see two different > > checks - one completed (With Tests) and one waiting (without). > > > > . ¯\_(ツ)_/¯ > > > > So I struggled a bit with guessing how it should be done. But I figured > it > > - out - your PR is green now, and if you see it as a required check - > > re-running the finalize warning or just rebasing your PR should do the > job. > > It was only really problematic for PRs that were completing in a last > hour > > or two where I tried to figure out what check I should put in.... And > > > > For the fun of it, I added the log of me fighting with GH naming at the > end > > of that email: > > > > > >> Speaking from a general perspective and in time sensitive situations > like > >> cutting an RC, I am sure > >> we would like to have some PRs in as and when they pass CI / sometimes > >> don't pass (known CI issues going on). > >> > > Yeah. That's the biggest worry I have. Those are the mitigations we can > > apply: > > > > a) temporarily disable branch protection when we are in "crunch" > situation > > b) push such PRs directly to gitbox - bypassing GH branch protection > > > > > >> This will make it slightly inconvenient and hard to keep track of > whether a > >> PR has been merged or not, at the very least. > >> > > Yes, when we push it directly we have to make sure to do it carefully and > > add PR# manually > > > >> I'm expressing this concern because I do not see any option to > "override" > >> the enable or disable automerge feature and merge > >> it myself. > >> > >> > > Yes. There is none. > > > > > >> Would love it if someone could help me with that information. > >> > >> P.S: It could be that I am not able to force merge because someone else > >> (Jarek) enabled the force merge for my PR? > >> > > Temp timing issue > > > > > >> Thanks & Regards, > >> Amogh Desai > >> > > ------------------------------------------------------------------ > > > > Lof of fighting with GH naming: > > > > commit 4a074c1b17726df4939fb8b426f6cb90b53f0881 > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 15:12:36 2025 +0200 > > > > Another attempt to set the right context > > > > commit af977853743a97d5785a9b7f9ce63e7fd91cd133 > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 14:49:51 2025 +0200 > > > > Another attempt to set the right context > > > > commit 97c5d9cf33e4c2f0654c744872ca8de452daa5ca > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 14:40:45 2025 +0200 > > > > Another attempt to set the right context > > > > commit 14d60c7ce99f30d3477a01d56dbce15460b11f28 > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 14:33:05 2025 +0200 > > > > Maybe this one will work? > > > > commit 156d0e8080c20b5cb61315ac6118cbce5abf75a9 > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 14:21:51 2025 +0200 > > > > Another attempt to set the right context > > > > commit 1adf5afcc9cca4a6492359b2883b5348f60e7f9f > > Author: Jarek Potiuk <ja...@potiuk.com> > > Date: Sat Apr 26 12:56:08 2025 +0200 > > > > Proper (?) context for branch protection experiment (#49815) > > > > > > > >> On Sat, Apr 26, 2025 at 5:41 PM Jarek Potiuk <ja...@potiuk.com> wrote: > >> > >>> Hello everyone, > >>> > >>> With some initial teething problems we've enabled an "experimental" > >> feature > >>> of "auto-merging" PRs in our repo. It should potentially help with > >> "focus" > >>> of maintainers, because they will not have to come-back to the PRs to > >> merge > >>> it once they enabled auto-merge for them. > >>> > >>> It works in the way, that when "required status checks", "reviews" and > >>> "resolved conversations" (the required conditions for "main" protected > >>> branch) are not met, committer can use "Enable auto-merge" on the PR > and > >>> when all conditions are met, the PR will get merged automatically. > >>> > >>> But after enabling it, it turns out that this has one **serious** > >> drawback. > >>> We currently cannot override the protection from GitHub UI, and it's > not > >>> possible to merge PR that did not pass one of the checks (So "Finalize > >>> tests / Summarize warnings" has to complete successfully in order to be > >>> able to merge PR, > >>> > >>> This is quite a bit of a blocker. But not entirely, because I've > learned > >> (I > >>> did not know it before) that we (committers) already have a way to > bypass > >>> ANY protection - by directly pushing code to main branch via gitbox URL > >>> exposed by Apache Infrastructure: > >>> > >>> https://gitbox.apache.org/repos/asf > >>> > >>> Specifically - you can set this as remote > >>> https://gitbox.apache.org/repos/asf/airflow.git - and push changes > >>> directly > >>> to "main" branch. It will bypass any protection. You do not even need > to > >>> get a review from another maintainer (yes - I just tested it and it > >> works). > >>> You just need to authenticate with your apache id / password. > >>> > >>> That is not great from a security and provenance point of view, but > well, > >>> ASF allows it for now (which is something we will have to fix > eventually > >> I > >>> think. It requires using git CLI/local client to push such branches > and > >>> there are some small things we have to remember (like manually adding > PR > >> # > >>> to the branch we are pushing or not having PR# in the merged commit at > >>> all). > >>> > >>> Certainly not as convenient as the merge button in PR - but workable if > >> we > >>> want to merge something quickly, regardless of the status (and > apparently > >>> regardless of review / approval which is a bummer). > >>> > >>> I left it enabled for a moment - the weekend maximum and maybe > beginning > >>> of Monday and would love to hear what you think. > >>> > >>> J. > >>> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > For additional commands, e-mail: dev-h...@airflow.apache.org > >
