Because the precheckins didn't run the tests in jkd8, they only run certain tests with jdk11. That's why the test passed all PR checks but failed in the pipe.
On Thu, Oct 31, 2019, 3:12 PM Nabarun Nag <n...@apache.org> wrote: > Jinmei, it's answered in the third email in this chain. Aaron asked the > same question. [the process won't take more than 30 min, also its good to > confirm that the revert won't turn the pipeline red] > I am more worried that how a commit that made the pipeline red made it into > the codebase. > > Regards > Naba > > On Thu, Oct 31, 2019 at 2:37 PM Jinmei Liao <jil...@pivotal.io> wrote: > > > I am not sure whether this is related to this topic or not, but recently > I > > had to revert one of my commit, before I could just do a revert and push > to > > develop, but now that is blocked. I had to file a PR to revert a change > > that's causing the pipeline to be red. My question is: do we still need > to > > follow the same process (waiting for one review approval) to revert a > > commit? > > > > On Thu, Oct 31, 2019 at 10:17 AM Udo Kohlmeyer <u...@apache.com> wrote: > > > > > You are correct... Break glass is a sign that whatever needed to be > > > done, was not going to work using the prescribed approach.. > > > > > > I see this "break glass" as a special handshake or someone with more > > > "authority" needs to be agree with this... but given there is not > > > "someone with more authority" in Apache... this would have to be > > > consensus between either committers or PMC members. > > > > > > --Udo > > > > > > On 10/31/19 9:58 AM, Mark Hanson wrote: > > > > -1 for "Break glass" approach. Needing a break glass approach is a > > sign. > > > I wonder how hard that would be to make exist. I think our break glass > > > approach is that we have someone with the power disable the > restrictions > > in > > > Github for the window that we must and then we put it back. > > > > > > > > Thanks, > > > > Mark > > > > > > > >> On Oct 31, 2019, at 9:26 AM, Benjamin Ross <br...@pivotal.io> > wrote: > > > >> > > > >> I would agree with udo, +1 to having an emergency 'break glass' > > override > > > >> but -1 to having the ability to do it routinely or for convenience. > > > >> > > > >> The main use case in my mind is for infrastructure related issues > that > > > >> block a PR behind unrelated changes which can be really frustrating > > and > > > >> inconvenient (StressNewTest is a big culprit here) but I'm hoping > that > > > if > > > >> constant issues with this arise it will lead to fixing the offending > > > >> infrastructure, whether that means changing test itself or the > > > architecture > > > >> in which it runs, to make our pipelines more reliable. If we smooth > > over > > > >> PR's that run into issues every time Stress Tests break a test which > > > only > > > >> had logging changes (or similar situations) then there's no > incentive > > > for > > > >> us to improve the Stress Tests job. > > > >> > > > >> Having said all that, being completely without the ability to > perform > > an > > > >> emergency override if everything goes sideways and the only way to > fix > > > it > > > >> is to push a commit which can't get a green pipeline seems like a > > pretty > > > >> good idea to me as long as we all agree on what circumstances > qualify > > > as an > > > >> emergency. > > > >> > > > >> On Thu, Oct 31, 2019 at 2:43 AM Ju@N <jujora...@gmail.com> wrote: > > > >> > > > >>> -1 for allowing overrides. > > > >>> If there's an edge case on which it's required, then we could use > it > > > at the > > > >>> very last resources *if and only if* it's been discussed and > approved > > > >>> through the dev list for that particular case. > > > >>> Cheers. > > > >>> > > > >>> > > > >>> On Wed, Oct 30, 2019 at 11:35 PM Robert Houghton < > > rhough...@pivotal.io > > > > > > > >>> wrote: > > > >>> > > > >>>> Any committer has the 'status' permission on apache/geode.git. > Some > > > API > > > >>>> tokens have it as well. Its curl + github API reasoning to do > this. > > > >>>> > > > >>>> On Wed, Oct 30, 2019 at 2:02 PM Jason Huynh <jhu...@pivotal.io> > > > wrote: > > > >>>> > > > >>>>> If we are going to allow overrides, then maybe what Owen is > > > describing > > > >>>>> should occur. Make a request on the dev list and explain the > > > >>> reasoning. > > > >>>>> I don't think this has been done and a few have already been > > > >>> overridden. > > > >>>>> Also who has the capability to override and knows how. How is > that > > > >>>>> determined? > > > >>>>> > > > >>>>> On Wed, Oct 30, 2019 at 1:59 PM Owen Nichols < > onich...@pivotal.io> > > > >>>> wrote: > > > >>>>>>> How do you override a check, anyway? > > > >>>>>> Much like asking for jira permissions, wiki permissions, etc, > just > > > >>> ask > > > >>>> on > > > >>>>>> the dev list ;) > > > >>>>>> > > > >>>>>> Presumably this type of request would be made as a “last resort” > > > >>>>> following > > > >>>>>> a dev list discussion wherein all other reasonable options had > > been > > > >>>>>> exhausted (reworking or splitting up the PR, pushing empty > > commits, > > > >>>>>> rebasing the PR, etc) > > > >>>>>> > > > >>>>>>> On Oct 30, 2019, at 1:42 PM, Dan Smith <dsm...@pivotal.io> > > wrote: > > > >>>>>>> > > > >>>>>>> +1 for allowing overrides. I think we should avoid backing > > > >>> ourselves > > > >>>>>> into a > > > >>>>>>> corner where we can't get anything into develop without talking > > to > > > >>>>> apache > > > >>>>>>> infra. Some infrastructure things we can't even fix without > > > >>> pushing a > > > >>>>>>> change develop! > > > >>>>>>> > > > >>>>>>> How do you override a check, anyway? > > > >>>>>>> > > > >>>>>>> -Dan > > > >>>>>>> > > > >>>>>>> On Wed, Oct 30, 2019 at 12:58 PM Donal Evans < > doev...@pivotal.io > > > > > > >>>>> wrote: > > > >>>>>>>> -1 to overriding from me. > > > >>>>>>>> > > > >>>>>>>> The question I have here is what's the rush? Is anything ever > so > > > >>>>>>>> time-sensitive that you can't even wait the 15 minutes it > takes > > > >>> for > > > >>>> it > > > >>>>>> to > > > >>>>>>>> build and run unit tests? If some infrastructure problem is > > > >>>> preventing > > > >>>>>>>> builds or tests from completing then that should be fixed > before > > > >>> any > > > >>>>> new > > > >>>>>>>> changes are added, otherwise what's the point in even having > the > > > >>> pre > > > >>>>>>>> check-in process? > > > >>>>>>>> > > > >>>>>>>> -Donal > > > >>>>>>>> > > > >>>>>>>> On Wed, Oct 30, 2019 at 11:44 AM Nabarun Nag <n...@apache.org > > > > > >>>> wrote: > > > >>>>>>>>> @Aaron > > > >>>>>>>>> It's okay to wait for at least the build, and unit tests to > > > >>>> complete, > > > >>>>>> to > > > >>>>>>>>> cover all the bases. [There may have been commits in between > > > >>> which > > > >>>>> may > > > >>>>>>>>> result in failure because of the revert] And it's not hard > to > > > >>> get > > > >>>> a > > > >>>>> PR > > > >>>>>>>>> approval. > > > >>>>>>>>> > > > >>>>>>>>> -1 on overriding. If the infrastructure is down, which is the > > > >>> test > > > >>>>>>>>> framework designed to ensure that we are not checking in > > unwanted > > > >>>>>> changes > > > >>>>>>>>> into Apache Geode, wait for the infrastructure to be up, get > > your > > > >>>>>> changes > > > >>>>>>>>> verified, get the review from a fellow committer and then > > > >>> check-in > > > >>>>> your > > > >>>>>>>>> changes. > > > >>>>>>>>> > > > >>>>>>>>> I still don't understand why will anyone not wait for unit > > tests > > > >>>> and > > > >>>>>>>> build > > > >>>>>>>>> to be successful. > > > >>>>>>>>> > > > >>>>>>>>> Regards > > > >>>>>>>>> Nabarun Nag > > > >>>>>>>>> > > > >>>>>>>>> On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey < > > > >>>> alind...@pivotal.io> > > > >>>>>>>>> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> One case when it might be acceptable to overrule a PR check > is > > > >>>>>>>> reverting > > > >>>>>>>>> a > > > >>>>>>>>>> commit. Before the branch protection was enabled, a > committer > > > >>>> could > > > >>>>>>>>> revert > > > >>>>>>>>>> a commit without a PR. Now that PRs are mandatory, we have > to > > > >>> wait > > > >>>>> for > > > >>>>>>>>> the > > > >>>>>>>>>> checks to run in order to revert a commit. Usually we are > > > >>>> reverting > > > >>>>> a > > > >>>>>>>>>> commit because it's causing problems, so I think overruling > > the > > > >>> PR > > > >>>>>>>> checks > > > >>>>>>>>>> may be acceptable in that case. > > > >>>>>>>>>> > > > >>>>>>>>>> - Aaron > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> On Wed, Oct 30, 2019 at 11:11 AM Owen Nichols < > > > >>>> onich...@pivotal.io> > > > >>>>>>>>> wrote: > > > >>>>>>>>>>> Our new branch-protection rules can sometimes lead to > > > >>> unexpected > > > >>>>>>>>>> obstacles > > > >>>>>>>>>>> when infrastructure issues impede the intended process. > > Should > > > >>>> we > > > >>>>>>>>>> discuss > > > >>>>>>>>>>> such cases as they come up, and should overruling the > result > > > >>> of a > > > >>>>> PR > > > >>>>>>>>>> check > > > >>>>>>>>>>> ever be an option on the table? > > > >>>>>>>>>>> > > > >>>>>>>>>>> -Owen > > > >>>>>> > > > >>> > > > >>> -- > > > >>> Ju@N > > > >>> > > > > > > > > > -- > > Cheers > > > > Jinmei > > >
