You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Owen Nichols <on...@pivotal.io> on 2019/10/30 18:04:08 UTC

[DISCUSS] is overriding a PR check ever justified?

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

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Jinmei Liao <ji...@pivotal.io>.
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 <nn...@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 <ji...@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 <ud...@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 <ju...@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 <jh...@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 <ds...@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
> > > >>>
> > >
> >
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Nabarun Nag <nn...@apache.org>.
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 <ji...@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 <ud...@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 <ju...@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 <jh...@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 <on...@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 <ds...@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 <nn...@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
> > >>>
> >
>
>
> --
> Cheers
>
> Jinmei
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Jinmei Liao <ji...@pivotal.io>.
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 <ud...@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 <ju...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
> >>>
>


-- 
Cheers

Jinmei

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Kirk Lund <kl...@apache.org>.
Does everyone realize that you just voted NO to what now needs to be done
for "[VOTE] Fix bad-merge of GEODE-7488"?

So right now, if we do not override the PR check, we have no way to fix the
PR checks.

On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io> wrote:

> Tallying the votes from this thread, it looks like the majority vote is to
> NEVER allow override even in extreme circumstance.
>
> Naba: -1
> Donal: -1
> Dan: +1
> Udo: +1 (extreme circumstances beyond 99.999999999% case)
> Juan: -1
> Ben: +1 (emergency 'break glass’)
> Mark: -1
>
>
> > On Oct 31, 2019, at 10:17 AM, Udo Kohlmeyer <ud...@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 <ju...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
> >>>>
>
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Robert Houghton <rh...@pivotal.io>.
Answering here, because it was asked:
@nnag This commit was allowed to merge because we test 'geode' code in the
PR pipeline, but we do not test the CI structure and scripts. This is to
protect the infrastructure from being hijacked to do things like mine
bitcoin, operate as a botnet, etc. The CI directory is what defined WHAT
runs, and HOW (what infrastructure, instance size, etc). So the CI used in
PR pipeline is from the develop branch.

So a PR was filed that could not have found this bug, and it was merged.
Then all future PRs use that bad CI script, and nothing ever passes again.

On Fri, Nov 22, 2019, 14:19 Jason Huynh <jh...@pivotal.io> wrote:

> @Udo- I believe that just noticing how often we have to override, might
> help influence what the correct decision or better yet, solution, might
> be.  Not necessarily needing a vote email but just an email that describes
> why we needed to override.  I think it will help us get a better
> understanding of when we have had to override and might show us a trend
> over time on what issues or areas we may better coverage in.  Personally
> I'd prefer if we'd have to override less often and in the open when we do.
>
> On Fri, Nov 22, 2019 at 2:02 PM Udo Kohlmeyer <ud...@apache.com> wrote:
>
> > @Jason, thank you for clarification.. I just pointed out to @Naba that
> > it was the wrong thread. As, like in many cases, the original intent of
> > the thread is lost because someone has asked a question, not directly
> > relating to the thread and then the whole thread is derailed.
> >
> > When I asked for a vote, I was asking should we be voting on whether we
> > should allow overrides. It was inconclusive, with 4 against and 3 for
> > overrides. Which really leaves us in a position where some feel we
> > should allow the "break glass emergency" override of branch protection
> > and some don't, and the rest of the 90+ committers who don't care.
> >
> > Are you suggesting that every override now becomes a vote on the dev
> > list? Given that we don't have a real stance on whether we allow it or
> > not, maybe that is best UNTIL we hit a scenario where we cannot get
> > consensus on the override.
> >
> > --Udo
> >
> > On 11/22/19 1:06 PM, Jason Huynh wrote:
> > > @Udo - I think Naba was asking why the original commit that broke the
> > > pipeline wasn't detected.
> > >
> > > I think instead of a vote email, an email alerting the dev list that an
> > > override needs to take place is still good to have.  If nothing else,
> to
> > > identify areas that we might be able to improve with additional
> coverage
> > or
> > > checks.
> > >
> > >
> > >
> > >
> > > On Fri, Nov 22, 2019 at 12:40 PM Udo Kohlmeyer <uk...@pivotal.io>
> > > wrote:
> > >
> > >> @Naba.. wrong thread :)
> > >>
> > >> We have real scenario here now, where we have no consensus on whether
> we
> > >> are allowed or not allowed to override.. Do we vote now? OR do we
> apply
> > >> common sense?
> > >>
> > >> TBH, at this junction we should really just do whatever we believe is
> > >> correct. A committer is appointed due to trust, so we should trust
> that
> > >> our committers will do the right thing.
> > >>
> > >> But the same trust that our committers would always do the right thing
> > >> has gotten us to this point where we don't trust....
> > >>
> > >> MUCH bigger chicken-and-egg problem.
> > >>
> > >> I motion that we vote on this. I would also like to request all those
> > >> AGAINST the override to provide strategies for us to not
> shoot-ourselves
> > >> in the foot. (like Dan said)
> > >>
> > >> --Udo
> > >>
> > >> On 11/22/19 12:30 PM, Nabarun Nag wrote:
> > >>> Just out of curiosity, why did the PR checks for GEODE-7488 not fail
> > and
> > >>> allowed it be merged? Is something lacking in our testing?
> > >>>
> > >>> On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io>
> wrote:
> > >>>
> > >>>> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io>
> > >> wrote:
> > >>>>> Tallying the votes from this thread, it looks like the majority
> vote
> > is
> > >>>> to
> > >>>>> NEVER allow override even in extreme circumstance.
> > >>>>>
> > >>>> I think a better way of summarizing this thread so far is that there
> > >> isn't
> > >>>> really a consensus on this point, opinions seem to be fairly split.
> > This
> > >>>> wasn't a vote, and not everybody who expressed an opinion put a
> number
> > >> next
> > >>>> to their opinion or was directly aligned with the statement above.
> > >>>>
> > >>>> Maybe folks who think there should not be an override option could
> > >> propose
> > >>>> a specific process for dealing with issues like what Robert just did
> > and
> > >>>> try to bring the rest of us on board with that?
> > >>>>
> > >>>> -Dan
> > >>>>
> >
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Jason Huynh <jh...@pivotal.io>.
@Udo- I believe that just noticing how often we have to override, might
help influence what the correct decision or better yet, solution, might
be.  Not necessarily needing a vote email but just an email that describes
why we needed to override.  I think it will help us get a better
understanding of when we have had to override and might show us a trend
over time on what issues or areas we may better coverage in.  Personally
I'd prefer if we'd have to override less often and in the open when we do.

On Fri, Nov 22, 2019 at 2:02 PM Udo Kohlmeyer <ud...@apache.com> wrote:

> @Jason, thank you for clarification.. I just pointed out to @Naba that
> it was the wrong thread. As, like in many cases, the original intent of
> the thread is lost because someone has asked a question, not directly
> relating to the thread and then the whole thread is derailed.
>
> When I asked for a vote, I was asking should we be voting on whether we
> should allow overrides. It was inconclusive, with 4 against and 3 for
> overrides. Which really leaves us in a position where some feel we
> should allow the "break glass emergency" override of branch protection
> and some don't, and the rest of the 90+ committers who don't care.
>
> Are you suggesting that every override now becomes a vote on the dev
> list? Given that we don't have a real stance on whether we allow it or
> not, maybe that is best UNTIL we hit a scenario where we cannot get
> consensus on the override.
>
> --Udo
>
> On 11/22/19 1:06 PM, Jason Huynh wrote:
> > @Udo - I think Naba was asking why the original commit that broke the
> > pipeline wasn't detected.
> >
> > I think instead of a vote email, an email alerting the dev list that an
> > override needs to take place is still good to have.  If nothing else, to
> > identify areas that we might be able to improve with additional coverage
> or
> > checks.
> >
> >
> >
> >
> > On Fri, Nov 22, 2019 at 12:40 PM Udo Kohlmeyer <uk...@pivotal.io>
> > wrote:
> >
> >> @Naba.. wrong thread :)
> >>
> >> We have real scenario here now, where we have no consensus on whether we
> >> are allowed or not allowed to override.. Do we vote now? OR do we apply
> >> common sense?
> >>
> >> TBH, at this junction we should really just do whatever we believe is
> >> correct. A committer is appointed due to trust, so we should trust that
> >> our committers will do the right thing.
> >>
> >> But the same trust that our committers would always do the right thing
> >> has gotten us to this point where we don't trust....
> >>
> >> MUCH bigger chicken-and-egg problem.
> >>
> >> I motion that we vote on this. I would also like to request all those
> >> AGAINST the override to provide strategies for us to not shoot-ourselves
> >> in the foot. (like Dan said)
> >>
> >> --Udo
> >>
> >> On 11/22/19 12:30 PM, Nabarun Nag wrote:
> >>> Just out of curiosity, why did the PR checks for GEODE-7488 not fail
> and
> >>> allowed it be merged? Is something lacking in our testing?
> >>>
> >>> On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:
> >>>
> >>>> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io>
> >> wrote:
> >>>>> Tallying the votes from this thread, it looks like the majority vote
> is
> >>>> to
> >>>>> NEVER allow override even in extreme circumstance.
> >>>>>
> >>>> I think a better way of summarizing this thread so far is that there
> >> isn't
> >>>> really a consensus on this point, opinions seem to be fairly split.
> This
> >>>> wasn't a vote, and not everybody who expressed an opinion put a number
> >> next
> >>>> to their opinion or was directly aligned with the statement above.
> >>>>
> >>>> Maybe folks who think there should not be an override option could
> >> propose
> >>>> a specific process for dealing with issues like what Robert just did
> and
> >>>> try to bring the rest of us on board with that?
> >>>>
> >>>> -Dan
> >>>>
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Udo Kohlmeyer <ud...@apache.com>.
@Jason, thank you for clarification.. I just pointed out to @Naba that 
it was the wrong thread. As, like in many cases, the original intent of 
the thread is lost because someone has asked a question, not directly 
relating to the thread and then the whole thread is derailed.

When I asked for a vote, I was asking should we be voting on whether we 
should allow overrides. It was inconclusive, with 4 against and 3 for 
overrides. Which really leaves us in a position where some feel we 
should allow the "break glass emergency" override of branch protection 
and some don't, and the rest of the 90+ committers who don't care.

Are you suggesting that every override now becomes a vote on the dev 
list? Given that we don't have a real stance on whether we allow it or 
not, maybe that is best UNTIL we hit a scenario where we cannot get 
consensus on the override.

--Udo

On 11/22/19 1:06 PM, Jason Huynh wrote:
> @Udo - I think Naba was asking why the original commit that broke the
> pipeline wasn't detected.
>
> I think instead of a vote email, an email alerting the dev list that an
> override needs to take place is still good to have.  If nothing else, to
> identify areas that we might be able to improve with additional coverage or
> checks.
>
>
>
>
> On Fri, Nov 22, 2019 at 12:40 PM Udo Kohlmeyer <uk...@pivotal.io>
> wrote:
>
>> @Naba.. wrong thread :)
>>
>> We have real scenario here now, where we have no consensus on whether we
>> are allowed or not allowed to override.. Do we vote now? OR do we apply
>> common sense?
>>
>> TBH, at this junction we should really just do whatever we believe is
>> correct. A committer is appointed due to trust, so we should trust that
>> our committers will do the right thing.
>>
>> But the same trust that our committers would always do the right thing
>> has gotten us to this point where we don't trust....
>>
>> MUCH bigger chicken-and-egg problem.
>>
>> I motion that we vote on this. I would also like to request all those
>> AGAINST the override to provide strategies for us to not shoot-ourselves
>> in the foot. (like Dan said)
>>
>> --Udo
>>
>> On 11/22/19 12:30 PM, Nabarun Nag wrote:
>>> Just out of curiosity, why did the PR checks for GEODE-7488 not fail and
>>> allowed it be merged? Is something lacking in our testing?
>>>
>>> On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:
>>>
>>>> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io>
>> wrote:
>>>>> Tallying the votes from this thread, it looks like the majority vote is
>>>> to
>>>>> NEVER allow override even in extreme circumstance.
>>>>>
>>>> I think a better way of summarizing this thread so far is that there
>> isn't
>>>> really a consensus on this point, opinions seem to be fairly split. This
>>>> wasn't a vote, and not everybody who expressed an opinion put a number
>> next
>>>> to their opinion or was directly aligned with the statement above.
>>>>
>>>> Maybe folks who think there should not be an override option could
>> propose
>>>> a specific process for dealing with issues like what Robert just did and
>>>> try to bring the rest of us on board with that?
>>>>
>>>> -Dan
>>>>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Jason Huynh <jh...@pivotal.io>.
@Udo - I think Naba was asking why the original commit that broke the
pipeline wasn't detected.

I think instead of a vote email, an email alerting the dev list that an
override needs to take place is still good to have.  If nothing else, to
identify areas that we might be able to improve with additional coverage or
checks.




On Fri, Nov 22, 2019 at 12:40 PM Udo Kohlmeyer <uk...@pivotal.io>
wrote:

> @Naba.. wrong thread :)
>
> We have real scenario here now, where we have no consensus on whether we
> are allowed or not allowed to override.. Do we vote now? OR do we apply
> common sense?
>
> TBH, at this junction we should really just do whatever we believe is
> correct. A committer is appointed due to trust, so we should trust that
> our committers will do the right thing.
>
> But the same trust that our committers would always do the right thing
> has gotten us to this point where we don't trust....
>
> MUCH bigger chicken-and-egg problem.
>
> I motion that we vote on this. I would also like to request all those
> AGAINST the override to provide strategies for us to not shoot-ourselves
> in the foot. (like Dan said)
>
> --Udo
>
> On 11/22/19 12:30 PM, Nabarun Nag wrote:
> > Just out of curiosity, why did the PR checks for GEODE-7488 not fail and
> > allowed it be merged? Is something lacking in our testing?
> >
> > On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:
> >
> >> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io>
> wrote:
> >>
> >>> Tallying the votes from this thread, it looks like the majority vote is
> >> to
> >>> NEVER allow override even in extreme circumstance.
> >>>
> >> I think a better way of summarizing this thread so far is that there
> isn't
> >> really a consensus on this point, opinions seem to be fairly split. This
> >> wasn't a vote, and not everybody who expressed an opinion put a number
> next
> >> to their opinion or was directly aligned with the statement above.
> >>
> >> Maybe folks who think there should not be an override option could
> propose
> >> a specific process for dealing with issues like what Robert just did and
> >> try to bring the rest of us on board with that?
> >>
> >> -Dan
> >>
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Udo Kohlmeyer <uk...@pivotal.io>.
@Naba.. wrong thread :)

We have real scenario here now, where we have no consensus on whether we 
are allowed or not allowed to override.. Do we vote now? OR do we apply 
common sense?

TBH, at this junction we should really just do whatever we believe is 
correct. A committer is appointed due to trust, so we should trust that 
our committers will do the right thing.

But the same trust that our committers would always do the right thing 
has gotten us to this point where we don't trust....

MUCH bigger chicken-and-egg problem.

I motion that we vote on this. I would also like to request all those 
AGAINST the override to provide strategies for us to not shoot-ourselves 
in the foot. (like Dan said)

--Udo

On 11/22/19 12:30 PM, Nabarun Nag wrote:
> Just out of curiosity, why did the PR checks for GEODE-7488 not fail and
> allowed it be merged? Is something lacking in our testing?
>
> On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:
>
>> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io> wrote:
>>
>>> Tallying the votes from this thread, it looks like the majority vote is
>> to
>>> NEVER allow override even in extreme circumstance.
>>>
>> I think a better way of summarizing this thread so far is that there isn't
>> really a consensus on this point, opinions seem to be fairly split. This
>> wasn't a vote, and not everybody who expressed an opinion put a number next
>> to their opinion or was directly aligned with the statement above.
>>
>> Maybe folks who think there should not be an override option could propose
>> a specific process for dealing with issues like what Robert just did and
>> try to bring the rest of us on board with that?
>>
>> -Dan
>>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Nabarun Nag <nn...@apache.org>.
Just out of curiosity, why did the PR checks for GEODE-7488 not fail and
allowed it be merged? Is something lacking in our testing?

On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:

> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io> wrote:
>
> > Tallying the votes from this thread, it looks like the majority vote is
> to
> > NEVER allow override even in extreme circumstance.
> >
>
> I think a better way of summarizing this thread so far is that there isn't
> really a consensus on this point, opinions seem to be fairly split. This
> wasn't a vote, and not everybody who expressed an opinion put a number next
> to their opinion or was directly aligned with the statement above.
>
> Maybe folks who think there should not be an override option could propose
> a specific process for dealing with issues like what Robert just did and
> try to bring the rest of us on board with that?
>
> -Dan
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Donal Evans <do...@pivotal.io>.
In light of the recent issue with GEODE-7488, I think I should
clarify/expand what I said earlier in this thread. While I'm definitely
against bypassing PR checks when it's done just for convenience or
impatience, I agree with Ben and Udo's stance that in "emergencies" it
should be at least considered.

If something is broken in such a way that the ONLY way to fix it is to
force through a commit and bypass PR checks then there's not really an
argument against that. However, if it's a case of "X test is failing
because that test has underlying problems" or something similar, then the
solution should be to fix the test and improve the quality of our pipeline
rather than just sweep it under the rug.

On Fri, Nov 22, 2019 at 12:19 PM Dan Smith <ds...@pivotal.io> wrote:

> On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io> wrote:
>
> > Tallying the votes from this thread, it looks like the majority vote is
> to
> > NEVER allow override even in extreme circumstance.
> >
>
> I think a better way of summarizing this thread so far is that there isn't
> really a consensus on this point, opinions seem to be fairly split. This
> wasn't a vote, and not everybody who expressed an opinion put a number next
> to their opinion or was directly aligned with the statement above.
>
> Maybe folks who think there should not be an override option could propose
> a specific process for dealing with issues like what Robert just did and
> try to bring the rest of us on board with that?
>
> -Dan
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Dan Smith <ds...@pivotal.io>.
On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <on...@pivotal.io> wrote:

> Tallying the votes from this thread, it looks like the majority vote is to
> NEVER allow override even in extreme circumstance.
>

I think a better way of summarizing this thread so far is that there isn't
really a consensus on this point, opinions seem to be fairly split. This
wasn't a vote, and not everybody who expressed an opinion put a number next
to their opinion or was directly aligned with the statement above.

Maybe folks who think there should not be an override option could propose
a specific process for dealing with issues like what Robert just did and
try to bring the rest of us on board with that?

-Dan

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Owen Nichols <on...@pivotal.io>.
Tallying the votes from this thread, it looks like the majority vote is to NEVER allow override even in extreme circumstance.

Naba: -1
Donal: -1
Dan: +1
Udo: +1 (extreme circumstances beyond 99.999999999% case)
Juan: -1
Ben: +1 (emergency 'break glass’)
Mark: -1 


> On Oct 31, 2019, at 10:17 AM, Udo Kohlmeyer <ud...@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 <ju...@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 <rh...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
>>>> 


Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Udo Kohlmeyer <ud...@apache.com>.
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 <ju...@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 <rh...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
>>>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Mark Hanson <mh...@pivotal.io>.
-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 <ju...@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 <rh...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
>> 


Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Benjamin Ross <br...@pivotal.io>.
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 <ju...@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 <rh...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by "Ju@N" <ju...@gmail.com>.
-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 <rh...@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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Robert Houghton <rh...@pivotal.io>.
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 <jh...@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 <on...@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 <ds...@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 <do...@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 <nn...@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 <al...@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 <on...@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
> > >>>>
> > >>>
> > >>
> >
> >
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Jason Huynh <jh...@pivotal.io>.
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 <on...@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 <ds...@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 <do...@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 <nn...@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 <al...@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 <on...@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
> >>>>
> >>>
> >>
>
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Udo Kohlmeyer <ud...@apache.com>.
+1 to override in extreme circumstances

-1 to have an override flag with simple access for the common man.

I think we need a "break glass" override..

but not something that is easily accessible. The 99.999999999% case has 
to go through the current process and constraints...

BUT I think we need to have a "break glass" capability.

--Udo

On 10/30/19 1:58 PM, Owen Nichols 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 <ds...@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 <do...@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 <nn...@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 <al...@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 <on...@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

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Owen Nichols <on...@pivotal.io>.
> 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 <ds...@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 <do...@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 <nn...@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 <al...@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 <on...@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
>>>> 
>>> 
>> 


Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Dan Smith <ds...@pivotal.io>.
+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 <do...@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 <nn...@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 <al...@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 <on...@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
> > >
> >
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Donal Evans <do...@pivotal.io>.
-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 <nn...@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 <al...@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 <on...@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
> >
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Nabarun Nag <nn...@apache.org>.
@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 <al...@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 <on...@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
>

Re: [DISCUSS] is overriding a PR check ever justified?

Posted by Aaron Lindsey <al...@pivotal.io>.
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 <on...@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