geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hanson <mhan...@pivotal.io>
Subject Re: [DISCUSS] is overriding a PR check ever justified?
Date Thu, 31 Oct 2019 16:58:21 GMT
-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 <bross@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 <jujoramos@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 <rhoughton@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 <jhuynh@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 <onichols@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 <dsmith@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 <doevans@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 <nnag@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 <
>>> alindsey@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 <
>>> onichols@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
>> 


Mime
View raw message