Re: [DISCUSS] including Java11 in the pipelines

[Owen Nichols]

PRs created/updated after 4pm today will now get the additional Java11 checks :)

The non-gating jobs have also been shifted right in the pipeline (this is only 
visible when you click on the “complete 
”
 tab, no effect on the default view)

-Owen

> On Nov 8, 2018, at 1:19 PM, Owen Nichols  wrote:
> 
> Sounds like the overwhelming consensus is to keep Java11 gating, add Java11 
> to PR, and defer non-gating tests.
> 
> I have prepared PRs for these changes:
> https://github.com/apache/geode/pull/2806 (add Java11 to PR pipeline)
> https://github.com/apache/geode/pull/2816 (run non-gating jobs only after 
> gating jobs have passed)
> 
> Please review and merge.
> 
> Thanks,
> -Owen
> 
>> On Nov 8, 2018, at 9:41 AM, Patrick Rhomberg  wrote:
>> 
>> I agree with Jinmei on all points.  I definitely think there should be
>> parity between precheckin and the main pipeline, but that might just be
>> because I caused the main pipeline to fail on Java11 this week.
>> 
>> On Wed, Nov 7, 2018 at 1:34 PM, Jinmei Liao  wrote:
>> 
>>> First of all, I believe all gating tests should be run in precheckin. If we
>>> make jdk11 tests gating, we should make it part of the precheckin. If we
>>> don't put them in precheckin, they should not be gating.
>>> 
>>> Secondly, If we don't make jdk11 tests gating, soon they will become like
>>> windows tests, people only look at them after it's been failing for days,
>>> which is not good.
>>> 
>>> Thirdly, for non-gating tests, we probably should run them after all the
>>> gating tests are done.
>>> 
>>> On Wed, Nov 7, 2018 at 11:39 AM Owen Nichols  wrote:
>>> 
 Now that tests are passing under Java 11, it was recommended last week to
 make Java 11 tests gating for the develop pipeline.  [Fyi, Windows tests
 are not yet gating, meaning the pipeline will success and publish
>>> artifacts
 even if a Windows tests fails.]
 
 Three topics merit discussion:
 
 1) For the Geode 1.8 release, should the release notes advertise
 “experimental support for Java 11” or no support?  If the latter, should
 Java 11 tests still be gating on the 1.8 release branch, or only on
>>> develop?
 
 2) As of now, pre-checkin runs tests only against Java8.  Now that Java11
 is gating in develop, should we now be testing against both Java8 and
 Java11 as part of validating pull requests?
 
 3) In the develop pipeline, should non-gating jobs continue to be run in
 parallel with gating jobs?  Or would it be better to change the develop
 pipeline to only run the non-gating tests after all gating jobs have
>>> passed?
 
 Thanks,
 -Owen
 
 P.S. after a brief trial of combining Java8 and Java11 tests into a
>>> single
 pipeline job, that was reverted and it looks like separate jobs are here
>>> to
 stay.  Combined jobs made it difficult to look at dual scrolling outputs
 and dual archive-results steps.  If you feel there’s a better solution,
 please speak up.
 
 
>>> 
>>> --
>>> Cheers
>>> 
>>> Jinmei
>>> 
> 

Re: [DISCUSS] Disable merge for failing pull requests

[Jacob Barrett]

-1

I think there is still plenty of flakiness in the tests that PRs can go red
due to something flaky. Blocking a merge when the issue is known to be
flaky would be disruptive to progress. I think it will also just encourage
someone to use direct pushing to work around false reds. This might lead to
even worse transgressions, like merging changes without PRs.

Do we really have a problem with someone merging legitimately broken PRs?


On Fri, Nov 9, 2018 at 12:55 PM Dan Smith  wrote:

> Hi all,
>
> Kirks emails reminded me - I think we are at the point now with our PR
> checks where we should not be merging anything to develop that doesn't pass
> all of the PR checks.
>
> I propose we disable the merge button unless a PR is passing all of the
> checks. If we are in agreement I'll follow up with infra to see how to make
> that happen.
>
> This would not completely prevent pushing directly to develop from the
> command line, but since most developers seem to be using the github UI, I
> hope that it will steer people towards getting the PRs passing instead of
> using the command line.
>
> Thoughts?
> -Dan
>

RED ALERT: EvictionDUnitTest failed again DistributedTestOpenJDK8 #101

[Kirk Lund]

GEODE-5943: EvictionDUnitTest failed in jdk11

https://issues.apache.org/jira/browse/GEODE-5943

https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/DistributedTestOpenJDK8/builds/101

org.apache.geode.internal.cache.eviction.EvictionDUnitTest >
testDummyInlineNCentralizedEviction[offHeap=false] FAILED
org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.internal.cache.eviction.EvictionDUnitTest$$Lambda$59/926122170.call
in VM 1 running on Host d55391503e0b with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
at org.apache.geode.test.dunit.VM.invoke(VM.java:384)
at
org.apache.geode.test.junit.rules.VMProvider.invoke(VMProvider.java:74)
at
org.apache.geode.internal.cache.eviction.EvictionDUnitTest.testDummyInlineNCentralizedEviction(EvictionDUnitTest.java:125)

Caused by:
org.awaitility.core.ConditionTimeoutException: Condition with
lambda expression in
org.apache.geode.internal.cache.eviction.EvictionDUnitTest that uses
org.apache.geode.internal.cache.LocalRegion,
org.apache.geode.internal.cache.LocalRegionint was not fulfilled within 300
seconds.
at
org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:145)
at
org.awaitility.core.CallableCondition.await(CallableCondition.java:79)
at
org.awaitility.core.CallableCondition.await(CallableCondition.java:27)
at
org.awaitility.core.ConditionFactory.until(ConditionFactory.java:890)
at
org.awaitility.core.ConditionFactory.until(ConditionFactory.java:848)
at
org.apache.geode.internal.cache.eviction.EvictionDUnitTest.sendEventAndWaitForExpectedEviction(EvictionDUnitTest.java:249)
at
org.apache.geode.internal.cache.eviction.EvictionDUnitTest.lambda$testDummyInlineNCentralizedEviction$7d47808e$1(EvictionDUnitTest.java:125)

8210 tests completed, 1 failed, 494 skipped

> Task :geode-core:distributedTest FAILED

Re: Our use of Jira "Fix Version/s"

[Helena Bales]

+1 to Alexander's suggestion. I don't know where to put stuff like that,
but once we have one we should link to it from our new CIO instructions
page.

On Wed, Nov 7, 2018 at 2:59 PM Alexander Murmann 
wrote:

> Kirk, I think most of us are in agreement that it works as you are
> describing. The notes for the release manager also assume that we are all
> following this process.
> However, I also was unable to find something obvious in the wiki. The
> closest is the "JIRA Guidelines" page
> . That
> page is pretty implicit about it by just mentioning that you should set the
> version as part of completing a story.
>
> It might be nice to have a paragraph somewhere that explains how to file a
> JIRA ticket. Where would be a good place(s) that would actually be seen?
>
> On Wed, Nov 7, 2018 at 2:52 PM Kirk Lund  wrote:
>
> > Does anyone have a link to a wiki page describing our use of "Fix
> > Version/s" on Geode Jira tickets?
> >
> > My understanding is that when we file a ticket we leave it blank. Then
> when
> > we commit a fix to develop we change the Jira ticket to "Resolved" and
> fill
> > in the "Fix Version/s" with the upcoming release (ex: 1.8.0). Then when
> we
> > release 1.8.0, all tickets that are in "Resolved" with "Fix Version/s" of
> > 1.8.0 will be changed to "Closed" with "Fix Version/s" 1.8.0.
> >
> > Some of our community are filing tickets with "Fix Version/s" pre-filled
> > with the version that we hope to fix the ticket in.
> >
> > Anyone have a definitive answer for this process?
> >
> > Thanks,
> > Kirk
> >
>

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Jinmei Liao]

Yeah, I guess. But why going out of this way to avoid using rules? Why
rules are bad? Rules just encapsulate common befores and afters to avoid
duplicate code in every tests. I don't see why we should avoid using it.

On Fri, Nov 9, 2018, 3:44 PM Galen O'Sullivan  I wonder if we could use some sort of assertions to validate that that
> tests have cleaned up, at least in a few ways? For example, if a
> Cache/Locator/DistributedSystem is still running after a test has finished,
> that's a good sign of a dirty environment. If system properties are still
> set, that's a good sign of a dirty environment. We could use a custom test
> runner, or even add a rule to all our tests en masse that checks that
> things are cleaned up.
>
> Jinmei, for single-JVM tests, you could write a method for your test (or
> test class) that sets whatever properties you need and returns a Cache
> constructed with those properties. Then you can use try-with-resources to
> make sure that the Cache is properly closed. Is that a good alternative to
> using a rule?
>
> On Fri, Nov 9, 2018 at 3:28 PM Helena Bales  wrote:
>
> > +1 for deprecating old test bases. Many of the tests that gave me the
> most
> > trouble this summer were because of JUnit4CacheTestCase.
> > Also +1 for pulling out Blackboard into a rule.
> >
> > I will, however, argue for continuing to use ClusterStartupRule. The
> > benefit of that is that it makes sure that all JVMs started for servers
> and
> > locators are cleaned up at the end of the tests, even if the tests fail.
> We
> > could certainly spend time making that code easier to understand, but I
> > don't think that starting clusters is straightforward enough to have
> > confidence that it will get done correctly every time. Multi-threaded
> tests
> > should be a cautionary tale for this; some implementations were fine, but
> > many polluted the system with threads that never stopped and tests that
> > didn't actually test anything.
> > As I see it, we are paying in readability for tests that do things the
> > right way.
> >
> > On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund  wrote:
> >
> > > I would love to see you apply some of your passion for that to
> improving
> > > the User APIs so there's less boiler plate code for the Users as well.
> > >
> > > Please don't forget that to Developers who are not familiar with our
> > Rules
> > > such as ClusterStarterRule, that it can be very difficult to understand
> > > what it has done and how it has configured Geode. The more the Rule
> does,
> > > the greater this problem. The fact that most of these Rules use
> Internal
> > > APIs instead of User APIs is also a problem in my opinion because we're
> > not
> > > testing exactly what a User would do or can do.
> > >
> > > To many of us Developers, figuring out what all the rules have
> configured
> > > and done is a much bigger problem than it is to deal with verbose code
> > in a
> > > setUp method that uses CacheFactory directly. On one hand I want to
> say,
> > do
> > > as you prefer but we also have to consider that other Developers need
> to
> > > maintain these tests that are using the Rules, so I will continue to
> > > advocate for the writing of tests using Geode User APIs as much as
> > possible
> > > for the reasons I already stated.
> > >
> > > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao  wrote:
> > >
> > > > I like using the rules because it reduces boiler plate code and the
> > > chance
> > > > of not cleaning up properly after your tests. It also make what you
> are
> > > > really trying to do in the test stand out more in the test code.
> > > >
> > > > On Fri, Nov 9, 2018 at 1:37 PM Kirk Lund  wrote:
> > > >
> > > > > We need to pull out the DUnit Blackboard from DistributedTestCase
> and
> > > > > repackage it as a BlackboardRule. It makes sense to make that a
> JUnit
> > > > Rule
> > > > > because it's not part of Geode or its User APIs but it's really
> > useful
> > > > for
> > > > > distributed testing in general. It's also probably the last useful
> > > thing
> > > > > that's still in DistributedTestCase and no where else.
> > > > >
> > > > > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt <
> > > > bschucha...@pivotal.io>
> > > > > wrote:
> > > > >
> > > > > > I agree with Kirk about the Rules and I agree with Galen about
> > moving
> > > > > away
> > > > > > from the old abstract test classes.  I think that is also in the
> > > spirit
> > > > > of
> > > > > > what Kirk is saying.
> > > > > >
> > > > > > There are also tests that have complicated methods for creating
> > > caches
> > > > > and
> > > > > > regions.  These methods have many parameters and are sometimes in
> > > > Helper
> > > > > > classes.  I've found these especially difficult to deal with when
> > > > fixing
> > > > > > flaky tests because changing one of the Helper methods affects
> many
> > > > > tests.
> > > > > >
> > > > > >
> > > > > > On 11/9/18 11:31 AM, Kirk Lund wrote:
> > > > > >
> > > > > >> *I would like to encourage 

Re: [DISCUSS] Disable merge for failing pull requests

[Kenneth Howe]

+1

> On Nov 9, 2018, at 2:07 PM, Helena Bales  wrote:
> 
> +1
> As far as the failure rate goes, even if the rate is 25% restarting the
> pipeline usually gives a passing run. And this would make us less likely to
> incorrectly dismiss a failure as not related. If the second build also
> fails with the same failures, those should be investigated and resolved in
> some way before the PR is merged.
> 
> On Fri, Nov 9, 2018 at 1:49 PM Anthony Baker  wrote:
> 
>> It looks like the current failure rate (post-PR, including all types of
>> failures) for DistributedTest is around 25%.  Do most people experience
>> similar failure rates on the *-pr pipeline?  I’m specifically wondering
>> about failures unrelated to your changes.
>> 
>> Anthony
>> 
>> 
>>> On Nov 9, 2018, at 12:55 PM, Dan Smith  wrote:
>>> 
>>> Hi all,
>>> 
>>> Kirks emails reminded me - I think we are at the point now with our PR
>>> checks where we should not be merging anything to develop that doesn't
>> pass
>>> all of the PR checks.
>>> 
>>> I propose we disable the merge button unless a PR is passing all of the
>>> checks. If we are in agreement I'll follow up with infra to see how to
>> make
>>> that happen.
>>> 
>>> This would not completely prevent pushing directly to develop from the
>>> command line, but since most developers seem to be using the github UI, I
>>> hope that it will steer people towards getting the PRs passing instead of
>>> using the command line.
>>> 
>>> Thoughts?
>>> -Dan
>> 
>> 

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Galen O'Sullivan]

I wonder if we could use some sort of assertions to validate that that
tests have cleaned up, at least in a few ways? For example, if a
Cache/Locator/DistributedSystem is still running after a test has finished,
that's a good sign of a dirty environment. If system properties are still
set, that's a good sign of a dirty environment. We could use a custom test
runner, or even add a rule to all our tests en masse that checks that
things are cleaned up.

Jinmei, for single-JVM tests, you could write a method for your test (or
test class) that sets whatever properties you need and returns a Cache
constructed with those properties. Then you can use try-with-resources to
make sure that the Cache is properly closed. Is that a good alternative to
using a rule?

On Fri, Nov 9, 2018 at 3:28 PM Helena Bales  wrote:

> +1 for deprecating old test bases. Many of the tests that gave me the most
> trouble this summer were because of JUnit4CacheTestCase.
> Also +1 for pulling out Blackboard into a rule.
>
> I will, however, argue for continuing to use ClusterStartupRule. The
> benefit of that is that it makes sure that all JVMs started for servers and
> locators are cleaned up at the end of the tests, even if the tests fail. We
> could certainly spend time making that code easier to understand, but I
> don't think that starting clusters is straightforward enough to have
> confidence that it will get done correctly every time. Multi-threaded tests
> should be a cautionary tale for this; some implementations were fine, but
> many polluted the system with threads that never stopped and tests that
> didn't actually test anything.
> As I see it, we are paying in readability for tests that do things the
> right way.
>
> On Fri, Nov 9, 2018 at 2:31 PM Kirk Lund  wrote:
>
> > I would love to see you apply some of your passion for that to improving
> > the User APIs so there's less boiler plate code for the Users as well.
> >
> > Please don't forget that to Developers who are not familiar with our
> Rules
> > such as ClusterStarterRule, that it can be very difficult to understand
> > what it has done and how it has configured Geode. The more the Rule does,
> > the greater this problem. The fact that most of these Rules use Internal
> > APIs instead of User APIs is also a problem in my opinion because we're
> not
> > testing exactly what a User would do or can do.
> >
> > To many of us Developers, figuring out what all the rules have configured
> > and done is a much bigger problem than it is to deal with verbose code
> in a
> > setUp method that uses CacheFactory directly. On one hand I want to say,
> do
> > as you prefer but we also have to consider that other Developers need to
> > maintain these tests that are using the Rules, so I will continue to
> > advocate for the writing of tests using Geode User APIs as much as
> possible
> > for the reasons I already stated.
> >
> > On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao  wrote:
> >
> > > I like using the rules because it reduces boiler plate code and the
> > chance
> > > of not cleaning up properly after your tests. It also make what you are
> > > really trying to do in the test stand out more in the test code.
> > >
> > > On Fri, Nov 9, 2018 at 1:37 PM Kirk Lund  wrote:
> > >
> > > > We need to pull out the DUnit Blackboard from DistributedTestCase and
> > > > repackage it as a BlackboardRule. It makes sense to make that a JUnit
> > > Rule
> > > > because it's not part of Geode or its User APIs but it's really
> useful
> > > for
> > > > distributed testing in general. It's also probably the last useful
> > thing
> > > > that's still in DistributedTestCase and no where else.
> > > >
> > > > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt <
> > > bschucha...@pivotal.io>
> > > > wrote:
> > > >
> > > > > I agree with Kirk about the Rules and I agree with Galen about
> moving
> > > > away
> > > > > from the old abstract test classes.  I think that is also in the
> > spirit
> > > > of
> > > > > what Kirk is saying.
> > > > >
> > > > > There are also tests that have complicated methods for creating
> > caches
> > > > and
> > > > > regions.  These methods have many parameters and are sometimes in
> > > Helper
> > > > > classes.  I've found these especially difficult to deal with when
> > > fixing
> > > > > flaky tests because changing one of the Helper methods affects many
> > > > tests.
> > > > >
> > > > >
> > > > > On 11/9/18 11:31 AM, Kirk Lund wrote:
> > > > >
> > > > >> *I would like to encourage all Geode developers to start writing
> > tests
> > > > >> directly against the Geode User APIs* even in DistributedTests.
> I'm
> > no
> > > > >> longer using *CacheRule, ClientCacheRule, ServerStarterRule,
> > > > >> LocatorStarterRule, or ClusterStarterRule* and I'm against
> > encouraging
> > > > >>
> > > > >> their use any longer. I'll explain why below.
> > > > >>
> > > > >> Here's an example for an IntegrationTest that needs a Cache but
> not
> > > any
> > > > >> CacheServers:
> > 

Re: [DISCUSS] LGTM on pull requests

[Jacob Barrett]

I opened a ticket with infra earlier this week to enable PR integration. There 
hasn’t been any movement. 

> On Nov 9, 2018, at 3:00 PM, Nabarun Nag  wrote:
> 
> As per running periodically , LGTM runs it every Monday.
> 
> As for who would fix it, LGTM mentions which commit caused the failure and 
> who was the author of it. So i think its the author's responsibility to fix 
> it.
> 
> Personally, LGTM list a table that shows how many alerts we caused by which 
> author [ https://lgtm.com/projects/g/apache/geode/contributors:java 
>  ]
> I cleaning up whatever alerts I have introduced into Apache Geode.
> 
> Regards
> Nabarun
> 
> 
>> On Nov 9, 2018, at 2:54 PM, Alexander Murmann  wrote:
>> 
>> I don't have strong opinions on this, but I am always suspect of CI jobs
>> that indicate quality that only run periodically. If the job discovers
>> something that needs improvement who is going to do the work and when?
>> 
>>> On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:
>>> 
>>> Well, we could run it periodically such as weekly rather than as part of
>>> the main pipeline or precheckin.
>>> 
>>> On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
>>> wrote:
>>> 
 +1, although I do wonder about the overhead of making PRs increasing more
 than it already feels like to me as a new contributor (as the person who
 made the geospatial contribution). If this was a gradle task maybe like
 spotless?
 
 On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
 wrote:
 
> I'd like to see LGTM run on pull requests.  Otherwise I think we're
> fighting a losing battle trying to improve the quality of our code. For
> instance, we just had a nice contribution of geospatial functionality
> that raised 5 alerts, but we only found out about it after the code was
> merged to develop.
> 
> LGTM allows that kind of integration but you have to be the repo
>>> "owner"
> to set it up.
> 
> 
> https://lgtm.com/projects/g/apache/geode/
> 
> 
> 
 
>>> 
> 

Re: [DISCUSS] LGTM on pull requests

[Nabarun Nag]

As per running periodically , LGTM runs it every Monday.

As for who would fix it, LGTM mentions which commit caused the failure and who 
was the author of it. So i think its the author's responsibility to fix it.

Personally, LGTM list a table that shows how many alerts we caused by which 
author [ https://lgtm.com/projects/g/apache/geode/contributors:java 
 ]
I cleaning up whatever alerts I have introduced into Apache Geode.

Regards
Nabarun


> On Nov 9, 2018, at 2:54 PM, Alexander Murmann  wrote:
> 
> I don't have strong opinions on this, but I am always suspect of CI jobs
> that indicate quality that only run periodically. If the job discovers
> something that needs improvement who is going to do the work and when?
> 
> On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:
> 
>> Well, we could run it periodically such as weekly rather than as part of
>> the main pipeline or precheckin.
>> 
>> On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
>> wrote:
>> 
>>> +1, although I do wonder about the overhead of making PRs increasing more
>>> than it already feels like to me as a new contributor (as the person who
>>> made the geospatial contribution). If this was a gradle task maybe like
>>> spotless?
>>> 
>>> On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
>>> wrote:
>>> 
 I'd like to see LGTM run on pull requests.  Otherwise I think we're
 fighting a losing battle trying to improve the quality of our code. For
 instance, we just had a nice contribution of geospatial functionality
 that raised 5 alerts, but we only found out about it after the code was
 merged to develop.
 
 LGTM allows that kind of integration but you have to be the repo
>> "owner"
 to set it up.
 
 
 https://lgtm.com/projects/g/apache/geode/
 
 
 
>>> 
>> 

Re: [DISCUSS] LGTM on pull requests

[Alexander Murmann]

I don't have strong opinions on this, but I am always suspect of CI jobs
that indicate quality that only run periodically. If the job discovers
something that needs improvement who is going to do the work and when?

On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:

> Well, we could run it periodically such as weekly rather than as part of
> the main pipeline or precheckin.
>
> On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
> wrote:
>
> > +1, although I do wonder about the overhead of making PRs increasing more
> > than it already feels like to me as a new contributor (as the person who
> > made the geospatial contribution). If this was a gradle task maybe like
> > spotless?
> >
> > On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
> > wrote:
> >
> > > I'd like to see LGTM run on pull requests.  Otherwise I think we're
> > > fighting a losing battle trying to improve the quality of our code. For
> > > instance, we just had a nice contribution of geospatial functionality
> > > that raised 5 alerts, but we only found out about it after the code was
> > > merged to develop.
> > >
> > > LGTM allows that kind of integration but you have to be the repo
> "owner"
> > > to set it up.
> > >
> > >
> > > https://lgtm.com/projects/g/apache/geode/
> > >
> > >
> > >
> >
>

Re: [DISCUSS] LGTM on pull requests

[Kirk Lund]

Well, we could run it periodically such as weekly rather than as part of
the main pipeline or precheckin.

On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri  wrote:

> +1, although I do wonder about the overhead of making PRs increasing more
> than it already feels like to me as a new contributor (as the person who
> made the geospatial contribution). If this was a gradle task maybe like
> spotless?
>
> On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
> wrote:
>
> > I'd like to see LGTM run on pull requests.  Otherwise I think we're
> > fighting a losing battle trying to improve the quality of our code. For
> > instance, we just had a nice contribution of geospatial functionality
> > that raised 5 alerts, but we only found out about it after the code was
> > merged to develop.
> >
> > LGTM allows that kind of integration but you have to be the repo "owner"
> > to set it up.
> >
> >
> > https://lgtm.com/projects/g/apache/geode/
> >
> >
> >
>

Re: [DISCUSS] LGTM on pull requests

[Aditya Anchuri]

+1, although I do wonder about the overhead of making PRs increasing more
than it already feels like to me as a new contributor (as the person who
made the geospatial contribution). If this was a gradle task maybe like
spotless?

On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
wrote:

> I'd like to see LGTM run on pull requests.  Otherwise I think we're
> fighting a losing battle trying to improve the quality of our code. For
> instance, we just had a nice contribution of geospatial functionality
> that raised 5 alerts, but we only found out about it after the code was
> merged to develop.
>
> LGTM allows that kind of integration but you have to be the repo "owner"
> to set it up.
>
>
> https://lgtm.com/projects/g/apache/geode/
>
>
>

Re: [DISCUSS] LGTM on pull requests

[Kirk Lund]

FindBugs is another one that could potentially help some. I get tired of
adding reviews that say "Please change this member variable to be private"
-- tools like LGTM and FindBugs can help guide us to better code and
prevent us from losing ground in something like the improvements you all
have made for LGTM. Thanks by the way!

On Fri, Nov 9, 2018 at 2:20 PM, Bruce Schuchardt 
wrote:

> I'd like to see LGTM run on pull requests.  Otherwise I think we're
> fighting a losing battle trying to improve the quality of our code. For
> instance, we just had a nice contribution of geospatial functionality that
> raised 5 alerts, but we only found out about it after the code was merged
> to develop.
>
> LGTM allows that kind of integration but you have to be the repo "owner"
> to set it up.
>
>
> https://lgtm.com/projects/g/apache/geode/
>
>
>

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Kirk Lund]

I would love to see you apply some of your passion for that to improving
the User APIs so there's less boiler plate code for the Users as well.

Please don't forget that to Developers who are not familiar with our Rules
such as ClusterStarterRule, that it can be very difficult to understand
what it has done and how it has configured Geode. The more the Rule does,
the greater this problem. The fact that most of these Rules use Internal
APIs instead of User APIs is also a problem in my opinion because we're not
testing exactly what a User would do or can do.

To many of us Developers, figuring out what all the rules have configured
and done is a much bigger problem than it is to deal with verbose code in a
setUp method that uses CacheFactory directly. On one hand I want to say, do
as you prefer but we also have to consider that other Developers need to
maintain these tests that are using the Rules, so I will continue to
advocate for the writing of tests using Geode User APIs as much as possible
for the reasons I already stated.

On Fri, Nov 9, 2018 at 1:44 PM, Jinmei Liao  wrote:

> I like using the rules because it reduces boiler plate code and the chance
> of not cleaning up properly after your tests. It also make what you are
> really trying to do in the test stand out more in the test code.
>
> On Fri, Nov 9, 2018 at 1:37 PM Kirk Lund  wrote:
>
> > We need to pull out the DUnit Blackboard from DistributedTestCase and
> > repackage it as a BlackboardRule. It makes sense to make that a JUnit
> Rule
> > because it's not part of Geode or its User APIs but it's really useful
> for
> > distributed testing in general. It's also probably the last useful thing
> > that's still in DistributedTestCase and no where else.
> >
> > On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt <
> bschucha...@pivotal.io>
> > wrote:
> >
> > > I agree with Kirk about the Rules and I agree with Galen about moving
> > away
> > > from the old abstract test classes.  I think that is also in the spirit
> > of
> > > what Kirk is saying.
> > >
> > > There are also tests that have complicated methods for creating caches
> > and
> > > regions.  These methods have many parameters and are sometimes in
> Helper
> > > classes.  I've found these especially difficult to deal with when
> fixing
> > > flaky tests because changing one of the Helper methods affects many
> > tests.
> > >
> > >
> > > On 11/9/18 11:31 AM, Kirk Lund wrote:
> > >
> > >> *I would like to encourage all Geode developers to start writing tests
> > >> directly against the Geode User APIs* even in DistributedTests. I'm no
> > >> longer using *CacheRule, ClientCacheRule, ServerStarterRule,
> > >> LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
> > >>
> > >> their use any longer. I'll explain why below.
> > >>
> > >> Here's an example for an IntegrationTest that needs a Cache but not
> any
> > >> CacheServers:
> > >>
> > >> private Cache cache;
> > >>
> > >> @Before
> > >> public void setUp() {
> > >>Properties config = new Properties();
> > >>config.setProperty(LOCATORS, "");
> > >>cache = new CacheFactory(config).create();
> > >> }
> > >>
> > >> @After
> > >> public void tearDown() {
> > >>cache.close();
> > >> }
> > >>
> > >> That's some pretty simple code and as a Developer, I can tell exactly
> > what
> > >> it's doing and what the config is.
> > >>
> > >> Here's an example of the kind of Geode User API code that I use to
> > create
> > >> Servers in a DistributedTests now:
> > >>
> > >>private void createServer(String serverName, File serverDir, int
> > >> locatorPort) {
> > >>  ServerLauncher.Builder builder = new ServerLauncher.Builder();
> > >>  builder.setMemberName(serverName);
> > >>  builder.setWorkingDirectory(serverDir.getAbsolutePath());
> > >>  builder.setServerPort(0);
> > >>  builder.set(LOCATORS, "localHost[" + locatorPort + "]");
> > >>  builder.set(DISABLE_AUTO_RECONNECT, "false");
> > >>  builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
> > >>  builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
> > >>  builder.set(MEMBER_TIMEOUT, "2000");
> > >>
> > >>  serverLauncher = builder.build();
> > >>  serverLauncher.start();
> > >>  assertThat(serverLauncher.isRunning()).isTrue();
> > >>}
> > >>
> > >> In particular, I think we should be using ServerLauncher and
> > >> LocatorLauncher instead of Rules when we want a full-stack Locator or
> > >> full-stack Server that looks like what a User is going to startup.
> > >>
> > >> Here are my reasons:
> > >>
> > >> 1) I want to learn and use the Geode User APIs directly, not someone's
> > >> (even mine) Testing API that hides the Geode User APIs. If I see a
> test
> > >> fail, I want to see exactly what was configured and what User APIs
> were
> > >> used right there in the test without having to open other classes. I
> > don't
> > >> want to have to spend even 15 minutes digging through some JUnit Rule
> to
> > >> figure out how 

[DISCUSS] LGTM on pull requests

[Bruce Schuchardt]

I'd like to see LGTM run on pull requests.  Otherwise I think we're 
fighting a losing battle trying to improve the quality of our code. For 
instance, we just had a nice contribution of geospatial functionality 
that raised 5 alerts, but we only found out about it after the code was 
merged to develop.


LGTM allows that kind of integration but you have to be the repo "owner" 
to set it up.



https://lgtm.com/projects/g/apache/geode/

Re: [DISCUSS] Disable merge for failing pull requests

[Helena Bales]

+1
As far as the failure rate goes, even if the rate is 25% restarting the
pipeline usually gives a passing run. And this would make us less likely to
incorrectly dismiss a failure as not related. If the second build also
fails with the same failures, those should be investigated and resolved in
some way before the PR is merged.

On Fri, Nov 9, 2018 at 1:49 PM Anthony Baker  wrote:

> It looks like the current failure rate (post-PR, including all types of
> failures) for DistributedTest is around 25%.  Do most people experience
> similar failure rates on the *-pr pipeline?  I’m specifically wondering
> about failures unrelated to your changes.
>
> Anthony
>
>
> > On Nov 9, 2018, at 12:55 PM, Dan Smith  wrote:
> >
> > Hi all,
> >
> > Kirks emails reminded me - I think we are at the point now with our PR
> > checks where we should not be merging anything to develop that doesn't
> pass
> > all of the PR checks.
> >
> > I propose we disable the merge button unless a PR is passing all of the
> > checks. If we are in agreement I'll follow up with infra to see how to
> make
> > that happen.
> >
> > This would not completely prevent pushing directly to develop from the
> > command line, but since most developers seem to be using the github UI, I
> > hope that it will steer people towards getting the PRs passing instead of
> > using the command line.
> >
> > Thoughts?
> > -Dan
>
>

Re: [DISCUSS] Disable merge for failing pull requests

[Anthony Baker]

It looks like the current failure rate (post-PR, including all types of 
failures) for DistributedTest is around 25%.  Do most people experience similar 
failure rates on the *-pr pipeline?  I’m specifically wondering about failures 
unrelated to your changes.

Anthony


> On Nov 9, 2018, at 12:55 PM, Dan Smith  wrote:
> 
> Hi all,
> 
> Kirks emails reminded me - I think we are at the point now with our PR
> checks where we should not be merging anything to develop that doesn't pass
> all of the PR checks.
> 
> I propose we disable the merge button unless a PR is passing all of the
> checks. If we are in agreement I'll follow up with infra to see how to make
> that happen.
> 
> This would not completely prevent pushing directly to develop from the
> command line, but since most developers seem to be using the github UI, I
> hope that it will steer people towards getting the PRs passing instead of
> using the command line.
> 
> Thoughts?
> -Dan

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Jinmei Liao]

I like using the rules because it reduces boiler plate code and the chance
of not cleaning up properly after your tests. It also make what you are
really trying to do in the test stand out more in the test code.

On Fri, Nov 9, 2018 at 1:37 PM Kirk Lund  wrote:

> We need to pull out the DUnit Blackboard from DistributedTestCase and
> repackage it as a BlackboardRule. It makes sense to make that a JUnit Rule
> because it's not part of Geode or its User APIs but it's really useful for
> distributed testing in general. It's also probably the last useful thing
> that's still in DistributedTestCase and no where else.
>
> On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt 
> wrote:
>
> > I agree with Kirk about the Rules and I agree with Galen about moving
> away
> > from the old abstract test classes.  I think that is also in the spirit
> of
> > what Kirk is saying.
> >
> > There are also tests that have complicated methods for creating caches
> and
> > regions.  These methods have many parameters and are sometimes in Helper
> > classes.  I've found these especially difficult to deal with when fixing
> > flaky tests because changing one of the Helper methods affects many
> tests.
> >
> >
> > On 11/9/18 11:31 AM, Kirk Lund wrote:
> >
> >> *I would like to encourage all Geode developers to start writing tests
> >> directly against the Geode User APIs* even in DistributedTests. I'm no
> >> longer using *CacheRule, ClientCacheRule, ServerStarterRule,
> >> LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
> >>
> >> their use any longer. I'll explain why below.
> >>
> >> Here's an example for an IntegrationTest that needs a Cache but not any
> >> CacheServers:
> >>
> >> private Cache cache;
> >>
> >> @Before
> >> public void setUp() {
> >>Properties config = new Properties();
> >>config.setProperty(LOCATORS, "");
> >>cache = new CacheFactory(config).create();
> >> }
> >>
> >> @After
> >> public void tearDown() {
> >>cache.close();
> >> }
> >>
> >> That's some pretty simple code and as a Developer, I can tell exactly
> what
> >> it's doing and what the config is.
> >>
> >> Here's an example of the kind of Geode User API code that I use to
> create
> >> Servers in a DistributedTests now:
> >>
> >>private void createServer(String serverName, File serverDir, int
> >> locatorPort) {
> >>  ServerLauncher.Builder builder = new ServerLauncher.Builder();
> >>  builder.setMemberName(serverName);
> >>  builder.setWorkingDirectory(serverDir.getAbsolutePath());
> >>  builder.setServerPort(0);
> >>  builder.set(LOCATORS, "localHost[" + locatorPort + "]");
> >>  builder.set(DISABLE_AUTO_RECONNECT, "false");
> >>  builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
> >>  builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
> >>  builder.set(MEMBER_TIMEOUT, "2000");
> >>
> >>  serverLauncher = builder.build();
> >>  serverLauncher.start();
> >>  assertThat(serverLauncher.isRunning()).isTrue();
> >>}
> >>
> >> In particular, I think we should be using ServerLauncher and
> >> LocatorLauncher instead of Rules when we want a full-stack Locator or
> >> full-stack Server that looks like what a User is going to startup.
> >>
> >> Here are my reasons:
> >>
> >> 1) I want to learn and use the Geode User APIs directly, not someone's
> >> (even mine) Testing API that hides the Geode User APIs. If I see a test
> >> fail, I want to see exactly what was configured and what User APIs were
> >> used right there in the test without having to open other classes. I
> don't
> >> want to have to spend even 15 minutes digging through some JUnit Rule to
> >> figure out how PDX was configured.
> >>
> >> 2) We need to make sure that the Geode User APIs are easy to use and are
> >> complete. If we're writing tests against Testing APIs instead then we
> >> don't
> >> feel the Users' pain if our API is painful. If the reason to use a Rule
> is
> >> because our User API is overly-verbose of difficult, then that's even
> more
> >> reason to use the Geode User API, so we recognize that it needs to
> change!
> >>
> >> GemFire had a long history of hiding its User APIs behind elaborate
> >> Testing
> >> APIs and we all used these fancy, easier to use, more compact Testing
> >> APIs.
> >> This promotes complicated, inconsistent and potentially incomplete User
> >> APIs for Users to actually use. The result: difficult to use product
> with
> >> difficult to use APIs and User APIs that are missing important things
> that
> >> then Users have to resort to internal APIs to use. I'm strongly
> convinced
> >> that using elaborate Testing APIs is at least largely responsible for
> >> making GemFire and now Geode difficult to use and that's why I'm pushing
> >> so
> >> hard to write tests with Geode User APIs instead of convenient custom
> >> Rules
> >>
> >> Since I started using ServerLauncher and LocatorLauncher APIs directly
> in
> >> my DistributedTests I made a very important 

Re: [DISCUSS] including Java11 in the pipelines

[Kirk Lund]

+1 I added my approval to your PRs. I don't know if the yml is correct or
not, but I support the overall goal.

On Thu, Nov 8, 2018 at 1:19 PM, Owen Nichols  wrote:

> Sounds like the overwhelming consensus is to keep Java11 gating, add
> Java11 to PR, and defer non-gating tests.
>
> I have prepared PRs for these changes:
> https://github.com/apache/geode/pull/2806 (add Java11 to PR pipeline)
> https://github.com/apache/geode/pull/2816 (run non-gating jobs only after
> gating jobs have passed)
>
> Please review and merge.
>
> Thanks,
> -Owen
>
> > On Nov 8, 2018, at 9:41 AM, Patrick Rhomberg 
> wrote:
> >
> > I agree with Jinmei on all points.  I definitely think there should be
> > parity between precheckin and the main pipeline, but that might just be
> > because I caused the main pipeline to fail on Java11 this week.
> >
> > On Wed, Nov 7, 2018 at 1:34 PM, Jinmei Liao  wrote:
> >
> >> First of all, I believe all gating tests should be run in precheckin.
> If we
> >> make jdk11 tests gating, we should make it part of the precheckin. If we
> >> don't put them in precheckin, they should not be gating.
> >>
> >> Secondly, If we don't make jdk11 tests gating, soon they will become
> like
> >> windows tests, people only look at them after it's been failing for
> days,
> >> which is not good.
> >>
> >> Thirdly, for non-gating tests, we probably should run them after all the
> >> gating tests are done.
> >>
> >> On Wed, Nov 7, 2018 at 11:39 AM Owen Nichols 
> wrote:
> >>
> >>> Now that tests are passing under Java 11, it was recommended last week
> to
> >>> make Java 11 tests gating for the develop pipeline.  [Fyi, Windows
> tests
> >>> are not yet gating, meaning the pipeline will success and publish
> >> artifacts
> >>> even if a Windows tests fails.]
> >>>
> >>> Three topics merit discussion:
> >>>
> >>> 1) For the Geode 1.8 release, should the release notes advertise
> >>> “experimental support for Java 11” or no support?  If the latter,
> should
> >>> Java 11 tests still be gating on the 1.8 release branch, or only on
> >> develop?
> >>>
> >>> 2) As of now, pre-checkin runs tests only against Java8.  Now that
> Java11
> >>> is gating in develop, should we now be testing against both Java8 and
> >>> Java11 as part of validating pull requests?
> >>>
> >>> 3) In the develop pipeline, should non-gating jobs continue to be run
> in
> >>> parallel with gating jobs?  Or would it be better to change the develop
> >>> pipeline to only run the non-gating tests after all gating jobs have
> >> passed?
> >>>
> >>> Thanks,
> >>> -Owen
> >>>
> >>> P.S. after a brief trial of combining Java8 and Java11 tests into a
> >> single
> >>> pipeline job, that was reverted and it looks like separate jobs are
> here
> >> to
> >>> stay.  Combined jobs made it difficult to look at dual scrolling
> outputs
> >>> and dual archive-results steps.  If you feel there’s a better solution,
> >>> please speak up.
> >>>
> >>>
> >>
> >> --
> >> Cheers
> >>
> >> Jinmei
> >>
>
>

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Kirk Lund]

We need to pull out the DUnit Blackboard from DistributedTestCase and
repackage it as a BlackboardRule. It makes sense to make that a JUnit Rule
because it's not part of Geode or its User APIs but it's really useful for
distributed testing in general. It's also probably the last useful thing
that's still in DistributedTestCase and no where else.

On Fri, Nov 9, 2018 at 12:12 PM, Bruce Schuchardt 
wrote:

> I agree with Kirk about the Rules and I agree with Galen about moving away
> from the old abstract test classes.  I think that is also in the spirit of
> what Kirk is saying.
>
> There are also tests that have complicated methods for creating caches and
> regions.  These methods have many parameters and are sometimes in Helper
> classes.  I've found these especially difficult to deal with when fixing
> flaky tests because changing one of the Helper methods affects many tests.
>
>
> On 11/9/18 11:31 AM, Kirk Lund wrote:
>
>> *I would like to encourage all Geode developers to start writing tests
>> directly against the Geode User APIs* even in DistributedTests. I'm no
>> longer using *CacheRule, ClientCacheRule, ServerStarterRule,
>> LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
>>
>> their use any longer. I'll explain why below.
>>
>> Here's an example for an IntegrationTest that needs a Cache but not any
>> CacheServers:
>>
>> private Cache cache;
>>
>> @Before
>> public void setUp() {
>>Properties config = new Properties();
>>config.setProperty(LOCATORS, "");
>>cache = new CacheFactory(config).create();
>> }
>>
>> @After
>> public void tearDown() {
>>cache.close();
>> }
>>
>> That's some pretty simple code and as a Developer, I can tell exactly what
>> it's doing and what the config is.
>>
>> Here's an example of the kind of Geode User API code that I use to create
>> Servers in a DistributedTests now:
>>
>>private void createServer(String serverName, File serverDir, int
>> locatorPort) {
>>  ServerLauncher.Builder builder = new ServerLauncher.Builder();
>>  builder.setMemberName(serverName);
>>  builder.setWorkingDirectory(serverDir.getAbsolutePath());
>>  builder.setServerPort(0);
>>  builder.set(LOCATORS, "localHost[" + locatorPort + "]");
>>  builder.set(DISABLE_AUTO_RECONNECT, "false");
>>  builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
>>  builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
>>  builder.set(MEMBER_TIMEOUT, "2000");
>>
>>  serverLauncher = builder.build();
>>  serverLauncher.start();
>>  assertThat(serverLauncher.isRunning()).isTrue();
>>}
>>
>> In particular, I think we should be using ServerLauncher and
>> LocatorLauncher instead of Rules when we want a full-stack Locator or
>> full-stack Server that looks like what a User is going to startup.
>>
>> Here are my reasons:
>>
>> 1) I want to learn and use the Geode User APIs directly, not someone's
>> (even mine) Testing API that hides the Geode User APIs. If I see a test
>> fail, I want to see exactly what was configured and what User APIs were
>> used right there in the test without having to open other classes. I don't
>> want to have to spend even 15 minutes digging through some JUnit Rule to
>> figure out how PDX was configured.
>>
>> 2) We need to make sure that the Geode User APIs are easy to use and are
>> complete. If we're writing tests against Testing APIs instead then we
>> don't
>> feel the Users' pain if our API is painful. If the reason to use a Rule is
>> because our User API is overly-verbose of difficult, then that's even more
>> reason to use the Geode User API, so we recognize that it needs to change!
>>
>> GemFire had a long history of hiding its User APIs behind elaborate
>> Testing
>> APIs and we all used these fancy, easier to use, more compact Testing
>> APIs.
>> This promotes complicated, inconsistent and potentially incomplete User
>> APIs for Users to actually use. The result: difficult to use product with
>> difficult to use APIs and User APIs that are missing important things that
>> then Users have to resort to internal APIs to use. I'm strongly convinced
>> that using elaborate Testing APIs is at least largely responsible for
>> making GemFire and now Geode difficult to use and that's why I'm pushing
>> so
>> hard to write tests with Geode User APIs instead of convenient custom
>> Rules
>>
>> Since I started using ServerLauncher and LocatorLauncher APIs directly in
>> my DistributedTests I made a very important discovery: the User has no way
>> to get a reference to the Cache. This is why I recently started a
>> discussion thread about add getCache and getLocator to these Launcher
>> APIs.
>> If we keep using elaborate Testing APIs including custom Geode JUnit Rules
>> to hide these APIs, we'll never make these discoveries that I feel are
>> vital for our Users. We need to make things a LOT easier for the Users
>> going forward.
>>
>> The above is why I think we should be using User APIs in the tests even

Re: [DISCUSS] Disable merge for failing pull requests

[Alexander Murmann]

+1

On Fri, Nov 9, 2018 at 12:58 PM Robert Houghton 
wrote:

> +1
>
> On Fri, Nov 9, 2018, 12:55 Dan Smith 
> > Hi all,
> >
> > Kirks emails reminded me - I think we are at the point now with our PR
> > checks where we should not be merging anything to develop that doesn't
> pass
> > all of the PR checks.
> >
> > I propose we disable the merge button unless a PR is passing all of the
> > checks. If we are in agreement I'll follow up with infra to see how to
> make
> > that happen.
> >
> > This would not completely prevent pushing directly to develop from the
> > command line, but since most developers seem to be using the github UI, I
> > hope that it will steer people towards getting the PRs passing instead of
> > using the command line.
> >
> > Thoughts?
> > -Dan
> >
>

Re: [DISCUSS] Disable merge for failing pull requests

[Owen Nichols]

+1 assuming “all of the PR checks” will soon include Java 11

Implementation-wise should be as simple at getting someone with admin 
privileges on the geode repo to change the settings for “develop” branch to 
"Require status checks to pass before merging."

> On Nov 9, 2018, at 12:58 PM, Robert Houghton  wrote:
> 
> +1
> 
> On Fri, Nov 9, 2018, 12:55 Dan Smith  
>> Hi all,
>> 
>> Kirks emails reminded me - I think we are at the point now with our PR
>> checks where we should not be merging anything to develop that doesn't pass
>> all of the PR checks.
>> 
>> I propose we disable the merge button unless a PR is passing all of the
>> checks. If we are in agreement I'll follow up with infra to see how to make
>> that happen.
>> 
>> This would not completely prevent pushing directly to develop from the
>> command line, but since most developers seem to be using the github UI, I
>> hope that it will steer people towards getting the PRs passing instead of
>> using the command line.
>> 
>> Thoughts?
>> -Dan
>> 

Re: [DISCUSS] Disable merge for failing pull requests

[Kirk Lund]

+1

On Fri, Nov 9, 2018 at 12:58 PM, Robert Houghton 
wrote:

> +1
>
> On Fri, Nov 9, 2018, 12:55 Dan Smith 
> > Hi all,
> >
> > Kirks emails reminded me - I think we are at the point now with our PR
> > checks where we should not be merging anything to develop that doesn't
> pass
> > all of the PR checks.
> >
> > I propose we disable the merge button unless a PR is passing all of the
> > checks. If we are in agreement I'll follow up with infra to see how to
> make
> > that happen.
> >
> > This would not completely prevent pushing directly to develop from the
> > command line, but since most developers seem to be using the github UI, I
> > hope that it will steer people towards getting the PRs passing instead of
> > using the command line.
> >
> > Thoughts?
> > -Dan
> >
>

Re: [DISCUSS] Disable merge for failing pull requests

[Robert Houghton]

+1

On Fri, Nov 9, 2018, 12:55 Dan Smith  Hi all,
>
> Kirks emails reminded me - I think we are at the point now with our PR
> checks where we should not be merging anything to develop that doesn't pass
> all of the PR checks.
>
> I propose we disable the merge button unless a PR is passing all of the
> checks. If we are in agreement I'll follow up with infra to see how to make
> that happen.
>
> This would not completely prevent pushing directly to develop from the
> command line, but since most developers seem to be using the github UI, I
> hope that it will steer people towards getting the PRs passing instead of
> using the command line.
>
> Thoughts?
> -Dan
>

[DISCUSS] Disable merge for failing pull requests

[Dan Smith]

Hi all,

Kirks emails reminded me - I think we are at the point now with our PR
checks where we should not be merging anything to develop that doesn't pass
all of the PR checks.

I propose we disable the merge button unless a PR is passing all of the
checks. If we are in agreement I'll follow up with infra to see how to make
that happen.

This would not completely prevent pushing directly to develop from the
command line, but since most developers seem to be using the github UI, I
hope that it will steer people towards getting the PRs passing instead of
using the command line.

Thoughts?
-Dan

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Bruce Schuchardt]

I agree with Kirk about the Rules and I agree with Galen about moving 
away from the old abstract test classes.  I think that is also in the 
spirit of what Kirk is saying.


There are also tests that have complicated methods for creating caches 
and regions.  These methods have many parameters and are sometimes in 
Helper classes.  I've found these especially difficult to deal with when 
fixing flaky tests because changing one of the Helper methods affects 
many tests.



On 11/9/18 11:31 AM, Kirk Lund wrote:

*I would like to encourage all Geode developers to start writing tests
directly against the Geode User APIs* even in DistributedTests. I'm no
longer using *CacheRule, ClientCacheRule, ServerStarterRule,
LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
their use any longer. I'll explain why below.

Here's an example for an IntegrationTest that needs a Cache but not any
CacheServers:

private Cache cache;

@Before
public void setUp() {
   Properties config = new Properties();
   config.setProperty(LOCATORS, "");
   cache = new CacheFactory(config).create();
}

@After
public void tearDown() {
   cache.close();
}

That's some pretty simple code and as a Developer, I can tell exactly what
it's doing and what the config is.

Here's an example of the kind of Geode User API code that I use to create
Servers in a DistributedTests now:

   private void createServer(String serverName, File serverDir, int
locatorPort) {
 ServerLauncher.Builder builder = new ServerLauncher.Builder();
 builder.setMemberName(serverName);
 builder.setWorkingDirectory(serverDir.getAbsolutePath());
 builder.setServerPort(0);
 builder.set(LOCATORS, "localHost[" + locatorPort + "]");
 builder.set(DISABLE_AUTO_RECONNECT, "false");
 builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
 builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
 builder.set(MEMBER_TIMEOUT, "2000");

 serverLauncher = builder.build();
 serverLauncher.start();
 assertThat(serverLauncher.isRunning()).isTrue();
   }

In particular, I think we should be using ServerLauncher and
LocatorLauncher instead of Rules when we want a full-stack Locator or
full-stack Server that looks like what a User is going to startup.

Here are my reasons:

1) I want to learn and use the Geode User APIs directly, not someone's
(even mine) Testing API that hides the Geode User APIs. If I see a test
fail, I want to see exactly what was configured and what User APIs were
used right there in the test without having to open other classes. I don't
want to have to spend even 15 minutes digging through some JUnit Rule to
figure out how PDX was configured.

2) We need to make sure that the Geode User APIs are easy to use and are
complete. If we're writing tests against Testing APIs instead then we don't
feel the Users' pain if our API is painful. If the reason to use a Rule is
because our User API is overly-verbose of difficult, then that's even more
reason to use the Geode User API, so we recognize that it needs to change!

GemFire had a long history of hiding its User APIs behind elaborate Testing
APIs and we all used these fancy, easier to use, more compact Testing APIs.
This promotes complicated, inconsistent and potentially incomplete User
APIs for Users to actually use. The result: difficult to use product with
difficult to use APIs and User APIs that are missing important things that
then Users have to resort to internal APIs to use. I'm strongly convinced
that using elaborate Testing APIs is at least largely responsible for
making GemFire and now Geode difficult to use and that's why I'm pushing so
hard to write tests with Geode User APIs instead of convenient custom Rules

Since I started using ServerLauncher and LocatorLauncher APIs directly in
my DistributedTests I made a very important discovery: the User has no way
to get a reference to the Cache. This is why I recently started a
discussion thread about add getCache and getLocator to these Launcher APIs.
If we keep using elaborate Testing APIs including custom Geode JUnit Rules
to hide these APIs, we'll never make these discoveries that I feel are
vital for our Users. We need to make things a LOT easier for the Users
going forward.

The above is why I think we should be using User APIs in the tests even for
setUp and tearDown. Save the custom JUnit Rules for NON-GEODE things like
configuring JSON or LOG4J or allowing use of ErrorCollector in all DUnit
VMs.

Thanks,
Kirk

On Fri, Nov 9, 2018 at 10:49 AM, Galen O'Sullivan 
wrote:


I was looking at a test recently that extended JUnit4CacheTestCase and only
used a single server, and changed it to use ClusterStartupRule.

JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase
and with the move to ClusterStartupRule for distributed tests, rather than
class inheritance, I think we should deprecate JUnit4CacheTestCase and
change the comment to imply that classes should inherit from it just
because they 

Re: RED ALARM: DistributedTestOpenJDK8 has failed -- no checkins to DEVELOP allowed

[Kirk Lund]

DistributedTestOpenJDK8 #99 is *green* and thanks to Jinmei we have a PR
filed for improving EvictionDUnitTest.

Please consider the RED ALARM resolved.

https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/DistributedTestOpenJDK8/builds/99

On Fri, Nov 9, 2018 at 11:36 AM, Kirk Lund  wrote:

> Thanks! I'm waiting for Build #99 to finish (hopefully green) and I'll
> send out one more update on this.
>
> On Fri, Nov 9, 2018 at 11:33 AM, Jinmei Liao  wrote:
>
>> It is a flaky test, I have a PR that would give more detailed error
>> message
>> when it fails:
>>
>> https://github.com/apache/geode/pull/2808
>>
>> On Fri, Nov 9, 2018 at 10:47 AM Kirk Lund  wrote:
>>
>> > This test has failed in DistributedTestOpenJDK8 #98. Nobody can merge to
>> > DEVELOP until I announce that the RED ALARM is turned off and the Build
>> is
>> > fixed. There is an exception: the fix for this test can be committed to
>> > DEVELOP.
>> >
>> >
>> > Thank you,
>> >
>> > Kirk
>> >
>> >
>> > GEODE-5943: EvictionDUnitTest failed in jdk11
>> >
>> > https://issues.apache.org/jira/browse/GEODE-5943
>> >
>> >
>> >
>> > https://concourse.apachegeode-ci.info/teams/main/pipelines/a
>> pache-develop-main/jobs/DistributedTestOpenJDK8/builds/98
>> >
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest >
>> > testDummyInlineNCentralizedEviction[offHeap=false] FAILED
>> > org.apache.geode.test.dunit.RMIException: While invoking
>> >
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest$$
>> Lambda$58/1839308999.call
>> > in VM 0 running on Host a7d4026cfc8d with 4 VMs
>> > at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
>> > at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
>> > at org.apache.geode.test.dunit.VM.invoke(VM.java:384)
>> > at
>> > org.apache.geode.test.junit.rules.VMProvider.invoke(VMProvider.java:74)
>> > at
>> >
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.t
>> estDummyInlineNCentralizedEviction(EvictionDUnitTest.java:124)
>> >
>> > Caused by:
>> > org.awaitility.core.ConditionTimeoutException: Condition with
>> > lambda expression in
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest that uses
>> > org.apache.geode.internal.cache.LocalRegion,
>> > org.apache.geode.internal.cache.LocalRegionint was not fulfilled
>> within 300
>> > seconds.
>> > at
>> > org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:145)
>> > at
>> > org.awaitility.core.CallableCondition.await(CallableCondition.java:79)
>> > at
>> > org.awaitility.core.CallableCondition.await(CallableCondition.java:27)
>> > at
>> > org.awaitility.core.ConditionFactory.until(ConditionFactory.java:890)
>> > at
>> > org.awaitility.core.ConditionFactory.until(ConditionFactory.java:848)
>> > at
>> >
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.s
>> endEventAndWaitForExpectedEviction(EvictionDUnitTest.java:249)
>> > at
>> >
>> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.
>> lambda$testDummyInlineNCentralizedEviction$b8b59e6f$1(Evicti
>> onDUnitTest.java:124)
>> >
>>
>>
>> --
>> Cheers
>>
>> Jinmei
>>
>
>

Re: RED ALARM: DistributedTestOpenJDK8 has failed -- no checkins to DEVELOP allowed

[Kirk Lund]

Thanks! I'm waiting for Build #99 to finish (hopefully green) and I'll send
out one more update on this.

On Fri, Nov 9, 2018 at 11:33 AM, Jinmei Liao  wrote:

> It is a flaky test, I have a PR that would give more detailed error message
> when it fails:
>
> https://github.com/apache/geode/pull/2808
>
> On Fri, Nov 9, 2018 at 10:47 AM Kirk Lund  wrote:
>
> > This test has failed in DistributedTestOpenJDK8 #98. Nobody can merge to
> > DEVELOP until I announce that the RED ALARM is turned off and the Build
> is
> > fixed. There is an exception: the fix for this test can be committed to
> > DEVELOP.
> >
> >
> > Thank you,
> >
> > Kirk
> >
> >
> > GEODE-5943: EvictionDUnitTest failed in jdk11
> >
> > https://issues.apache.org/jira/browse/GEODE-5943
> >
> >
> >
> > https://concourse.apachegeode-ci.info/teams/main/pipelines/
> apache-develop-main/jobs/DistributedTestOpenJDK8/builds/98
> >
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest >
> > testDummyInlineNCentralizedEviction[offHeap=false] FAILED
> > org.apache.geode.test.dunit.RMIException: While invoking
> >
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest$$Lambda$58/
> 1839308999.call
> > in VM 0 running on Host a7d4026cfc8d with 4 VMs
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:384)
> > at
> > org.apache.geode.test.junit.rules.VMProvider.invoke(VMProvider.java:74)
> > at
> >
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.
> testDummyInlineNCentralizedEviction(EvictionDUnitTest.java:124)
> >
> > Caused by:
> > org.awaitility.core.ConditionTimeoutException: Condition with
> > lambda expression in
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest that uses
> > org.apache.geode.internal.cache.LocalRegion,
> > org.apache.geode.internal.cache.LocalRegionint was not fulfilled within
> 300
> > seconds.
> > at
> > org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:145)
> > at
> > org.awaitility.core.CallableCondition.await(CallableCondition.java:79)
> > at
> > org.awaitility.core.CallableCondition.await(CallableCondition.java:27)
> > at
> > org.awaitility.core.ConditionFactory.until(ConditionFactory.java:890)
> > at
> > org.awaitility.core.ConditionFactory.until(ConditionFactory.java:848)
> > at
> >
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.
> sendEventAndWaitForExpectedEviction(EvictionDUnitTest.java:249)
> > at
> >
> > org.apache.geode.internal.cache.eviction.EvictionDUnitTest.lambda$
> testDummyInlineNCentralizedEviction$b8b59e6f$1(EvictionDUnitTest.java:124)
> >
>
>
> --
> Cheers
>
> Jinmei
>

Re: RED ALARM: DistributedTestOpenJDK8 has failed -- no checkins to DEVELOP allowed

[Jinmei Liao]

It is a flaky test, I have a PR that would give more detailed error message
when it fails:

https://github.com/apache/geode/pull/2808

On Fri, Nov 9, 2018 at 10:47 AM Kirk Lund  wrote:

> This test has failed in DistributedTestOpenJDK8 #98. Nobody can merge to
> DEVELOP until I announce that the RED ALARM is turned off and the Build is
> fixed. There is an exception: the fix for this test can be committed to
> DEVELOP.
>
>
> Thank you,
>
> Kirk
>
>
> GEODE-5943: EvictionDUnitTest failed in jdk11
>
> https://issues.apache.org/jira/browse/GEODE-5943
>
>
>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/DistributedTestOpenJDK8/builds/98
>
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest >
> testDummyInlineNCentralizedEviction[offHeap=false] FAILED
> org.apache.geode.test.dunit.RMIException: While invoking
>
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest$$Lambda$58/1839308999.call
> in VM 0 running on Host a7d4026cfc8d with 4 VMs
> at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:384)
> at
> org.apache.geode.test.junit.rules.VMProvider.invoke(VMProvider.java:74)
> at
>
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest.testDummyInlineNCentralizedEviction(EvictionDUnitTest.java:124)
>
> Caused by:
> org.awaitility.core.ConditionTimeoutException: Condition with
> lambda expression in
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest that uses
> org.apache.geode.internal.cache.LocalRegion,
> org.apache.geode.internal.cache.LocalRegionint was not fulfilled within 300
> seconds.
> at
> org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:145)
> at
> org.awaitility.core.CallableCondition.await(CallableCondition.java:79)
> at
> org.awaitility.core.CallableCondition.await(CallableCondition.java:27)
> at
> org.awaitility.core.ConditionFactory.until(ConditionFactory.java:890)
> at
> org.awaitility.core.ConditionFactory.until(ConditionFactory.java:848)
> at
>
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest.sendEventAndWaitForExpectedEviction(EvictionDUnitTest.java:249)
> at
>
> org.apache.geode.internal.cache.eviction.EvictionDUnitTest.lambda$testDummyInlineNCentralizedEviction$b8b59e6f$1(EvictionDUnitTest.java:124)
>


-- 
Cheers

Jinmei

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Kirk Lund]

*I would like to encourage all Geode developers to start writing tests
directly against the Geode User APIs* even in DistributedTests. I'm no
longer using *CacheRule, ClientCacheRule, ServerStarterRule,
LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
their use any longer. I'll explain why below.

Here's an example for an IntegrationTest that needs a Cache but not any
CacheServers:

private Cache cache;

@Before
public void setUp() {
  Properties config = new Properties();
  config.setProperty(LOCATORS, "");
  cache = new CacheFactory(config).create();
}

@After
public void tearDown() {
  cache.close();
}

That's some pretty simple code and as a Developer, I can tell exactly what
it's doing and what the config is.

Here's an example of the kind of Geode User API code that I use to create
Servers in a DistributedTests now:

  private void createServer(String serverName, File serverDir, int
locatorPort) {
ServerLauncher.Builder builder = new ServerLauncher.Builder();
builder.setMemberName(serverName);
builder.setWorkingDirectory(serverDir.getAbsolutePath());
builder.setServerPort(0);
builder.set(LOCATORS, "localHost[" + locatorPort + "]");
builder.set(DISABLE_AUTO_RECONNECT, "false");
builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
builder.set(MEMBER_TIMEOUT, "2000");

serverLauncher = builder.build();
serverLauncher.start();
assertThat(serverLauncher.isRunning()).isTrue();
  }

In particular, I think we should be using ServerLauncher and
LocatorLauncher instead of Rules when we want a full-stack Locator or
full-stack Server that looks like what a User is going to startup.

Here are my reasons:

1) I want to learn and use the Geode User APIs directly, not someone's
(even mine) Testing API that hides the Geode User APIs. If I see a test
fail, I want to see exactly what was configured and what User APIs were
used right there in the test without having to open other classes. I don't
want to have to spend even 15 minutes digging through some JUnit Rule to
figure out how PDX was configured.

2) We need to make sure that the Geode User APIs are easy to use and are
complete. If we're writing tests against Testing APIs instead then we don't
feel the Users' pain if our API is painful. If the reason to use a Rule is
because our User API is overly-verbose of difficult, then that's even more
reason to use the Geode User API, so we recognize that it needs to change!

GemFire had a long history of hiding its User APIs behind elaborate Testing
APIs and we all used these fancy, easier to use, more compact Testing APIs.
This promotes complicated, inconsistent and potentially incomplete User
APIs for Users to actually use. The result: difficult to use product with
difficult to use APIs and User APIs that are missing important things that
then Users have to resort to internal APIs to use. I'm strongly convinced
that using elaborate Testing APIs is at least largely responsible for
making GemFire and now Geode difficult to use and that's why I'm pushing so
hard to write tests with Geode User APIs instead of convenient custom Rules

Since I started using ServerLauncher and LocatorLauncher APIs directly in
my DistributedTests I made a very important discovery: the User has no way
to get a reference to the Cache. This is why I recently started a
discussion thread about add getCache and getLocator to these Launcher APIs.
If we keep using elaborate Testing APIs including custom Geode JUnit Rules
to hide these APIs, we'll never make these discoveries that I feel are
vital for our Users. We need to make things a LOT easier for the Users
going forward.

The above is why I think we should be using User APIs in the tests even for
setUp and tearDown. Save the custom JUnit Rules for NON-GEODE things like
configuring JSON or LOG4J or allowing use of ErrorCollector in all DUnit
VMs.

Thanks,
Kirk

On Fri, Nov 9, 2018 at 10:49 AM, Galen O'Sullivan 
wrote:

> I was looking at a test recently that extended JUnit4CacheTestCase and only
> used a single server, and changed it to use ClusterStartupRule.
>
> JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase
> and with the move to ClusterStartupRule for distributed tests, rather than
> class inheritance, I think we should deprecate JUnit4CacheTestCase and
> change the comment to imply that classes should inherit from it just
> because they require a Cache.
>
> Is is worth deprecating JUnit4DistributedTestCase as well and encouraging
> the use of ClusterStartupRule instead?
>
> Thanks,
> Galen
>

Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Jinmei Liao]

+1 on this. We should break away from extending base test class now. It
makes it quite hard to understand how the test cluster is set up and what
the test is doing.

On Fri, Nov 9, 2018 at 10:50 AM Galen O'Sullivan 
wrote:

> I was looking at a test recently that extended JUnit4CacheTestCase and only
> used a single server, and changed it to use ClusterStartupRule.
>
> JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase
> and with the move to ClusterStartupRule for distributed tests, rather than
> class inheritance, I think we should deprecate JUnit4CacheTestCase and
> change the comment to imply that classes should inherit from it just
> because they require a Cache.
>
> Is is worth deprecating JUnit4DistributedTestCase as well and encouraging
> the use of ClusterStartupRule instead?
>
> Thanks,
> Galen
>


-- 
Cheers

Jinmei

[DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)

[Galen O'Sullivan]

I was looking at a test recently that extended JUnit4CacheTestCase and only
used a single server, and changed it to use ClusterStartupRule.

JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase
and with the move to ClusterStartupRule for distributed tests, rather than
class inheritance, I think we should deprecate JUnit4CacheTestCase and
change the comment to imply that classes should inherit from it just
because they require a Cache.

Is is worth deprecating JUnit4DistributedTestCase as well and encouraging
the use of ClusterStartupRule instead?

Thanks,
Galen

Re: RED ALARM: build has failed -- no checkins to DEVELOP allowed

[Kirk Lund]

Build checkPom has been restored to GREEN.


On Fri, Nov 9, 2018 at 10:21 AM, Kirk Lund  wrote:

> PS: The fix for checkPom an be committed to DEVELOP.
>
>
> On Fri, Nov 9, 2018 at 10:14 AM, Kirk Lund  wrote:
>
>> Now that I have your attention ;) I'd like to make broken builds a BIGGER
>> DEAL! Nobody can merge to DEVELOP until I announce that the RED ALARM is
>> turned off and the Build is fixed.
>>
>> Thank you,
>> Kirk
>>
>> GEODE-6024: Develop build fails due to checkPom (unknown dependency added)
>> https://issues.apache.org/jira/browse/GEODE-6024
>>
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/a
>> pache-develop-main/jobs/Build/builds/135
>>
>> FAILURE: Build completed with 10 failures.
>>
>> 1: Task failed with an exception.
>> ---
>> * Where:
>> Script '/home/geode/geode/gradle/publish.gradle' line: 180
>>
>> * What went wrong:
>> Execution failed for task ':geode-connectors:checkPom'.
>> > The geode-connectors pom-default.xml has changed. Verify dependencies.
>>   When changes have been approved, copy :
>>cp 
>> /home/geode/geode/geode-connectors/build/publications/maven/pom-default.xml
>> file:/home/geode/geode/geode-connectors/src/test/resources/e
>> xpected-pom.xml
>>   Removed Dependencies
>>   --
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-common]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-core]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>
>>   New Dependencies
>>   --
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-common]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-core]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>
>>
>>
>> * Try:
>> Run with --stacktrace option to get the stack trace. Run with --info or
>> --debug option to get more log output. Run with --scan to get full insights.
>> 
>> ==
>>
>> 2: Task failed with an exception.
>> ---
>> * Where:
>> Script '/home/geode/geode/gradle/publish.gradle' line: 180
>>
>> * What went wrong:
>> Execution failed for task ':geode-cq:checkPom'.
>> > The geode-cq pom-default.xml has changed. Verify dependencies.
>>   When changes have been approved, copy :
>>cp /home/geode/geode/geode-cq/build/publications/maven/pom-default.xml
>> file:/home/geode/geode/geode-cq/src/test/resources/expected-pom.xml
>>   Removed Dependencies
>>   --
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-core]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>
>>   New Dependencies
>>   --
>>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
>> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
>> value=[org.apache.geode]], {http://maven.apache.org/POM/4
>> .0.0}artifactId[attributes={}; value=[geode-core]], {
>> http://maven.apache.org/POM/4.0.0}version[attributes={};
>> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/4
>> .0.0}scope[attributes={}; value=[compile
>>
>>
>>
>> * Try:
>> Run with --stacktrace option to get the stack trace. Run with --info or
>> --debug option to get more log output. Run with --scan to get full insights.
>> 
>> ==
>>
>> 3: Task failed with an exception.
>> ---
>> * Where:
>> Script 

Re: RED ALARM: build has failed -- no checkins to DEVELOP allowed

[Kirk Lund]

PS: The fix for checkPom an be committed to DEVELOP.


On Fri, Nov 9, 2018 at 10:14 AM, Kirk Lund  wrote:

> Now that I have your attention ;) I'd like to make broken builds a BIGGER
> DEAL! Nobody can merge to DEVELOP until I announce that the RED ALARM is
> turned off and the Build is fixed.
>
> Thank you,
> Kirk
>
> GEODE-6024: Develop build fails due to checkPom (unknown dependency added)
> https://issues.apache.org/jira/browse/GEODE-6024
>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> apache-develop-main/jobs/Build/builds/135
>
> FAILURE: Build completed with 10 failures.
>
> 1: Task failed with an exception.
> ---
> * Where:
> Script '/home/geode/geode/gradle/publish.gradle' line: 180
>
> * What went wrong:
> Execution failed for task ':geode-connectors:checkPom'.
> > The geode-connectors pom-default.xml has changed. Verify dependencies.
>   When changes have been approved, copy :
>cp 
> /home/geode/geode/geode-connectors/build/publications/maven/pom-default.xml
> file:/home/geode/geode/geode-connectors/src/test/resources/
> expected-pom.xml
>   Removed Dependencies
>   --
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-common]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-core]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>
>   New Dependencies
>   --
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-common]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-core]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>
>
>
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or
> --debug option to get more log output. Run with --scan to get full insights.
> 
> ==
>
> 2: Task failed with an exception.
> ---
> * Where:
> Script '/home/geode/geode/gradle/publish.gradle' line: 180
>
> * What went wrong:
> Execution failed for task ':geode-cq:checkPom'.
> > The geode-cq pom-default.xml has changed. Verify dependencies.
>   When changes have been approved, copy :
>cp /home/geode/geode/geode-cq/build/publications/maven/pom-default.xml
> file:/home/geode/geode/geode-cq/src/test/resources/expected-pom.xml
>   Removed Dependencies
>   --
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-core]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.8.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>
>   New Dependencies
>   --
>   {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
> http://maven.apache.org/POM/4.0.0}groupId[attributes={};
> value=[org.apache.geode]], {http://maven.apache.org/POM/
> 4.0.0}artifactId[attributes={}; value=[geode-core]], {
> http://maven.apache.org/POM/4.0.0}version[attributes={};
> value=[1.9.0-SNAPSHOT]], {http://maven.apache.org/POM/
> 4.0.0}scope[attributes={}; value=[compile
>
>
>
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or
> --debug option to get more log output. Run with --scan to get full insights.
> 
> ==
>
> 3: Task failed with an exception.
> ---
> * Where:
> Script '/home/geode/geode/gradle/publish.gradle' line: 180
>
> * What went wrong:
> Execution failed for task ':geode-dunit:checkPom'.
> > The geode-dunit pom-default.xml has changed. Verify dependencies.
>   When changes have 

RED ALARM: build has failed -- no checkins to DEVELOP allowed

[Kirk Lund]

Now that I have your attention ;) I'd like to make broken builds a BIGGER
DEAL! Nobody can merge to DEVELOP until I announce that the RED ALARM is
turned off and the Build is fixed.

Thank you,
Kirk

GEODE-6024: Develop build fails due to checkPom (unknown dependency added)
https://issues.apache.org/jira/browse/GEODE-6024

https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/Build/builds/135

FAILURE: Build completed with 10 failures.

1: Task failed with an exception.
---
* Where:
Script '/home/geode/geode/gradle/publish.gradle' line: 180

* What went wrong:
Execution failed for task ':geode-connectors:checkPom'.
> The geode-connectors pom-default.xml has changed. Verify dependencies.
  When changes have been approved, copy :
   cp
/home/geode/geode/geode-connectors/build/publications/maven/pom-default.xml
file:/home/geode/geode/geode-connectors/src/test/resources/expected-pom.xml
  Removed Dependencies
  --
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-common]], {
http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.8.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-core]], {http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.8.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile

  New Dependencies
  --
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-common]], {
http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.9.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-core]], {http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.9.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile



* Try:
Run with --stacktrace option to get the stack trace. Run with --info or
--debug option to get more log output. Run with --scan to get full insights.
==

2: Task failed with an exception.
---
* Where:
Script '/home/geode/geode/gradle/publish.gradle' line: 180

* What went wrong:
Execution failed for task ':geode-cq:checkPom'.
> The geode-cq pom-default.xml has changed. Verify dependencies.
  When changes have been approved, copy :
   cp /home/geode/geode/geode-cq/build/publications/maven/pom-default.xml
file:/home/geode/geode/geode-cq/src/test/resources/expected-pom.xml
  Removed Dependencies
  --
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-core]], {http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.8.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile

  New Dependencies
  --
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{
http://maven.apache.org/POM/4.0.0}groupId[attributes={};
value=[org.apache.geode]], {
http://maven.apache.org/POM/4.0.0}artifactId[attributes={};
value=[geode-core]], {http://maven.apache.org/POM/4.0.0}version[attributes={};
value=[1.9.0-SNAPSHOT]], {
http://maven.apache.org/POM/4.0.0}scope[attributes={}; value=[compile



* Try:
Run with --stacktrace option to get the stack trace. Run with --info or
--debug option to get more log output. Run with --scan to get full insights.
==

3: Task failed with an exception.
---
* Where:
Script '/home/geode/geode/gradle/publish.gradle' line: 180

* What went wrong:
Execution failed for task ':geode-dunit:checkPom'.
> The geode-dunit pom-default.xml has changed. Verify dependencies.
  When changes have been approved, copy :
   cp
/home/geode/geode/geode-dunit/build/publications/maven/pom-default.xml
file:/home/geode/geode/geode-dunit/src/test/resources/expected-pom.xml
  Removed Dependencies
  --
  {http://maven.apache.org/POM/4.0.0}dependency[attributes={}; value=[{

Re: Lombok

[Aditya Anchuri]

I'm happy to put this work on hold for now. As I mentioned there are other
benefits beyond just getters and setters -- but I guess the risk with JDK10
(I was unaware of the problems between Lombok and JDK10) may be unknown
enough for us to take pause. Thanks for all your input!

-Aditya

On Fri, Nov 9, 2018 at 9:52 AM Aditya Anchuri  wrote:

> I apologize, I was slightly wrong earlier with regards to the the IntelliJ
> Lombok plugin -- people will not need the IntelliJ plugin for their code to
> compile -- but they will need to enable compile-time annotation processing
> as an option. The plugin is a nice-to-have.
>
>
>
> On Fri, Nov 9, 2018 at 9:40 AM Kirk Lund  wrote:
>
>> -1 Personally, I prefer to being able to see and manipulate ALL code. I
>> hate autogenerated code, and I don't mind boilerplate code. When I see a
>> class that has too many getters, then I see that as a sign that we need to
>> do some refactoring and correct the design of that class. Using Lombok
>> would hide too much and make it less obvious that we have Encapsulation
>> and
>> God Class problems to fix and change.
>>
>> Generated code also makes stepping through code with a debugger and doing
>> performance engineering a nightmare.
>>
>> On Fri, Nov 9, 2018 at 9:07 AM, Anthony Baker  wrote:
>>
>> > I was talking with Dale and he pointed me to this discussion:
>> > https://github.com/jhipster/generator-jhipster/issues/398 <
>> > https://github.com/jhipster/generator-jhipster/issues/398>
>> >
>> > I think it probably warrants more investigation (e.g. do the issues
>> > decried on the the internet still exist?) before we adopt this.
>> >
>> > Anthony
>> >
>> >
>> > > On Nov 9, 2018, at 9:00 AM, Jinmei Liao  wrote:
>> > >
>> > > So users who wish to download our code will need to read some
>> > documentation
>> > > on how to setup IDEA/Eclipse before they can compile. It's not a
>> simple
>> > > "import and work with default IDEA setup" now.
>> > >
>> > > +0 on this. Personally I would prefer to bear with the extra
>> > getter/setters
>> > > than giving new contributors more headache at startup.
>> > >
>> > > On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer  wrote:
>> > >
>> > >> As an informative comparison on conciseness offering same
>> functionality:
>> > >>
>> > >> Java Code:
>> > >>
>> > >> import java.io.Serializable; import lombok.Getter; import
>> > >> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver;
>> > import
>> > >> org.apache.geode.cache.configuration.DeclarableType; /** * This class
>> > >> stores the arguments provided in the create
>> > >> gateway-receiver command. */ @Getter public class
>> > >> GatewayReceiverFunctionArgsimplements Serializable {
>> > >>private static final long serialVersionUID =
>> -5158224572470173267L;
>> > >> private final BooleanmanualStart; private final IntegerstartPort;
>> > private
>> > >> final IntegerendPort; private final StringbindAddress; private final
>> > >> IntegersocketBufferSize; private final
>> IntegermaximumTimeBetweenPings;
>> > >> private final String[]gatewayTransportFilters; private final
>> > >> StringhostnameForSenders; private final BooleanifNotExists; public
>> > >> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean
>> > >> ifNotExists) {
>> > >>   this.manualStart = configuration.isManualStart();
>> this.startPort =
>> > >>  configuration.getStartPort() !=null ?
>> > >> Integer.valueOf(configuration.getStartPort()) :null; this.endPort =
>> > >>  configuration.getEndPort() !=null ?
>> > >> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress =
>> > >> configuration.getBindAddress(); this.socketBufferSize =
>> > >> configuration.getSocketBufferSize() !=null ?
>> > >> Integer.valueOf(configuration.getSocketBufferSize()) :null;
>> > >> this.maximumTimeBetweenPings = configuration.
>> > getMaximumTimeBetweenPings()
>> > >> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings())
>> > :null;
>> > >> this.gatewayTransportFilters = configuration.
>> > getGatewayTransportFilter()
>> > >> !=null ?
>> > >>
>> configuration.getGatewayTransportFilter().stream().map(DeclarableType::
>> > getClassName)
>> > >>  .toArray(String[]::new)
>> > >>  :null; this.hostnameForSenders =
>> > >> configuration.getHostnameForSenders(); this.ifNotExists =
>> ifNotExists;
>> > }
>> > >> }
>> > >>
>> > >> The equivalent Kotlin definition is:
>> > >>
>> > >> import org.apache.geode.cache.configuration.CacheConfig
>> > >> import org.apache.geode.cache.configuration.ClassWithParametersType
>> > >> import java.io.Serializable
>> > >>
>> > >> data class GatewayReceiverFunctionArgs
>> > >> @JvmOverloads private constructor(val manualStart: Boolean, val
>> > startPort:
>> > >> Int?, val endPort: Int?, val bindAddress: String, val
>> socketBufferSize:
>> > >> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters:
>> > >> Array, val hostNameForSender: String, val ifNotExists:
>> Boolean)
>> > :
>> 

Re: Lombok

[Aditya Anchuri]

I apologize, I was slightly wrong earlier with regards to the the IntelliJ
Lombok plugin -- people will not need the IntelliJ plugin for their code to
compile -- but they will need to enable compile-time annotation processing
as an option. The plugin is a nice-to-have.



On Fri, Nov 9, 2018 at 9:40 AM Kirk Lund  wrote:

> -1 Personally, I prefer to being able to see and manipulate ALL code. I
> hate autogenerated code, and I don't mind boilerplate code. When I see a
> class that has too many getters, then I see that as a sign that we need to
> do some refactoring and correct the design of that class. Using Lombok
> would hide too much and make it less obvious that we have Encapsulation and
> God Class problems to fix and change.
>
> Generated code also makes stepping through code with a debugger and doing
> performance engineering a nightmare.
>
> On Fri, Nov 9, 2018 at 9:07 AM, Anthony Baker  wrote:
>
> > I was talking with Dale and he pointed me to this discussion:
> > https://github.com/jhipster/generator-jhipster/issues/398 <
> > https://github.com/jhipster/generator-jhipster/issues/398>
> >
> > I think it probably warrants more investigation (e.g. do the issues
> > decried on the the internet still exist?) before we adopt this.
> >
> > Anthony
> >
> >
> > > On Nov 9, 2018, at 9:00 AM, Jinmei Liao  wrote:
> > >
> > > So users who wish to download our code will need to read some
> > documentation
> > > on how to setup IDEA/Eclipse before they can compile. It's not a simple
> > > "import and work with default IDEA setup" now.
> > >
> > > +0 on this. Personally I would prefer to bear with the extra
> > getter/setters
> > > than giving new contributors more headache at startup.
> > >
> > > On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer  wrote:
> > >
> > >> As an informative comparison on conciseness offering same
> functionality:
> > >>
> > >> Java Code:
> > >>
> > >> import java.io.Serializable; import lombok.Getter; import
> > >> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver;
> > import
> > >> org.apache.geode.cache.configuration.DeclarableType; /** * This class
> > >> stores the arguments provided in the create
> > >> gateway-receiver command. */ @Getter public class
> > >> GatewayReceiverFunctionArgsimplements Serializable {
> > >>private static final long serialVersionUID = -5158224572470173267L;
> > >> private final BooleanmanualStart; private final IntegerstartPort;
> > private
> > >> final IntegerendPort; private final StringbindAddress; private final
> > >> IntegersocketBufferSize; private final IntegermaximumTimeBetweenPings;
> > >> private final String[]gatewayTransportFilters; private final
> > >> StringhostnameForSenders; private final BooleanifNotExists; public
> > >> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean
> > >> ifNotExists) {
> > >>   this.manualStart = configuration.isManualStart();
> this.startPort =
> > >>  configuration.getStartPort() !=null ?
> > >> Integer.valueOf(configuration.getStartPort()) :null; this.endPort =
> > >>  configuration.getEndPort() !=null ?
> > >> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress =
> > >> configuration.getBindAddress(); this.socketBufferSize =
> > >> configuration.getSocketBufferSize() !=null ?
> > >> Integer.valueOf(configuration.getSocketBufferSize()) :null;
> > >> this.maximumTimeBetweenPings = configuration.
> > getMaximumTimeBetweenPings()
> > >> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings())
> > :null;
> > >> this.gatewayTransportFilters = configuration.
> > getGatewayTransportFilter()
> > >> !=null ?
> > >>
> configuration.getGatewayTransportFilter().stream().map(DeclarableType::
> > getClassName)
> > >>  .toArray(String[]::new)
> > >>  :null; this.hostnameForSenders =
> > >> configuration.getHostnameForSenders(); this.ifNotExists = ifNotExists;
> > }
> > >> }
> > >>
> > >> The equivalent Kotlin definition is:
> > >>
> > >> import org.apache.geode.cache.configuration.CacheConfig
> > >> import org.apache.geode.cache.configuration.ClassWithParametersType
> > >> import java.io.Serializable
> > >>
> > >> data class GatewayReceiverFunctionArgs
> > >> @JvmOverloads private constructor(val manualStart: Boolean, val
> > startPort:
> > >> Int?, val endPort: Int?, val bindAddress: String, val
> socketBufferSize:
> > >> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters:
> > >> Array, val hostNameForSender: String, val ifNotExists:
> Boolean)
> > :
> > >> Serializable{
> > >>
> > >> constructor(configuration: CacheConfig.GatewayReceiver,
> ifNotExists:
> > >> Boolean) :
> > >> this(configuration.isManualStart,
> > >> configuration.startPort?.toInt(), configuration.endPort?.toInt(),
> > >> configuration.bindAddress, configuration.socketBufferSize?.toInt(),
> > >> configuration.maximumTimeBetweenPings?.toInt(),
> > >> configuration.gatewayTransportFilter ?.map { classWithParametersType:
> > >> 

Re: Apache Geode Branch Housekeeping

[Michael Stolz]

The branch named "master" claims to be mine.
I don't exactly know how it became "mine".

--
Mike Stolz
Principal Engineer, GemFire Product Lead
Mobile: +1-631-835-4771



On Thu, Nov 8, 2018 at 7:55 PM Patrick Rhomberg 
wrote:

> Hello all!
>
>   We currently have 182 branches on the apache/geode repository.  This is
> excessive.
>   Please fork the main repository and save your branches there.  If you
> must use the apache/geode repository for some reason, please be diligent in
> your removal of dead branches.
>
>   *I would like to begin removing branches soon.*
>
>   Many branches appear to refer to JIRA tickets that are closed, or have
> pull requests against them that have been merged.  These seem safe to
> delete.
>   Many branches are thousands of commits behind develop or months to even
> years old.  Some of these branches have no commits since they were
> branched.  Release branches notwithstanding, these seem relatively safe to
> delete.  I suspect there are some that should be maintained for legal or
> legacy reasons, in which case please let me know.
>
>   This can be done most safely if you, as a responsible developer and
> reasonably-clean adult, were to pick up your own room.  If you have
> anything listed under "Your branches" at [1], please copy them to your fork
> and remove them from apache/geode.
>
> Thank you.
>
> Imagination is Change.
> ~ Patrick
>
> [1] https://github.com/apache/geode/branches/yours
>

Re: Lombok

[Kirk Lund]

-1 Personally, I prefer to being able to see and manipulate ALL code. I
hate autogenerated code, and I don't mind boilerplate code. When I see a
class that has too many getters, then I see that as a sign that we need to
do some refactoring and correct the design of that class. Using Lombok
would hide too much and make it less obvious that we have Encapsulation and
God Class problems to fix and change.

Generated code also makes stepping through code with a debugger and doing
performance engineering a nightmare.

On Fri, Nov 9, 2018 at 9:07 AM, Anthony Baker  wrote:

> I was talking with Dale and he pointed me to this discussion:
> https://github.com/jhipster/generator-jhipster/issues/398 <
> https://github.com/jhipster/generator-jhipster/issues/398>
>
> I think it probably warrants more investigation (e.g. do the issues
> decried on the the internet still exist?) before we adopt this.
>
> Anthony
>
>
> > On Nov 9, 2018, at 9:00 AM, Jinmei Liao  wrote:
> >
> > So users who wish to download our code will need to read some
> documentation
> > on how to setup IDEA/Eclipse before they can compile. It's not a simple
> > "import and work with default IDEA setup" now.
> >
> > +0 on this. Personally I would prefer to bear with the extra
> getter/setters
> > than giving new contributors more headache at startup.
> >
> > On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer  wrote:
> >
> >> As an informative comparison on conciseness offering same functionality:
> >>
> >> Java Code:
> >>
> >> import java.io.Serializable; import lombok.Getter; import
> >> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver;
> import
> >> org.apache.geode.cache.configuration.DeclarableType; /** * This class
> >> stores the arguments provided in the create
> >> gateway-receiver command. */ @Getter public class
> >> GatewayReceiverFunctionArgsimplements Serializable {
> >>private static final long serialVersionUID = -5158224572470173267L;
> >> private final BooleanmanualStart; private final IntegerstartPort;
> private
> >> final IntegerendPort; private final StringbindAddress; private final
> >> IntegersocketBufferSize; private final IntegermaximumTimeBetweenPings;
> >> private final String[]gatewayTransportFilters; private final
> >> StringhostnameForSenders; private final BooleanifNotExists; public
> >> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean
> >> ifNotExists) {
> >>   this.manualStart = configuration.isManualStart(); this.startPort =
> >>  configuration.getStartPort() !=null ?
> >> Integer.valueOf(configuration.getStartPort()) :null; this.endPort =
> >>  configuration.getEndPort() !=null ?
> >> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress =
> >> configuration.getBindAddress(); this.socketBufferSize =
> >> configuration.getSocketBufferSize() !=null ?
> >> Integer.valueOf(configuration.getSocketBufferSize()) :null;
> >> this.maximumTimeBetweenPings = configuration.
> getMaximumTimeBetweenPings()
> >> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings())
> :null;
> >> this.gatewayTransportFilters = configuration.
> getGatewayTransportFilter()
> >> !=null ?
> >> configuration.getGatewayTransportFilter().stream().map(DeclarableType::
> getClassName)
> >>  .toArray(String[]::new)
> >>  :null; this.hostnameForSenders =
> >> configuration.getHostnameForSenders(); this.ifNotExists = ifNotExists;
> }
> >> }
> >>
> >> The equivalent Kotlin definition is:
> >>
> >> import org.apache.geode.cache.configuration.CacheConfig
> >> import org.apache.geode.cache.configuration.ClassWithParametersType
> >> import java.io.Serializable
> >>
> >> data class GatewayReceiverFunctionArgs
> >> @JvmOverloads private constructor(val manualStart: Boolean, val
> startPort:
> >> Int?, val endPort: Int?, val bindAddress: String, val socketBufferSize:
> >> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters:
> >> Array, val hostNameForSender: String, val ifNotExists: Boolean)
> :
> >> Serializable{
> >>
> >> constructor(configuration: CacheConfig.GatewayReceiver, ifNotExists:
> >> Boolean) :
> >> this(configuration.isManualStart,
> >> configuration.startPort?.toInt(), configuration.endPort?.toInt(),
> >> configuration.bindAddress, configuration.socketBufferSize?.toInt(),
> >> configuration.maximumTimeBetweenPings?.toInt(),
> >> configuration.gatewayTransportFilter ?.map { classWithParametersType:
> >> ClassWithParametersType-> classWithParametersType.className }
> >> ?.toTypedArray()
> >> ?:emptyArray(), configuration.hostnameForSenders,
> >> ifNotExists)
> >>
> >>
> >> companion object {
> >> @JvmStatic val serialVersionUID = -5158224572470173267L }
> >> }
> >>
> >>
> >> On 11/8/18 12:02, Aditya Anchuri wrote:
> >>> I've only touched a few classes in my PR, but I feel like there's a lot
> >>> more boilerplate floating around that can be removed. Having said
> that, I
> >>> agree with your point regarding 

Re: Lombok

[Anthony Baker]

I was talking with Dale and he pointed me to this discussion:
https://github.com/jhipster/generator-jhipster/issues/398 


I think it probably warrants more investigation (e.g. do the issues decried on 
the the internet still exist?) before we adopt this.

Anthony


> On Nov 9, 2018, at 9:00 AM, Jinmei Liao  wrote:
> 
> So users who wish to download our code will need to read some documentation
> on how to setup IDEA/Eclipse before they can compile. It's not a simple
> "import and work with default IDEA setup" now.
> 
> +0 on this. Personally I would prefer to bear with the extra getter/setters
> than giving new contributors more headache at startup.
> 
> On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer  wrote:
> 
>> As an informative comparison on conciseness offering same functionality:
>> 
>> Java Code:
>> 
>> import java.io.Serializable; import lombok.Getter; import
>> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver; import
>> org.apache.geode.cache.configuration.DeclarableType; /** * This class
>> stores the arguments provided in the create
>> gateway-receiver command. */ @Getter public class
>> GatewayReceiverFunctionArgsimplements Serializable {
>>private static final long serialVersionUID = -5158224572470173267L;
>> private final BooleanmanualStart; private final IntegerstartPort; private
>> final IntegerendPort; private final StringbindAddress; private final
>> IntegersocketBufferSize; private final IntegermaximumTimeBetweenPings;
>> private final String[]gatewayTransportFilters; private final
>> StringhostnameForSenders; private final BooleanifNotExists; public
>> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean
>> ifNotExists) {
>>   this.manualStart = configuration.isManualStart(); this.startPort =
>>  configuration.getStartPort() !=null ?
>> Integer.valueOf(configuration.getStartPort()) :null; this.endPort =
>>  configuration.getEndPort() !=null ?
>> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress =
>> configuration.getBindAddress(); this.socketBufferSize =
>> configuration.getSocketBufferSize() !=null ?
>> Integer.valueOf(configuration.getSocketBufferSize()) :null;
>> this.maximumTimeBetweenPings = configuration.getMaximumTimeBetweenPings()
>> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings()) :null;
>> this.gatewayTransportFilters = configuration.getGatewayTransportFilter()
>> !=null ?
>> configuration.getGatewayTransportFilter().stream().map(DeclarableType::getClassName)
>>  .toArray(String[]::new)
>>  :null; this.hostnameForSenders =
>> configuration.getHostnameForSenders(); this.ifNotExists = ifNotExists; }
>> }
>> 
>> The equivalent Kotlin definition is:
>> 
>> import org.apache.geode.cache.configuration.CacheConfig
>> import org.apache.geode.cache.configuration.ClassWithParametersType
>> import java.io.Serializable
>> 
>> data class GatewayReceiverFunctionArgs
>> @JvmOverloads private constructor(val manualStart: Boolean, val startPort:
>> Int?, val endPort: Int?, val bindAddress: String, val socketBufferSize:
>> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters:
>> Array, val hostNameForSender: String, val ifNotExists: Boolean) :
>> Serializable{
>> 
>> constructor(configuration: CacheConfig.GatewayReceiver, ifNotExists:
>> Boolean) :
>> this(configuration.isManualStart,
>> configuration.startPort?.toInt(), configuration.endPort?.toInt(),
>> configuration.bindAddress, configuration.socketBufferSize?.toInt(),
>> configuration.maximumTimeBetweenPings?.toInt(),
>> configuration.gatewayTransportFilter ?.map { classWithParametersType:
>> ClassWithParametersType-> classWithParametersType.className }
>> ?.toTypedArray()
>> ?:emptyArray(), configuration.hostnameForSenders,
>> ifNotExists)
>> 
>> 
>> companion object {
>> @JvmStatic val serialVersionUID = -5158224572470173267L }
>> }
>> 
>> 
>> On 11/8/18 12:02, Aditya Anchuri wrote:
>>> I've only touched a few classes in my PR, but I feel like there's a lot
>>> more boilerplate floating around that can be removed. Having said that, I
>>> agree with your point regarding Kotlin, but for the Java code I would
>> find
>>> Lombok pretty useful. Have included a link to the PR:
>>> 
>>> https://github.com/apache/geode/pull/2815
>>> 
>>> -Aditya
>>> 
>>> 
>>> 
>>> On Thu, Nov 8, 2018 at 11:24 AM Udo Kohlmeyer  wrote:
>>> 
 The Spring world/community are heavy users of Lombok.
 
 In essence it is "nice", BUT it does now add a new dependency on a
 library that is to provide functionality that developers should provide.
 IJ Idea does provide support for Lombok.
 
 I have not yet seen any code bloat that Lombok could reduce for us.
 Also, the reduction is only in terms of "visible", the compiled class
 might be more verbose.
 
 Kotlin on the other hand, as some of the boilerplate code 

Re: Lombok

[Jinmei Liao]

So users who wish to download our code will need to read some documentation
on how to setup IDEA/Eclipse before they can compile. It's not a simple
"import and work with default IDEA setup" now.

+0 on this. Personally I would prefer to bear with the extra getter/setters
than giving new contributors more headache at startup.

On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer  wrote:

> As an informative comparison on conciseness offering same functionality:
>
> Java Code:
>
> import java.io.Serializable; import lombok.Getter; import
> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver; import
> org.apache.geode.cache.configuration.DeclarableType; /** * This class
> stores the arguments provided in the create
> gateway-receiver command. */ @Getter public class
> GatewayReceiverFunctionArgsimplements Serializable {
> private static final long serialVersionUID = -5158224572470173267L;
> private final BooleanmanualStart; private final IntegerstartPort; private
> final IntegerendPort; private final StringbindAddress; private final
> IntegersocketBufferSize; private final IntegermaximumTimeBetweenPings;
> private final String[]gatewayTransportFilters; private final
> StringhostnameForSenders; private final BooleanifNotExists; public
> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean
> ifNotExists) {
>this.manualStart = configuration.isManualStart(); this.startPort =
>   configuration.getStartPort() !=null ?
> Integer.valueOf(configuration.getStartPort()) :null; this.endPort =
>   configuration.getEndPort() !=null ?
> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress =
> configuration.getBindAddress(); this.socketBufferSize =
> configuration.getSocketBufferSize() !=null ?
> Integer.valueOf(configuration.getSocketBufferSize()) :null;
> this.maximumTimeBetweenPings = configuration.getMaximumTimeBetweenPings()
> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings()) :null;
> this.gatewayTransportFilters = configuration.getGatewayTransportFilter()
> !=null ?
> configuration.getGatewayTransportFilter().stream().map(DeclarableType::getClassName)
>   .toArray(String[]::new)
>   :null; this.hostnameForSenders =
> configuration.getHostnameForSenders(); this.ifNotExists = ifNotExists; }
> }
>
> The equivalent Kotlin definition is:
>
> import org.apache.geode.cache.configuration.CacheConfig
> import org.apache.geode.cache.configuration.ClassWithParametersType
> import java.io.Serializable
>
> data class GatewayReceiverFunctionArgs
> @JvmOverloads private constructor(val manualStart: Boolean, val startPort:
> Int?, val endPort: Int?, val bindAddress: String, val socketBufferSize:
> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters:
> Array, val hostNameForSender: String, val ifNotExists: Boolean) :
> Serializable{
>
>  constructor(configuration: CacheConfig.GatewayReceiver, ifNotExists:
> Boolean) :
>  this(configuration.isManualStart,
> configuration.startPort?.toInt(), configuration.endPort?.toInt(),
> configuration.bindAddress, configuration.socketBufferSize?.toInt(),
> configuration.maximumTimeBetweenPings?.toInt(),
> configuration.gatewayTransportFilter ?.map { classWithParametersType:
> ClassWithParametersType-> classWithParametersType.className }
> ?.toTypedArray()
>  ?:emptyArray(), configuration.hostnameForSenders,
> ifNotExists)
>
>
>  companion object {
>  @JvmStatic val serialVersionUID = -5158224572470173267L }
> }
>
>
> On 11/8/18 12:02, Aditya Anchuri wrote:
> > I've only touched a few classes in my PR, but I feel like there's a lot
> > more boilerplate floating around that can be removed. Having said that, I
> > agree with your point regarding Kotlin, but for the Java code I would
> find
> > Lombok pretty useful. Have included a link to the PR:
> >
> > https://github.com/apache/geode/pull/2815
> >
> > -Aditya
> >
> >
> >
> > On Thu, Nov 8, 2018 at 11:24 AM Udo Kohlmeyer  wrote:
> >
> >> The Spring world/community are heavy users of Lombok.
> >>
> >> In essence it is "nice", BUT it does now add a new dependency on a
> >> library that is to provide functionality that developers should provide.
> >> IJ Idea does provide support for Lombok.
> >>
> >> I have not yet seen any code bloat that Lombok could reduce for us.
> >> Also, the reduction is only in terms of "visible", the compiled class
> >> might be more verbose.
> >>
> >> Kotlin on the other hand, as some of the boilerplate code built in as a
> >> language feature. I prefer that over choosing a library, that might have
> >> compatibility issues in the future.
> >>
> >> Also, Kotlin's conciseness is also a language feature rather than
> >> library plugin. I've also seen cases where compiled Java was larger than
> >> the equivalent compiled Kotlin code.
> >>
> >> --Udo
> >>
> >>
> >> On 11/8/18 10:31, Aditya Anchuri wrote:
> >>> Hi everyone,
> >>>
> >>> I am considering adding Lombok as a 

Re: [DISCUSS] Cutting 1.8 release branch

[Jason Huynh]

I also think that the PR https://github.com/apache/geode/pull/2818, or
something that fixes this race, should make it into the release

On Fri, Nov 9, 2018 at 8:59 AM Jason Huynh  wrote:

> I just merged a change last night for GEODE-5884 that I think should make
> it into the release.
>
> On Thu, Nov 8, 2018 at 10:33 AM Anthony Baker  wrote:
>
>> I’m working on a review of LICENSE and NOTICE.  Looks like a few things
>> slipped in that need to be declared.
>>
>> Anthony
>>
>>
>> > On Nov 2, 2018, at 12:42 PM, Bruce Schuchardt 
>> wrote:
>> >
>> > Fixes for PRClientServerRegionFunctionExecutionDUnitTest and
>> CreateAsyncEventQueueCommandDUnitTest have been pushed to develop.
>> >
>> >
>> > On 11/2/18 11:05 AM, Alexander Murmann wrote:
>> >> Hi Ryan,
>> >>
>> >> I am currently waiting for the failing DUnit tests to pass and then
>> plan to
>> >> cut the release branch. Does it work for you if the fixes for
>> GEODE-5972
>> >> would be merged to the branch after it has been cut?
>> >>
>> >> On Fri, Nov 2, 2018 at 9:57 AM Ryan McMahon 
>> wrote:
>> >>
>> >>> Bill Burcham and I have been working on a data inconsistency issue
>> which
>> >>> involves a lost event across WAN sites during rebalance on the
>> originating
>> >>> site.  We are currently performing root cause analysis.  Below is a
>> Geode
>> >>> ticket which we will update with more details as we learn more.
>> >>>
>> >>> https://issues.apache.org/jira/browse/GEODE-5972
>> >>>
>> >>> On Thu, Nov 1, 2018 at 5:23 PM Ernest Burghardt <
>> eburgha...@pivotal.io>
>> >>> wrote:
>> >>>
>>  and PR 390 has been approved and merged
>> 
>>  On Thu, Nov 1, 2018 at 5:10 PM Ernest Burghardt <
>> eburgha...@pivotal.io>
>>  wrote:
>> 
>> > geode-native fixes are in
>>  https://github.com/apache/geode-native/pull/390
>> > On Thu, Nov 1, 2018 at 4:06 PM Anthony Baker 
>> >>> wrote:
>> >> The geode-native source headers I mentioned in [1] need to be
>> cleaned
>>  up.
>> >> Anthony
>> >>
>> >> [1]
>> >>
>> >>>
>> https://lists.apache.org/thread.html/8c9da19d7c0ef0149b1ed79bf0cecde38f17a854ecfa0f0a42f1ff0b@%3Cdev.geode.apache.org%3E
>> >>> On Nov 1, 2018, at 2:01 PM, Bruce Schuchardt <
>> >>> bschucha...@pivotal.io>
>> >> wrote:
>> >>> This PR has been merged to develop
>> >>>
>> >>>
>> >>> On 11/1/18 1:35 PM, Bruce Schuchardt wrote:
>>  I would like to get this PR in the release:
>> >> https://github.com/apache/geode/pull/2757
>>  I'm testing the merge to develop now
>> 
>> 
>>  On 11/1/18 1:18 PM, Sai Boorlagadda wrote:
>> > Sure! agree that we should hold the releases unless there is a
>> > critical issue.
>> >
>> > This is not a gating issue and the code is already committed to
>> >> develop.
>> > Sai
>> >
>> > On Thu, Nov 1, 2018 at 1:03 PM Alexander Murmann <
>>  amurm...@pivotal.io
>> > wrote:
>> >
>> >> In the spirit of the previously discussed timed releases, we
>> >>> should
>> >> only
>> >> hold cutting the release for critical issues, that for some
>> >>> reason
>> >> (not
>> >> sure how that might be) should not be fixed after the branch
>> has
>> >> been cut.
>> >> Waiting for features leads us down the slippery slope we are
>> >>> trying
>> >> to
>> >> avoid by having timed releases. Does that make sense?
>> >>
>> >> On Thu, Nov 1, 2018 at 12:45 PM Sai Boorlagadda <
>> >> sai.boorlaga...@gmail.com
>> >> wrote:
>> >>
>> >>> I would like to resolve GEODE-5338 as it is currently waiting
>> >>> for
>> >>> doc update.
>> >>>
>> >>> Sai
>> >>>
>> >>> On Thu, Nov 1, 2018 at 10:00 AM Alexander Murmann <
>> >> amurm...@pivotal.io>
>> >>> wrote:
>> >>>
>>  Hi everyone,
>> 
>>  It's time to cut the release branch, since we are moving to
>> >>> time
>> >> based
>>  releases. Are there any reasons why a release branch should
>> not
>>  be
>> >> cut
>> >> as
>>  soon as possible?
>> 
>> >>
>> >
>>
>>

Re: [DISCUSS] Cutting 1.8 release branch

[Jason Huynh]

I just merged a change last night for GEODE-5884 that I think should make
it into the release.

On Thu, Nov 8, 2018 at 10:33 AM Anthony Baker  wrote:

> I’m working on a review of LICENSE and NOTICE.  Looks like a few things
> slipped in that need to be declared.
>
> Anthony
>
>
> > On Nov 2, 2018, at 12:42 PM, Bruce Schuchardt 
> wrote:
> >
> > Fixes for PRClientServerRegionFunctionExecutionDUnitTest and
> CreateAsyncEventQueueCommandDUnitTest have been pushed to develop.
> >
> >
> > On 11/2/18 11:05 AM, Alexander Murmann wrote:
> >> Hi Ryan,
> >>
> >> I am currently waiting for the failing DUnit tests to pass and then
> plan to
> >> cut the release branch. Does it work for you if the fixes for GEODE-5972
> >> would be merged to the branch after it has been cut?
> >>
> >> On Fri, Nov 2, 2018 at 9:57 AM Ryan McMahon 
> wrote:
> >>
> >>> Bill Burcham and I have been working on a data inconsistency issue
> which
> >>> involves a lost event across WAN sites during rebalance on the
> originating
> >>> site.  We are currently performing root cause analysis.  Below is a
> Geode
> >>> ticket which we will update with more details as we learn more.
> >>>
> >>> https://issues.apache.org/jira/browse/GEODE-5972
> >>>
> >>> On Thu, Nov 1, 2018 at 5:23 PM Ernest Burghardt  >
> >>> wrote:
> >>>
>  and PR 390 has been approved and merged
> 
>  On Thu, Nov 1, 2018 at 5:10 PM Ernest Burghardt <
> eburgha...@pivotal.io>
>  wrote:
> 
> > geode-native fixes are in
>  https://github.com/apache/geode-native/pull/390
> > On Thu, Nov 1, 2018 at 4:06 PM Anthony Baker 
> >>> wrote:
> >> The geode-native source headers I mentioned in [1] need to be
> cleaned
>  up.
> >> Anthony
> >>
> >> [1]
> >>
> >>>
> https://lists.apache.org/thread.html/8c9da19d7c0ef0149b1ed79bf0cecde38f17a854ecfa0f0a42f1ff0b@%3Cdev.geode.apache.org%3E
> >>> On Nov 1, 2018, at 2:01 PM, Bruce Schuchardt <
> >>> bschucha...@pivotal.io>
> >> wrote:
> >>> This PR has been merged to develop
> >>>
> >>>
> >>> On 11/1/18 1:35 PM, Bruce Schuchardt wrote:
>  I would like to get this PR in the release:
> >> https://github.com/apache/geode/pull/2757
>  I'm testing the merge to develop now
> 
> 
>  On 11/1/18 1:18 PM, Sai Boorlagadda wrote:
> > Sure! agree that we should hold the releases unless there is a
> > critical issue.
> >
> > This is not a gating issue and the code is already committed to
> >> develop.
> > Sai
> >
> > On Thu, Nov 1, 2018 at 1:03 PM Alexander Murmann <
>  amurm...@pivotal.io
> > wrote:
> >
> >> In the spirit of the previously discussed timed releases, we
> >>> should
> >> only
> >> hold cutting the release for critical issues, that for some
> >>> reason
> >> (not
> >> sure how that might be) should not be fixed after the branch has
> >> been cut.
> >> Waiting for features leads us down the slippery slope we are
> >>> trying
> >> to
> >> avoid by having timed releases. Does that make sense?
> >>
> >> On Thu, Nov 1, 2018 at 12:45 PM Sai Boorlagadda <
> >> sai.boorlaga...@gmail.com
> >> wrote:
> >>
> >>> I would like to resolve GEODE-5338 as it is currently waiting
> >>> for
> >>> doc update.
> >>>
> >>> Sai
> >>>
> >>> On Thu, Nov 1, 2018 at 10:00 AM Alexander Murmann <
> >> amurm...@pivotal.io>
> >>> wrote:
> >>>
>  Hi everyone,
> 
>  It's time to cut the release branch, since we are moving to
> >>> time
> >> based
>  releases. Are there any reasons why a release branch should
> not
>  be
> >> cut
> >> as
>  soon as possible?
> 
> >>
> >
>
>

Re: Build Jobs not accessible?

[Juan José Ramos]

Thanks Jake,
Cheers.


On Fri, Nov 9, 2018 at 3:14 PM Jacob Barrett  wrote:

> I bet that after we regenerated it yesterday it isn’t public anymore. I’ll
> fix that shortly.
>
> -Jake
>
>
> > On Nov 9, 2018, at 6:59 AM, Juan José Ramos  wrote:
> >
> > Hello Jens,
> >
> > Yes, I've tried from different browsers after deleting all cookies,
> history
> > and data, same result.
> > I can access https://concourse.apachegeode-ci.info without any problems,
> > but I only see *apache-develop-main*, *apache-develop-metrics*,
> > *apache-develop-examples* and *apache-develop-image* pipelines. There
> used
> > to be another one called *apache-develop-pr*, right?, I recall seeing it
> > recently.
> > Best regards.
> >
> >
> >> On Fri, Nov 9, 2018 at 2:19 PM Jens Deppe  wrote:
> >>
> >> Hi Juan.
> >>
> >> It could be a cookie problem - have you tried clearing them?
> >>
> >> Can you access the top-level https://concourse.apachegeode-ci.info? I
> can
> >> access your build just fine.
> >>
> >> --Jens
> >>
> >>> On Fri, Nov 9, 2018 at 5:52 AM Ju@N  wrote:
> >>>
> >>> Hello team,
> >>>
> >>> I've submitted https://github.com/apache/geode/pull/2822 a couple of
> >>> hours ago and the concourse-ci/IntegrationTest failed, even when
> >> *./gradlew
> >>> integrationTest* runs fine locally.
> >>> I've tried to access the failed build (
> >>> https://concourse.apachegeode-ci.info/builds/14585) to see the output
> >> and
> >>> check what's happening but, after logging in, I only see the empty page
> >>> with  the *loading* message:
> >>>
> >>> [image: Screen Shot 2018-11-09 at 1.42.17 PM.png]
> >>>
> >>> I've also tried to access other failures from other *pull requests*
> (not
> >>> mine) and the result is exactly the same. Is everyone having the same
> >>> issue?, is it just my user?.
> >>> Best regards.
> >>>
> >>> --
> >>> Ju@N
> >>>
> >>
> >
> >
> > --
> > Juan José Ramos Cassella
> > Senior Technical Support Engineer
> > Email: jra...@pivotal.io
> > Office#: +353 21 4238611
> > Mobile#: +353 87 2074066
> > After Hours Contact#: +1 877 477 2269
> > Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
> > How to upload artifacts:
> > https://support.pivotal.io/hc/en-us/articles/204369073
> > How to escalate a ticket:
> > https://support.pivotal.io/hc/en-us/articles/203809556
> >
> > [image: support]  [image: twitter]
> >  [image: linkedin]
> >  [image: facebook]
> >  [image: google plus]
> >  [image: youtube]
> > <
> https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl>
>


-- 
Juan José Ramos Cassella
Senior Technical Support Engineer
Email: jra...@pivotal.io
Office#: +353 21 4238611
Mobile#: +353 87 2074066
After Hours Contact#: +1 877 477 2269
Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
How to upload artifacts:
https://support.pivotal.io/hc/en-us/articles/204369073
How to escalate a ticket:
https://support.pivotal.io/hc/en-us/articles/203809556

[image: support]  [image: twitter]
 [image: linkedin]
 [image: facebook]
 [image: google plus]
 [image: youtube]

Re: Build Jobs not accessible?

[Jacob Barrett]

I bet that after we regenerated it yesterday it isn’t public anymore. I’ll fix 
that shortly.

-Jake


> On Nov 9, 2018, at 6:59 AM, Juan José Ramos  wrote:
> 
> Hello Jens,
> 
> Yes, I've tried from different browsers after deleting all cookies, history
> and data, same result.
> I can access https://concourse.apachegeode-ci.info without any problems,
> but I only see *apache-develop-main*, *apache-develop-metrics*,
> *apache-develop-examples* and *apache-develop-image* pipelines. There used
> to be another one called *apache-develop-pr*, right?, I recall seeing it
> recently.
> Best regards.
> 
> 
>> On Fri, Nov 9, 2018 at 2:19 PM Jens Deppe  wrote:
>> 
>> Hi Juan.
>> 
>> It could be a cookie problem - have you tried clearing them?
>> 
>> Can you access the top-level https://concourse.apachegeode-ci.info? I can
>> access your build just fine.
>> 
>> --Jens
>> 
>>> On Fri, Nov 9, 2018 at 5:52 AM Ju@N  wrote:
>>> 
>>> Hello team,
>>> 
>>> I've submitted https://github.com/apache/geode/pull/2822 a couple of
>>> hours ago and the concourse-ci/IntegrationTest failed, even when
>> *./gradlew
>>> integrationTest* runs fine locally.
>>> I've tried to access the failed build (
>>> https://concourse.apachegeode-ci.info/builds/14585) to see the output
>> and
>>> check what's happening but, after logging in, I only see the empty page
>>> with  the *loading* message:
>>> 
>>> [image: Screen Shot 2018-11-09 at 1.42.17 PM.png]
>>> 
>>> I've also tried to access other failures from other *pull requests* (not
>>> mine) and the result is exactly the same. Is everyone having the same
>>> issue?, is it just my user?.
>>> Best regards.
>>> 
>>> --
>>> Ju@N
>>> 
>> 
> 
> 
> -- 
> Juan José Ramos Cassella
> Senior Technical Support Engineer
> Email: jra...@pivotal.io
> Office#: +353 21 4238611
> Mobile#: +353 87 2074066
> After Hours Contact#: +1 877 477 2269
> Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
> How to upload artifacts:
> https://support.pivotal.io/hc/en-us/articles/204369073
> How to escalate a ticket:
> https://support.pivotal.io/hc/en-us/articles/203809556
> 
> [image: support]  [image: twitter]
>  [image: linkedin]
>  [image: facebook]
>  [image: google plus]
>  [image: youtube]
> 

Re: Build Jobs not accessible?

[Juan José Ramos]

Hello Jens,

Yes, I've tried from different browsers after deleting all cookies, history
and data, same result.
I can access https://concourse.apachegeode-ci.info without any problems,
but I only see *apache-develop-main*, *apache-develop-metrics*,
*apache-develop-examples* and *apache-develop-image* pipelines. There used
to be another one called *apache-develop-pr*, right?, I recall seeing it
recently.
Best regards.


On Fri, Nov 9, 2018 at 2:19 PM Jens Deppe  wrote:

> Hi Juan.
>
> It could be a cookie problem - have you tried clearing them?
>
> Can you access the top-level https://concourse.apachegeode-ci.info? I can
> access your build just fine.
>
> --Jens
>
> On Fri, Nov 9, 2018 at 5:52 AM Ju@N  wrote:
>
> > Hello team,
> >
> > I've submitted https://github.com/apache/geode/pull/2822 a couple of
> > hours ago and the concourse-ci/IntegrationTest failed, even when
> *./gradlew
> > integrationTest* runs fine locally.
> > I've tried to access the failed build (
> > https://concourse.apachegeode-ci.info/builds/14585) to see the output
> and
> > check what's happening but, after logging in, I only see the empty page
> > with  the *loading* message:
> >
> > [image: Screen Shot 2018-11-09 at 1.42.17 PM.png]
> >
> > I've also tried to access other failures from other *pull requests* (not
> > mine) and the result is exactly the same. Is everyone having the same
> > issue?, is it just my user?.
> > Best regards.
> >
> > --
> > Ju@N
> >
>


-- 
Juan José Ramos Cassella
Senior Technical Support Engineer
Email: jra...@pivotal.io
Office#: +353 21 4238611
Mobile#: +353 87 2074066
After Hours Contact#: +1 877 477 2269
Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
How to upload artifacts:
https://support.pivotal.io/hc/en-us/articles/204369073
How to escalate a ticket:
https://support.pivotal.io/hc/en-us/articles/203809556

[image: support]  [image: twitter]
 [image: linkedin]
 [image: facebook]
 [image: google plus]
 [image: youtube]

Re: Build Jobs not accessible?

[Jens Deppe]

Hi Juan.

It could be a cookie problem - have you tried clearing them?

Can you access the top-level https://concourse.apachegeode-ci.info? I can
access your build just fine.

--Jens

On Fri, Nov 9, 2018 at 5:52 AM Ju@N  wrote:

> Hello team,
>
> I've submitted https://github.com/apache/geode/pull/2822 a couple of
> hours ago and the concourse-ci/IntegrationTest failed, even when *./gradlew
> integrationTest* runs fine locally.
> I've tried to access the failed build (
> https://concourse.apachegeode-ci.info/builds/14585) to see the output and
> check what's happening but, after logging in, I only see the empty page
> with  the *loading* message:
>
> [image: Screen Shot 2018-11-09 at 1.42.17 PM.png]
>
> I've also tried to access other failures from other *pull requests* (not
> mine) and the result is exactly the same. Is everyone having the same
> issue?, is it just my user?.
> Best regards.
>
> --
> Ju@N
>

Build Jobs not accessible?

[Ju@N]

Hello team,

I've submitted https://github.com/apache/geode/pull/2822 a couple of hours
ago and the concourse-ci/IntegrationTest failed, even when *./gradlew
integrationTest* runs fine locally.
I've tried to access the failed build (
https://concourse.apachegeode-ci.info/builds/14585) to see the output and
check what's happening but, after logging in, I only see the empty page
with  the *loading* message:

[image: Screen Shot 2018-11-09 at 1.42.17 PM.png]

I've also tried to access other failures from other *pull requests* (not
mine) and the result is exactly the same. Is everyone having the same
issue?, is it just my user?.
Best regards.

-- 
Ju@N