Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Xiaojian Zhou
It finished after 4 hour 51 minutes. It looks like we do need to increase
the timeout for stressNewTest.

On Thu, Oct 31, 2019 at 4:45 PM Darrel Schneider 
wrote:

> +1
>
> On Thu, Oct 31, 2019 at 4:16 PM Jinmei Liao  wrote:
>
> > +1
> >
> > On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou  wrote:
> >
> > > I'm curious to see the new stressNew test result too.
> > >
> > > On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols 
> > wrote:
> > >
> > > > I’ve retriggered StressNew <
> > > >
> > >
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> > > >
> > > > with a temporarily-increased timeout of 12 hours so we can see how
> long
> > > it
> > > > would actually take, to have some data point whether to propose a
> > > permanent
> > > > timeout increase or whether breaking up into multiple PRs is should
> be
> > > the
> > > > standard way to get around this.
> > > >
> > > > > On Oct 31, 2019, at 2:52 PM, Donal Evans 
> wrote:
> > > > >
> > > > > +1 to allowing this PR to be merged, although I'd lean strongly
> > toward
> > > > > facilitating this by temporarily increasing the timeout on the job
> to
> > > > allow
> > > > > it to actually pass rather than a manual override of the
> > StressNewTest.
> > > > >
> > > > > The fact that it's passed over 7000 times without failing is pretty
> > > > strong
> > > > > evidence that it's not a flaky test, which is what StressNewTest is
> > > > > supposed to catch, so there doesn't seem to be any risk associated
> > with
> > > > > circumventing it in this case, but if there's a feasible solution
> > that
> > > > > doesn't involve "cheating" or ignoring the test job, then that
> would
> > be
> > > > > preferable.
> > > > >
> > > > > - Donal
> > > > >
> > > > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh 
> > wrote:
> > > > >
> > > > >> Greetings,
> > > > >>
> > > > >> We have a pull request (https://github.com/apache/geode/pull/4250
> )
> > > > that is
> > > > >> running into a problem with stressNewTest.  Mostly the tests that
> > are
> > > > being
> > > > >> run are RollingUpgrade tests that take quite a bit of time to run
> > the
> > > > full
> > > > >> suite.  Because these tests are added/modified, the stressNewTest
> > > > doesn't
> > > > >> have enough time to complete the run because it runs them N(50)
> > number
> > > > of
> > > > >> times.
> > > > >>
> > > > >> However what has completed is 7400 tests and none of them have
> > failed:
> > > > >>
> > > > >>
> > > >
> > >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > > > >>
> > > > >> We would like to get this fix in before branching the next
> release,
> > > but
> > > > are
> > > > >> unable to due to stressNewTest gating the merge button.  I know we
> > > have
> > > > >> another thread about overrides etc, and maybe this is a data
> point,
> > > but
> > > > >> this isn't meant to discuss that.
> > > > >>
> > > > >> Would everyone be able to agree to allow someone to manually
> > override
> > > > and
> > > > >> merge this commit in (title of PR and reviews pending)?
> > > > >>
> > > >
> > > >
> > >
> >
>


Re: Re: unable to push

2019-10-31 Thread Nabarun Nag
It is how Github branch protection is designed.

This is my explanation from the information provided by you :

- The Pull Request you mentioned had 11 commits and when you tried to
merged it, I assume you tried to squash it to a single commit. It generates
a new SHA.
- When you are pushing that SHA to origin develop, GitHub could not find
that SHA in the list of approved Pull Requests SHAs.
- This is why it blocked your push to develop origin.

Workaround:
- This would have worked if there was only one commit in the Pull Request:
- If you still need to use the command line, you can create the Pull
Request with only one commit (after squashing).
- I generally, commit --amend over the last commit SHA and then force push,
so that it stays as a single commit.

Regards
Naba


On Thu, Oct 31, 2019 at 6:56 PM Bruce Schuchardt 
wrote:

> I just want to know why my PR that passed the tests and was approved
> couldn't be pushed from the command line Naba
>
> On 10/31/19 5:11 PM, Nabarun Nag wrote:
> > Hi Bruce,
> >
> > This was what was discussed in multiple email chains last week. GitHub
> > branch protection was enabled on the develop branch.
> >
> > Command-line and the merge button on the website have the same effect.
> > This
> > has now being implemented in a lot of Apache projects and we are
> > implementing it as the geode community is growing, to prevent
> > unintentional
> > red pipelines.
> >
> > It is a small inconvenience paid for saving a lot of our time in
> detecting
> > which commit caused the red pipeline or which commit introduced a flaky
> > test. Our time can be used in other productive work.
> >
> > Kindly reconsider as a majority of us have already moved to the GitHub
> > merge system on the website.
> >
> > Regards
> > Nabarun
> >
> >
> >
> > On Thu, Oct 31, 2019 at 4:45 PM Robert Houghton 
> > wrote:
> >
> >> Was there a pull request for this SHA?
> >>
> >> On Thu, Oct 31, 2019, 16:36 Bruce Schuchardt 
> >> wrote:
> >>
> >>> I just completed GEODE-7358 and was prevented from pushing from the
> >>> command-line. The Merge button on github worked, but why can't I have
> >>> command-line control of the process? I don't like giving control of my
> >>> merge to a web-site button. We should revert this change!
> >>>
> >>>
> >>> geode> git push --no-verify origin develop
> >>> Enumerating objects: 352, done.
> >>> Counting objects: 100% (352/352), done.
> >>> Delta compression using up to 8 threads
> >>> Compressing objects: 100% (192/192), done.
> >>> Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
> >>> Total 223 (delta 106), reused 59 (delta 3)
> >>> remote: Resolving deltas: 100% (106/106), completed with 93 local
> >> objects.
> >>> remote: error: GH006: Protected branch update failed for
> >>> refs/heads/develop.
> >>> remote: error: 4 of 4 required status checks are expected. At least 1
> >>> approving review is required by reviewers with write access.
> >>> To ssh://github.com/apache/geode.git
> >>> ! [remote rejected] develop -> develop (protected branch hook
> >>> declined)
> >>> error: failed to push some refs to 'ssh://
> >> g...@github.com/apache/geode.git'
> >>>
>


Fwd: Re: unable to push

2019-10-31 Thread Bruce Schuchardt
I just want to know why my PR that passed the tests and was approved 
couldn't be pushed from the command line Naba


On 10/31/19 5:11 PM, Nabarun Nag wrote:

Hi Bruce,

This was what was discussed in multiple email chains last week. GitHub
branch protection was enabled on the develop branch.

Command-line and the merge button on the website have the same effect. 
This

has now being implemented in a lot of Apache projects and we are
implementing it as the geode community is growing, to prevent 
unintentional

red pipelines.

It is a small inconvenience paid for saving a lot of our time in detecting
which commit caused the red pipeline or which commit introduced a flaky
test. Our time can be used in other productive work.

Kindly reconsider as a majority of us have already moved to the GitHub
merge system on the website.

Regards
Nabarun



On Thu, Oct 31, 2019 at 4:45 PM Robert Houghton 
wrote:


Was there a pull request for this SHA?

On Thu, Oct 31, 2019, 16:36 Bruce Schuchardt 
wrote:


I just completed GEODE-7358 and was prevented from pushing from the
command-line. The Merge button on github worked, but why can't I have
command-line control of the process? I don't like giving control of my
merge to a web-site button. We should revert this change!


geode> git push --no-verify origin develop
Enumerating objects: 352, done.
Counting objects: 100% (352/352), done.
Delta compression using up to 8 threads
Compressing objects: 100% (192/192), done.
Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
Total 223 (delta 106), reused 59 (delta 3)
remote: Resolving deltas: 100% (106/106), completed with 93 local

objects.

remote: error: GH006: Protected branch update failed for
refs/heads/develop.
remote: error: 4 of 4 required status checks are expected. At least 1
approving review is required by reviewers with write access.
To ssh://github.com/apache/geode.git
! [remote rejected] develop -> develop (protected branch hook
declined)
error: failed to push some refs to 'ssh://

g...@github.com/apache/geode.git'




Re: unable to push

2019-10-31 Thread Bruce Schuchardt
I just want to know why my PR that passed the tests and was approved 
couldn't be pushed from the command line Nab


On 10/31/19 5:11 PM, Nabarun Nag wrote:

Hi Bruce,

This was what was discussed in multiple email chains last week. GitHub
branch protection was enabled on the develop branch.

Command-line and the merge button on the website have the same effect. This
has now being implemented in a lot of Apache projects and we are
implementing it as the geode community is growing, to prevent unintentional
red pipelines.

It is a small inconvenience paid for saving a lot of our time in detecting
which commit caused the red pipeline or which commit introduced a flaky
test. Our time can be used in other productive work.

Kindly reconsider as a majority of us have already moved to the GitHub
merge system on the website.

Regards
Nabarun



On Thu, Oct 31, 2019 at 4:45 PM Robert Houghton 
wrote:


Was there a pull request for this SHA?

On Thu, Oct 31, 2019, 16:36 Bruce Schuchardt 
wrote:


I just completed GEODE-7358 and was prevented from pushing from the
command-line.  The Merge button on github worked, but why can't I have
command-line control of the process?  I don't like giving control of my
merge to a web-site button.  We should revert this change!


geode> git push --no-verify origin develop
Enumerating objects: 352, done.
Counting objects: 100% (352/352), done.
Delta compression using up to 8 threads
Compressing objects: 100% (192/192), done.
Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
Total 223 (delta 106), reused 59 (delta 3)
remote: Resolving deltas: 100% (106/106), completed with 93 local

objects.

remote: error: GH006: Protected branch update failed for
refs/heads/develop.
remote: error: 4 of 4 required status checks are expected. At least 1
approving review is required by reviewers with write access.
To ssh://github.com/apache/geode.git
   ! [remote rejected]   develop -> develop (protected branch hook
declined)
error: failed to push some refs to 'ssh://

g...@github.com/apache/geode.git'




Re: unable to push

2019-10-31 Thread Nabarun Nag
Hi Bruce,

This was what was discussed in multiple email chains last week. GitHub
branch protection was enabled on the develop branch.

Command-line and the merge button on the website have the same effect. This
has now being implemented in a lot of Apache projects and we are
implementing it as the geode community is growing, to prevent unintentional
red pipelines.

It is a small inconvenience paid for saving a lot of our time in detecting
which commit caused the red pipeline or which commit introduced a flaky
test. Our time can be used in other productive work.

Kindly reconsider as a majority of us have already moved to the GitHub
merge system on the website.

Regards
Nabarun



On Thu, Oct 31, 2019 at 4:45 PM Robert Houghton 
wrote:

> Was there a pull request for this SHA?
>
> On Thu, Oct 31, 2019, 16:36 Bruce Schuchardt 
> wrote:
>
> > I just completed GEODE-7358 and was prevented from pushing from the
> > command-line.  The Merge button on github worked, but why can't I have
> > command-line control of the process?  I don't like giving control of my
> > merge to a web-site button.  We should revert this change!
> >
> >
> > geode> git push --no-verify origin develop
> > Enumerating objects: 352, done.
> > Counting objects: 100% (352/352), done.
> > Delta compression using up to 8 threads
> > Compressing objects: 100% (192/192), done.
> > Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
> > Total 223 (delta 106), reused 59 (delta 3)
> > remote: Resolving deltas: 100% (106/106), completed with 93 local
> objects.
> > remote: error: GH006: Protected branch update failed for
> > refs/heads/develop.
> > remote: error: 4 of 4 required status checks are expected. At least 1
> > approving review is required by reviewers with write access.
> > To ssh://github.com/apache/geode.git
> >   ! [remote rejected]   develop -> develop (protected branch hook
> > declined)
> > error: failed to push some refs to 'ssh://
> g...@github.com/apache/geode.git'
> >
> >
>


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

2019-10-31 Thread Jinmei Liao
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  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  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  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 
> 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  wrote:
> > > >>
> > > >>> -1 for allowing overrides.
> > > >>> If there's an edge case on which it's required, then we could use
> it
> > > at the
> > > >>> very last resources *if and only if* it's been discussed and
> approved
> > > >>> through the dev list for that particular case.
> > > >>> Cheers.
> > > >>>
> > > >>>
> > > >>> On Wed, Oct 30, 2019 at 11:35 PM Robert Houghton <
> > rhough...@pivotal.io
> > > >
> > > >>> wrote:
> > > >>>
> > >  Any committer has the 'status' permission on apache/geode.git.
> Some
> > > API
> > >  tokens have it as well. Its curl + github API reasoning to do
> this.
> > > 
> > >  On Wed, Oct 30, 2019 at 2:02 PM Jason Huynh 
> > > wrote:
> > > 
> > > > If we are going to allow overrides, then maybe what Owen is
> > > describing
> > > > should occur.  Make a request on the dev list and explain the
> > > >>> reasoning.
> > > > I don't think this has been done and a few have already been
> > > >>> overridden.
> > > > Also who has the capability to override and knows how.  How is
> that
> > > > determined?
> > > >
> > > > On Wed, Oct 30, 2019 at 1:59 PM Owen Nichols <
> onich...@pivotal.io>
> > >  wrote:
> > > >>> How do you override a check, anyway?
> > > >> Much like asking for jira permissions, wiki permissions, etc,
> just
> > > >>> ask
> > >  on
> > > >> the dev list ;)
> > > >>
> > > >> Presumably this type of request would be made as a “last resort”
> > > > following
> > > >> a dev list discussion wherein all other reasonable options had
> > been
> > > >> exhausted (reworking or splitting up the PR, pushing emp

Re: unable to push

2019-10-31 Thread Robert Houghton
Was there a pull request for this SHA?

On Thu, Oct 31, 2019, 16:36 Bruce Schuchardt  wrote:

> I just completed GEODE-7358 and was prevented from pushing from the
> command-line.  The Merge button on github worked, but why can't I have
> command-line control of the process?  I don't like giving control of my
> merge to a web-site button.  We should revert this change!
>
>
> geode> git push --no-verify origin develop
> Enumerating objects: 352, done.
> Counting objects: 100% (352/352), done.
> Delta compression using up to 8 threads
> Compressing objects: 100% (192/192), done.
> Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
> Total 223 (delta 106), reused 59 (delta 3)
> remote: Resolving deltas: 100% (106/106), completed with 93 local objects.
> remote: error: GH006: Protected branch update failed for
> refs/heads/develop.
> remote: error: 4 of 4 required status checks are expected. At least 1
> approving review is required by reviewers with write access.
> To ssh://github.com/apache/geode.git
>   ! [remote rejected]   develop -> develop (protected branch hook
> declined)
> error: failed to push some refs to 'ssh://g...@github.com/apache/geode.git'
>
>


Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Darrel Schneider
+1

On Thu, Oct 31, 2019 at 4:16 PM Jinmei Liao  wrote:

> +1
>
> On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou  wrote:
>
> > I'm curious to see the new stressNew test result too.
> >
> > On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols 
> wrote:
> >
> > > I’ve retriggered StressNew <
> > >
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> > >
> > > with a temporarily-increased timeout of 12 hours so we can see how long
> > it
> > > would actually take, to have some data point whether to propose a
> > permanent
> > > timeout increase or whether breaking up into multiple PRs is should be
> > the
> > > standard way to get around this.
> > >
> > > > On Oct 31, 2019, at 2:52 PM, Donal Evans  wrote:
> > > >
> > > > +1 to allowing this PR to be merged, although I'd lean strongly
> toward
> > > > facilitating this by temporarily increasing the timeout on the job to
> > > allow
> > > > it to actually pass rather than a manual override of the
> StressNewTest.
> > > >
> > > > The fact that it's passed over 7000 times without failing is pretty
> > > strong
> > > > evidence that it's not a flaky test, which is what StressNewTest is
> > > > supposed to catch, so there doesn't seem to be any risk associated
> with
> > > > circumventing it in this case, but if there's a feasible solution
> that
> > > > doesn't involve "cheating" or ignoring the test job, then that would
> be
> > > > preferable.
> > > >
> > > > - Donal
> > > >
> > > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh 
> wrote:
> > > >
> > > >> Greetings,
> > > >>
> > > >> We have a pull request (https://github.com/apache/geode/pull/4250)
> > > that is
> > > >> running into a problem with stressNewTest.  Mostly the tests that
> are
> > > being
> > > >> run are RollingUpgrade tests that take quite a bit of time to run
> the
> > > full
> > > >> suite.  Because these tests are added/modified, the stressNewTest
> > > doesn't
> > > >> have enough time to complete the run because it runs them N(50)
> number
> > > of
> > > >> times.
> > > >>
> > > >> However what has completed is 7400 tests and none of them have
> failed:
> > > >>
> > > >>
> > >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > > >>
> > > >> We would like to get this fix in before branching the next release,
> > but
> > > are
> > > >> unable to due to stressNewTest gating the merge button.  I know we
> > have
> > > >> another thread about overrides etc, and maybe this is a data point,
> > but
> > > >> this isn't meant to discuss that.
> > > >>
> > > >> Would everyone be able to agree to allow someone to manually
> override
> > > and
> > > >> merge this commit in (title of PR and reviews pending)?
> > > >>
> > >
> > >
> >
>


unable to push

2019-10-31 Thread Bruce Schuchardt
I just completed GEODE-7358 and was prevented from pushing from the 
command-line.  The Merge button on github worked, but why can't I have 
command-line control of the process?  I don't like giving control of my 
merge to a web-site button.  We should revert this change!



geode> git push --no-verify origin develop
Enumerating objects: 352, done.
Counting objects: 100% (352/352), done.
Delta compression using up to 8 threads
Compressing objects: 100% (192/192), done.
Writing objects: 100% (223/223), 153.60 KiB | 9.04 MiB/s, done.
Total 223 (delta 106), reused 59 (delta 3)
remote: Resolving deltas: 100% (106/106), completed with 93 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/develop.
remote: error: 4 of 4 required status checks are expected. At least 1 
approving review is required by reviewers with write access.

To ssh://github.com/apache/geode.git
 ! [remote rejected]   develop -> develop (protected branch hook 
declined)

error: failed to push some refs to 'ssh://g...@github.com/apache/geode.git'



Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Jinmei Liao
+1

On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou  wrote:

> I'm curious to see the new stressNew test result too.
>
> On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols  wrote:
>
> > I’ve retriggered StressNew <
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> >
> > with a temporarily-increased timeout of 12 hours so we can see how long
> it
> > would actually take, to have some data point whether to propose a
> permanent
> > timeout increase or whether breaking up into multiple PRs is should be
> the
> > standard way to get around this.
> >
> > > On Oct 31, 2019, at 2:52 PM, Donal Evans  wrote:
> > >
> > > +1 to allowing this PR to be merged, although I'd lean strongly toward
> > > facilitating this by temporarily increasing the timeout on the job to
> > allow
> > > it to actually pass rather than a manual override of the StressNewTest.
> > >
> > > The fact that it's passed over 7000 times without failing is pretty
> > strong
> > > evidence that it's not a flaky test, which is what StressNewTest is
> > > supposed to catch, so there doesn't seem to be any risk associated with
> > > circumventing it in this case, but if there's a feasible solution that
> > > doesn't involve "cheating" or ignoring the test job, then that would be
> > > preferable.
> > >
> > > - Donal
> > >
> > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh  wrote:
> > >
> > >> Greetings,
> > >>
> > >> We have a pull request (https://github.com/apache/geode/pull/4250)
> > that is
> > >> running into a problem with stressNewTest.  Mostly the tests that are
> > being
> > >> run are RollingUpgrade tests that take quite a bit of time to run the
> > full
> > >> suite.  Because these tests are added/modified, the stressNewTest
> > doesn't
> > >> have enough time to complete the run because it runs them N(50) number
> > of
> > >> times.
> > >>
> > >> However what has completed is 7400 tests and none of them have failed:
> > >>
> > >>
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > >>
> > >> We would like to get this fix in before branching the next release,
> but
> > are
> > >> unable to due to stressNewTest gating the merge button.  I know we
> have
> > >> another thread about overrides etc, and maybe this is a data point,
> but
> > >> this isn't meant to discuss that.
> > >>
> > >> Would everyone be able to agree to allow someone to manually override
> > and
> > >> merge this commit in (title of PR and reviews pending)?
> > >>
> >
> >
>


Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Xiaojian Zhou
I'm curious to see the new stressNew test result too.

On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols  wrote:

> I’ve retriggered StressNew <
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758>
> with a temporarily-increased timeout of 12 hours so we can see how long it
> would actually take, to have some data point whether to propose a permanent
> timeout increase or whether breaking up into multiple PRs is should be the
> standard way to get around this.
>
> > On Oct 31, 2019, at 2:52 PM, Donal Evans  wrote:
> >
> > +1 to allowing this PR to be merged, although I'd lean strongly toward
> > facilitating this by temporarily increasing the timeout on the job to
> allow
> > it to actually pass rather than a manual override of the StressNewTest.
> >
> > The fact that it's passed over 7000 times without failing is pretty
> strong
> > evidence that it's not a flaky test, which is what StressNewTest is
> > supposed to catch, so there doesn't seem to be any risk associated with
> > circumventing it in this case, but if there's a feasible solution that
> > doesn't involve "cheating" or ignoring the test job, then that would be
> > preferable.
> >
> > - Donal
> >
> > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh  wrote:
> >
> >> Greetings,
> >>
> >> We have a pull request (https://github.com/apache/geode/pull/4250)
> that is
> >> running into a problem with stressNewTest.  Mostly the tests that are
> being
> >> run are RollingUpgrade tests that take quite a bit of time to run the
> full
> >> suite.  Because these tests are added/modified, the stressNewTest
> doesn't
> >> have enough time to complete the run because it runs them N(50) number
> of
> >> times.
> >>
> >> However what has completed is 7400 tests and none of them have failed:
> >>
> >>
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> >>
> >> We would like to get this fix in before branching the next release, but
> are
> >> unable to due to stressNewTest gating the merge button.  I know we have
> >> another thread about overrides etc, and maybe this is a data point, but
> >> this isn't meant to discuss that.
> >>
> >> Would everyone be able to agree to allow someone to manually override
> and
> >> merge this commit in (title of PR and reviews pending)?
> >>
>
>


Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Owen Nichols
I’ve retriggered StressNew 

 with a temporarily-increased timeout of 12 hours so we can see how long it 
would actually take, to have some data point whether to propose a permanent 
timeout increase or whether breaking up into multiple PRs is should be the 
standard way to get around this.

> On Oct 31, 2019, at 2:52 PM, Donal Evans  wrote:
> 
> +1 to allowing this PR to be merged, although I'd lean strongly toward
> facilitating this by temporarily increasing the timeout on the job to allow
> it to actually pass rather than a manual override of the StressNewTest.
> 
> The fact that it's passed over 7000 times without failing is pretty strong
> evidence that it's not a flaky test, which is what StressNewTest is
> supposed to catch, so there doesn't seem to be any risk associated with
> circumventing it in this case, but if there's a feasible solution that
> doesn't involve "cheating" or ignoring the test job, then that would be
> preferable.
> 
> - Donal
> 
> On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh  wrote:
> 
>> Greetings,
>> 
>> We have a pull request (https://github.com/apache/geode/pull/4250) that is
>> running into a problem with stressNewTest.  Mostly the tests that are being
>> run are RollingUpgrade tests that take quite a bit of time to run the full
>> suite.  Because these tests are added/modified, the stressNewTest doesn't
>> have enough time to complete the run because it runs them N(50) number of
>> times.
>> 
>> However what has completed is 7400 tests and none of them have failed:
>> 
>> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
>> 
>> We would like to get this fix in before branching the next release, but are
>> unable to due to stressNewTest gating the merge button.  I know we have
>> another thread about overrides etc, and maybe this is a data point, but
>> this isn't meant to discuss that.
>> 
>> Would everyone be able to agree to allow someone to manually override and
>> merge this commit in (title of PR and reviews pending)?
>> 



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

2019-10-31 Thread Nabarun Nag
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  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  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  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  wrote:
> > >>
> > >>> -1 for allowing overrides.
> > >>> If there's an edge case on which it's required, then we could use it
> > at the
> > >>> very last resources *if and only if* it's been discussed and approved
> > >>> through the dev list for that particular case.
> > >>> Cheers.
> > >>>
> > >>>
> > >>> On Wed, Oct 30, 2019 at 11:35 PM Robert Houghton <
> rhough...@pivotal.io
> > >
> > >>> wrote:
> > >>>
> >  Any committer has the 'status' permission on apache/geode.git. Some
> > API
> >  tokens have it as well. Its curl + github API reasoning to do this.
> > 
> >  On Wed, Oct 30, 2019 at 2:02 PM Jason Huynh 
> > 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 
> >  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 
> 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 

Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Nabarun Nag
Will breaking this PR into parts help. Put the tests in separate PRs?

+1 if it is inconvenient

On Thu, Oct 31, 2019 at 2:52 PM Donal Evans  wrote:

> +1 to allowing this PR to be merged, although I'd lean strongly toward
> facilitating this by temporarily increasing the timeout on the job to allow
> it to actually pass rather than a manual override of the StressNewTest.
>
> The fact that it's passed over 7000 times without failing is pretty strong
> evidence that it's not a flaky test, which is what StressNewTest is
> supposed to catch, so there doesn't seem to be any risk associated with
> circumventing it in this case, but if there's a feasible solution that
> doesn't involve "cheating" or ignoring the test job, then that would be
> preferable.
>
> - Donal
>
> On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh  wrote:
>
> > Greetings,
> >
> > We have a pull request (https://github.com/apache/geode/pull/4250) that
> is
> > running into a problem with stressNewTest.  Mostly the tests that are
> being
> > run are RollingUpgrade tests that take quite a bit of time to run the
> full
> > suite.  Because these tests are added/modified, the stressNewTest doesn't
> > have enough time to complete the run because it runs them N(50) number of
> > times.
> >
> > However what has completed is 7400 tests and none of them have failed:
> >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> >
> > We would like to get this fix in before branching the next release, but
> are
> > unable to due to stressNewTest gating the merge button.  I know we have
> > another thread about overrides etc, and maybe this is a data point, but
> > this isn't meant to discuss that.
> >
> > Would everyone be able to agree to allow someone to manually override and
> > merge this commit in (title of PR and reviews pending)?
> >
>


Re: [vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Donal Evans
+1 to allowing this PR to be merged, although I'd lean strongly toward
facilitating this by temporarily increasing the timeout on the job to allow
it to actually pass rather than a manual override of the StressNewTest.

The fact that it's passed over 7000 times without failing is pretty strong
evidence that it's not a flaky test, which is what StressNewTest is
supposed to catch, so there doesn't seem to be any risk associated with
circumventing it in this case, but if there's a feasible solution that
doesn't involve "cheating" or ignoring the test job, then that would be
preferable.

- Donal

On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh  wrote:

> Greetings,
>
> We have a pull request (https://github.com/apache/geode/pull/4250) that is
> running into a problem with stressNewTest.  Mostly the tests that are being
> run are RollingUpgrade tests that take quite a bit of time to run the full
> suite.  Because these tests are added/modified, the stressNewTest doesn't
> have enough time to complete the run because it runs them N(50) number of
> times.
>
> However what has completed is 7400 tests and none of them have failed:
>
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
>
> We would like to get this fix in before branching the next release, but are
> unable to due to stressNewTest gating the merge button.  I know we have
> another thread about overrides etc, and maybe this is a data point, but
> this isn't meant to discuss that.
>
> Would everyone be able to agree to allow someone to manually override and
> merge this commit in (title of PR and reviews pending)?
>


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

2019-10-31 Thread Jinmei Liao
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  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  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  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  >
> >>> 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 
> 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 
>  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  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 
> > 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

[vote/discuss]Override stressNewTest for Pull Request #4250?

2019-10-31 Thread Jason Huynh
Greetings,

We have a pull request (https://github.com/apache/geode/pull/4250) that is
running into a problem with stressNewTest.  Mostly the tests that are being
run are RollingUpgrade tests that take quite a bit of time to run the full
suite.  Because these tests are added/modified, the stressNewTest doesn't
have enough time to complete the run because it runs them N(50) number of
times.

However what has completed is 7400 tests and none of them have failed:
http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/

We would like to get this fix in before branching the next release, but are
unable to due to stressNewTest gating the merge button.  I know we have
another thread about overrides etc, and maybe this is a data point, but
this isn't meant to discuss that.

Would everyone be able to agree to allow someone to manually override and
merge this commit in (title of PR and reviews pending)?


Re: Jira permission to assign myself

2019-10-31 Thread Szu-Yu Lo
Thank you so much

On Thu, Oct 31, 2019 at 2:19 PM Dan Smith  wrote:

> Hi Szu-Yu Lo,
>
> You should have access now. Thanks for being interested in contributing to
> Geode! If you haven't already, you might want to check out our How to
> Contribute
> 
> page.
>
> Thanks again!
> -Dan
>
> On Thu, Oct 31, 2019 at 11:13 AM Szu-Yu Lo  wrote:
>
> > Hi Dan,
> >
> > My username is sylo.
> >
> > Thanks
> >
> > On Thu, Oct 31, 2019 at 2:12 PM Dan Smith  wrote:
> >
> > > Hi Szu-Yu Lo,
> > >
> > > What is your JIRA username? If you don't already have an account, you
> can
> > > create one.
> > >
> > > -Dan
> > >
> > > On Thu, Oct 31, 2019 at 10:09 AM Szu-Yu Lo 
> > wrote:
> > >
> > > > Hi,
> > > >
> > > > I am new to geode and would love to contributed  to geode. I am not
> > able
> > > to
> > > > assigned myself. It would be greatly appreciated, if I could have
> > > > permission to assign an issue to myself.
> > > >
> > > >
> > > > Regards,
> > > > Szu-Yu Lo
> > > >
> > >
> >
>


Re: [DISCUSS] Tweak to branch protection rules

2019-10-31 Thread Dan Smith
Seems like the consensus is to make this change. I'll follow up with infra.

-Dan

On Wed, Oct 30, 2019 at 11:06 AM Jason Huynh  wrote:

> +1, thanks Dan!
>
> On Wed, Oct 30, 2019 at 10:07 AM Aaron Lindsey 
> wrote:
>
> > +1
> >
> > - Aaron
> >
> >
> > On Wed, Oct 30, 2019 at 8:02 AM Ju@N  wrote:
> >
> > > Perfect Naba, thanks for answering this.
> > > My vote is +1 then.
> > >
> > > On Wed, Oct 30, 2019 at 2:37 PM Nabarun Nag  wrote:
> > >
> > > > The check box Dan is mentioning will just not invalidate any approved
> > > > review if the code is changed.
> > > > If a change is requested, the button will remain disabled.
> > > >
> > > > Regards
> > > > Naba
> > > >
> > > >
> > > > On Wed, Oct 30, 2019 at 6:27 AM Joris Melchior  >
> > > > wrote:
> > > >
> > > > > +1
> > > > >
> > > > > On Wed, Oct 30, 2019 at 5:27 AM Ju@N  wrote:
> > > > >
> > > > > > Question: this only applies for *approvals*, not for *refusals*,
> > > > right?;
> > > > > I
> > > > > > mean, the *merge pull request* button will remain blocked if
> there
> > > were
> > > > > > some changes requested by reviewers and the author of the PR adds
> > new
> > > > > > commits (either addressing those requested changes or not)?.
> > > > > > If the answer to the above is "yes", then +1.
> > > > > >
> > > > > > On Wed, Oct 30, 2019 at 1:44 AM Nabarun Nag 
> > wrote:
> > > > > >
> > > > > > > +1
> > > > > > >
> > > > > > > On Tue, Oct 29, 2019 at 6:21 PM Darrel Schneider <
> > > > > dschnei...@pivotal.io>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > +1
> > > > > > > >
> > > > > > > > On Tue, Oct 29, 2019 at 6:08 PM Owen Nichols <
> > > onich...@pivotal.io>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +1 …this has already bitten me a few times
> > > > > > > > >
> > > > > > > > > > On Oct 29, 2019, at 6:01 PM, Dan Smith <
> dsm...@pivotal.io>
> > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi all,
> > > > > > > > > >
> > > > > > > > > > It seems we've configured our branch protection rules
> such
> > > that
> > > > > > > > pushing a
> > > > > > > > > > change to a PR that has been approved invalidates the
> > > previous
> > > > > > > > approval.
> > > > > > > > > >
> > > > > > > > > > I think we should turn this off - it looks like it's an
> > > > optional
> > > > > > > > feature.
> > > > > > > > > > We should trust people to rerequest reviews if needed.
> > Right
> > > > now
> > > > > > this
> > > > > > > > is
> > > > > > > > > > adding busywork for people to reapprove minor changes
> > (Fixing
> > > > > merge
> > > > > > > > > > conflicts, spotless, etc.)
> > > > > > > > > >
> > > > > > > > > > If you all agree I'll ask infra to uncheck "Dismiss stale
> > > pull
> > > > > > > request
> > > > > > > > > > approvals when new commits are pushed." in our branch
> > > > protection
> > > > > > > rules.
> > > > > > > > > >
> > > > > > > > > > -Dan
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Ju@N
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > *Joris Melchior *
> > > > > CF Engineering
> > > > > Pivotal Toronto
> > > > > 416 877 5427
> > > > >
> > > > > “Programs must be written for people to read, and only incidentally
> > for
> > > > > machines to execute.” – *Hal Abelson*
> > > > > 
> > > > >
> > > >
> > >
> > >
> > > --
> > > Ju@N
> > >
> >
>


Re: Jira permission to assign myself

2019-10-31 Thread Dan Smith
Hi Szu-Yu Lo,

You should have access now. Thanks for being interested in contributing to
Geode! If you haven't already, you might want to check out our How to
Contribute
 page.

Thanks again!
-Dan

On Thu, Oct 31, 2019 at 11:13 AM Szu-Yu Lo  wrote:

> Hi Dan,
>
> My username is sylo.
>
> Thanks
>
> On Thu, Oct 31, 2019 at 2:12 PM Dan Smith  wrote:
>
> > Hi Szu-Yu Lo,
> >
> > What is your JIRA username? If you don't already have an account, you can
> > create one.
> >
> > -Dan
> >
> > On Thu, Oct 31, 2019 at 10:09 AM Szu-Yu Lo 
> wrote:
> >
> > > Hi,
> > >
> > > I am new to geode and would love to contributed  to geode. I am not
> able
> > to
> > > assigned myself. It would be greatly appreciated, if I could have
> > > permission to assign an issue to myself.
> > >
> > >
> > > Regards,
> > > Szu-Yu Lo
> > >
> >
>


Re: Jira permission to assign myself

2019-10-31 Thread Szu-Yu Lo
Hi Dan,

My username is sylo.

Thanks

On Thu, Oct 31, 2019 at 2:12 PM Dan Smith  wrote:

> Hi Szu-Yu Lo,
>
> What is your JIRA username? If you don't already have an account, you can
> create one.
>
> -Dan
>
> On Thu, Oct 31, 2019 at 10:09 AM Szu-Yu Lo  wrote:
>
> > Hi,
> >
> > I am new to geode and would love to contributed  to geode. I am not able
> to
> > assigned myself. It would be greatly appreciated, if I could have
> > permission to assign an issue to myself.
> >
> >
> > Regards,
> > Szu-Yu Lo
> >
>


Re: Jira permission to assign myself

2019-10-31 Thread Dan Smith
Hi Szu-Yu Lo,

What is your JIRA username? If you don't already have an account, you can
create one.

-Dan

On Thu, Oct 31, 2019 at 10:09 AM Szu-Yu Lo  wrote:

> Hi,
>
> I am new to geode and would love to contributed  to geode. I am not able to
> assigned myself. It would be greatly appreciated, if I could have
> permission to assign an issue to myself.
>
>
> Regards,
> Szu-Yu Lo
>


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

2019-10-31 Thread Udo Kohlmeyer
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  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  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 
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  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 

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

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 

wrote:

@Aaron
It's okay to wait for at least the build, and unit tests to

complete,

to

cover all the bases. [There may have been commits in between

which

may

result in failure because of the revert]  And it's not hard to

get

a

PR

approval.

-1 on overriding. If the infrastructure is down, which is the

test

framework designed to ensure that we are not checking in unwanted

changes

into Apache Geode, wait for the infrastructure to be up, get your

changes

verified, get the review from a fellow committer and then

check-in

your

changes.

I still don't understand why will anyone not wait for unit tests

and

build

to be successful.

Regards
Nabarun Nag

On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey <

alind...@pivotal.io>

wrote:


One case when it might be acceptable to overrule a PR check is

reverting

a

commit. Before the branch protection was enabled, a committer

could

revert

a commit without a PR. Now that PRs are mandatory, we have to

wait

for

the

checks to run in order to revert a com

Jira permission to assign myself

2019-10-31 Thread Szu-Yu Lo
Hi,

I am new to geode and would love to contributed  to geode. I am not able to
assigned myself. It would be greatly appreciated, if I could have
permission to assign an issue to myself.


Regards,
Szu-Yu Lo


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

2019-10-31 Thread Mark Hanson
-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  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  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 
>> 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  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 
>>> 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  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 
 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 
>>> wrote:
>>> 
 @Aaron
 It's okay to wait for at least the build, and unit tests to
>>> complete,
> to
 cover all the bases. [There may have been commits in between
>> which
 may
 result in failure because of the revert]  And it's not hard to
>> get
>>> a
 PR
 approval.
 
 -1 on overriding. If the infrastructure is down, which is the
>> test
 framework designed to ensure that we are not checking in unwanted
> changes
 into Apache Geode, wait for the infrastructure to be up, get your
> changes
 verified, get the review from a fellow committer and then
>> check-in
 your
 changes.
 
 I still don't understand why will anyone not wait for unit tests
>>> and
>>> build
 to be successful.
 
 Regards
 Nabarun Nag
 
 On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey <
>>> alind...@pivotal.io>
 wrote:
 
> One case when it migh

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

2019-10-31 Thread Benjamin Ross
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  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 
> 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  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 
> > 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  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 
> > > 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 
> > wrote:
> > > > >>
> > > > >>> @Aaron
> > > > >>> It's okay to wait for at least the build, and unit tests to
> > complete,
> > > > to
> > > > >>> cover all the bases. [There may have been commits in between
> which
> > > may
> > > > >>> result in failure because of the revert]  And it's not hard to
> get
> > a
> > > PR
> > > > >>> approval.
> > > > >>>
> > > > >>> -1 on overriding. If the infrastructure is down, which is the
> test
> > > > >>> framework designed to ensure that we are not checking in unwanted
> > > > changes
> > > > >>> into Apache Geode, wait for the infrastructure to be up, get your
> > > > changes
> > > > >>> verified, get the review from a fellow committer and then
> check-in
> > > your
> > > > >>> changes.
> > > > >>>
> > > > >>> I still don't understand why will anyone not wait for unit tests
> > and
> > > > >> build
> > > > >>> to be successful.
> > > > >>>
> > > > >>> Regards
> > > > >>> Nabarun Nag
> > > > >>>
> > > > >>> On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey <
> > alind...@pivotal.io>
> > > > >>> wrote:
> > > > >>>
> > > >  One case when it might be acceptable to overrule a PR check is
> > > > >> reverting
> > > > >>> a
> > > >  commit. Before the branch protection was enabled, a committer
> > could
> > > > >>> revert
> > > >  a commit without a PR. Now that PRs are mandatory, we h

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

2019-10-31 Thread Ju@N
-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 
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  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 
> 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  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 
> > 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 
> wrote:
> > > >>
> > > >>> @Aaron
> > > >>> It's okay to wait for at least the build, and unit tests to
> complete,
> > > to
> > > >>> cover all the bases. [There may have been commits in between which
> > may
> > > >>> result in failure because of the revert]  And it's not hard to get
> a
> > PR
> > > >>> approval.
> > > >>>
> > > >>> -1 on overriding. If the infrastructure is down, which is the test
> > > >>> framework designed to ensure that we are not checking in unwanted
> > > changes
> > > >>> into Apache Geode, wait for the infrastructure to be up, get your
> > > changes
> > > >>> verified, get the review from a fellow committer and then check-in
> > your
> > > >>> changes.
> > > >>>
> > > >>> I still don't understand why will anyone not wait for unit tests
> and
> > > >> build
> > > >>> to be successful.
> > > >>>
> > > >>> Regards
> > > >>> Nabarun Nag
> > > >>>
> > > >>> On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey <
> alind...@pivotal.io>
> > > >>> wrote:
> > > >>>
> > >  One case when it might be acceptable to overrule a PR check is
> > > >> reverting
> > > >>> a
> > >  commit. Before the branch protection was enabled, a committer
> could
> > > >>> revert
> > >  a commit without a PR. Now that PRs are mandatory, we have to wait
> > for
> > > >>> the
> > >  checks to run in order to revert a commit. Usually we are
> reverting
> > a
> > >  commit because it's causing problems, so I think overruling the PR
> > > >> checks
> > >  may be acceptable in that case.
> > > 
> > >  - Aaron
> > > 
> > > 
> > >  On Wed, Oct 30, 2019 at 11:11 AM Owen Nichols <
> onich...@pivotal.io>
> > > >>> wrote:
> > > 
> > > > Our new branch-protection rules can sometimes lead to unexpected
> > >  obstacles
> > > > when infrastructure issues impede the intended process.  Should
> we
> > >  discuss
> > > > such cases as they come up, and should overruling the result of a
> > PR
> > >  check
> > > > ever be an option on the table?
> > > >
> > > > -Owen
> > > 
> > > >>>
> > > >>
> > >
> > >
> >
>


-- 
Ju@N