GitHub CodeQL > LGTM

[Robert Houghton]

Geode PR checks for LGTM have been failing since last week due to an analysis 
timeout of 4 hours (prior to the timeouts, we were succeeding in 40-ish 
minutes). To get our checks running again I have enabled the GitHub workflow 
for CodeQL on the Java, JS, Go, and Python languages. Reports have been 
generated for the develop branch and for new PRs to it.  Going forward, I will 
be making a PR to replace the LGTM mandatory check with the CodeQL one. Look 
for that PR for further discussion 

Thanks!
-Robert Houghton.

Re: [PROPOSAL] annul support/1.15

[Robert Houghton]

I would like GEODE-1028 [1] to be considered for support/1.15. It is a 
significant change to the build logic, and I think would be a benefit to 
inclusion in the release branch. PR to develop is available[2]

Thank you,
-Robert Houghton

[1] https://issues.apache.org/jira/browse/GEODE-10283
[2 ]https://github.com/apache/geode/pull/7600

From: Anthony Baker 
Date: Friday, May 6, 2022 at 10:19 AM
To: dev@geode.apache.org 
Subject: Re: [PROPOSAL] annul support/1.15
Owen, with all the recent work I think we are in an excellent position to 
resume work on the 1.15 release. While there are a few thing still outstanding, 
let’s go ahead and recut the release branch as of Monday, 2022-05-09. Would you 
be willing to resume release manager duties?

@Everyone - please chime in if you have in-progress work that you want to ship 
with 1.15 (ideally this is labeled in JIRA with “blocks-1.15.0”).

Thanks,
Anthony


> On Mar 16, 2022, at 2:12 PM, Owen Nichols  wrote:
>
> Seven weeks after cutting support/1.15, Jira now shows 11 blockers, up from 5 
> a few weeks ago.  I wonder if perhaps we cut the release branch prematurely?  
> I propose that we abandon this branch and focus on getting develop closer to 
> what we want to ship, then discuss re-cutting the branch.
>
> If this proposal is approved, I will archive support/1.15 as 
> support/1.15.old, revert develop's numbering to 1.15.0, and bulk-update all 
> Jira tickets fixed in 1.16.0 to fixed in 1.15.0 instead.  Build numbering 
> would start from 1.15.0-build.1000 to easily distinguish pre- and post- recut.
>
> Please vote/discuss with a goal of reaching consensus by 3PM PDT Monday Mar 
> 21.
>
> Thanks,
> -Geode 1.15.0 Release Manager
>
>

Re: Proposal: Cut 1.15 release branch from SHA 8f7193c827ee3198ae374101221c02039c70a561

[Robert Houghton]

A third option, is to cut the branch as normal, and be ready to revert if we 
see some kind of issue as we are stabilizing the redis changes

On Jan 25, 2022 17:10, Alexander Murmann  wrote:
Hi everyone,

Last week we discussed to cut the 1.15 release branch. I would like to propose 
that we cut the branch from last week's SHA 
8f7193c827ee3198ae374101221c02039c70a561. The following commit is a very large 
refactor. Nothing obvious seems wrong with that change, but given that we 
frequently only discover very subtle, but important changes to Geode after a 
long time, I think that this would allow us to reduce some risk for 1.15 and 
its future users and give this large change some time to proof itself on the 
develop branch. I'd love to avoid that work entirely, but am concerned that we 
only might find out about problems a few weeks from now or worse, after we 
shipped.

Another option might be to branch from head and revert the change on the 
release branch. I am uncertain which approach will proof less work.

Re: LGTM as a gating job on PRs not actually gating?

[Robert Houghton]

The test has successfully found a failure! Isn’t that interesting! I’m going to 
have to look more at how we can configure this via .asf.yml…



From: Donal Evans 
Date: Tuesday, January 4, 2022 at 3:20 PM
To: dev@geode.apache.org 
Subject: LGTM as a gating job on PRs not actually gating?
I just noticed something when reviewing a PR. We recently voted to make the 
LGTM check a required precheckin job, but even if that job finds newly 
introduced alerts, it doesn't turn red and block the PR. This seems like it's 
not working as intended.

The comment from LGTM showing that there are newly introduced alerts in the PR: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7219%23issuecomment-1005181341data=04%7C01%7Crhoughton%40vmware.com%7Cd6e65375f2f14cb1ecf808d9cfd8d5dd%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769352482495865%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=ijOnxfdNO7jLa85akx%2BXoiQKteueZOhcfMXbxFN3BKM%3Dreserved=0
The "passing" LGTM job for the PR showing newly introduced alerts: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7219%2Fchecks%3Fcheck_run_id%3D4707178926data=04%7C01%7Crhoughton%40vmware.com%7Cd6e65375f2f14cb1ecf808d9cfd8d5dd%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769352482495865%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=4nfV3tde3CI79tAm8%2BKN3WYNHo8eWWY%2BePNeAeL204M%3Dreserved=0

I'm not sure what the solution to this is, but it certainly seems like the 
intended result wasn't achieved when making the LGTM check a required 
pre-checkin job.

Re: [DISCUSS] Adding LTGM as gating PR checks

[Robert Houghton]

Short answer would be to work with the rest of the community to get the check 
to pass, fix the LGTM configuration, something like that. Otherwise, the 
Concourse CI has the ability to set PR context messages.

-Robert

From: Owen Nichols 
Date: Thursday, December 16, 2021 at 10:40 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding LTGM as gating PR checks
Requiring LGTM looks good to me.  It does not seem to have a high rate of 
false-positives like some other linters, but if we are making it gating, what 
would the process look like to override a false-positive?

On 12/16/21, 10:37 AM, "Anthony Baker"  wrote:

Thanks Robert, I think this is important. I think this is a good first step.

In future I think we should consider adding a CI job to ensure that 
pre-existing security errors are addressed. Perhaps GitHub code scanning is 
worth investigating since they have acquired the LGTM product.

Anthony


> On Dec 16, 2021, at 10:08 AM, Robert Houghton  
wrote:
>
> We have had LGTM tests enabled on Apache Geode PRs for quite some time, 
and have done a great job of trending those warnings and errors to in the right 
direction. I would like to make the change to our GitHub to make those changes 
blocking for all new PRs, given their reliability and lack-of-flakiness.
>
> Does anyone have strong feelings against that?
    >
> -Robert Houghton

Re: [DISCUSS] Adding LTGM as gating PR checks

[Robert Houghton]

Excuse me. I meant to link the PR that would enable this behavior: 
https://github.com/apache/geode/pull/7197

-Robert Houghton

From: Robert Houghton 
Date: Thursday, December 16, 2021 at 10:08 AM
To: geode 
Subject: [DISCUSS] Adding LTGM as gating PR checks
We have had LGTM tests enabled on Apache Geode PRs for quite some time, and 
have done a great job of trending those warnings and errors to in the right 
direction. I would like to make the change to our GitHub to make those changes 
blocking for all new PRs, given their reliability and lack-of-flakiness.

Does anyone have strong feelings against that?

-Robert Houghton

[DISCUSS] Adding LTGM as gating PR checks

[Robert Houghton]

We have had LGTM tests enabled on Apache Geode PRs for quite some time, and 
have done a great job of trending those warnings and errors to in the right 
direction. I would like to make the change to our GitHub to make those changes 
blocking for all new PRs, given their reliability and lack-of-flakiness.

Does anyone have strong feelings against that?

-Robert Houghton

Re: [DISCUSS] batch patch release voting

[Robert Houghton]

I would hope that all RCs had been checked thoroughly before any release VOTE 
occurs.

From: Donal Evans 
Date: Thursday, December 2, 2021 at 11:47 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] batch patch release voting
The thing I wonder about with this decision is what happens if there are no 
problems with two of the releases, but one of them has some showstopping issue 
that needs to be addressed.

Given that not every fix that's going into 1.14.1 is going into 1.12.6, it's 
possible that an unrelated issue with the 1.14 patch release could hold up the 
release of the perfectly fine 1.12 release if the vote is on releasing all 
three. In that case, would there need to be a new RC for the 1.12 and 1.13 
releases, given that the vote for them would technically have failed? Would 
there need to be a new vote on all three releases, or just go ahead with the 
1.12 and 1.13 release and re-vote on the 1.14 once the issue is fixed?

From: Owen Nichols 
Sent: Wednesday, December 1, 2021 11:45 AM
To: dev@geode.apache.org 
Subject: [DISCUSS] batch patch release voting

Geode's N-2 support policy can lead to "waves" of patch releases.  For example, 
1.12.6, 1.13.5 and 1.14.1 are all likely this month, with many of the same 
fixes in each.

Some open source projects group waves of patch releases into a single 
announcement.  It might be nice to do this for Geode.

For voting, does anyone have any preference whether to still hold three 
separate votes, or would it be ok to vote on a batch of patch releases at once? 
 If so, would 3 days still be enough time for the voting deadline?

Re: [DISCUSS] batch patch release voting

[Robert Houghton]

In the case where its one change being backported all over, I say batch up the 
vote.

From: Owen Nichols 
Date: Wednesday, December 1, 2021 at 11:46 AM
To: dev@geode.apache.org 
Subject: [DISCUSS] batch patch release voting
Geode's N-2 support policy can lead to "waves" of patch releases.  For example, 
1.12.6, 1.13.5 and 1.14.1 are all likely this month, with many of the same 
fixes in each.

Some open source projects group waves of patch releases into a single 
announcement.  It might be nice to do this for Geode.

For voting, does anyone have any preference whether to still hold three 
separate votes, or would it be ok to vote on a batch of patch releases at once? 
 If so, would 3 days still be enough time for the voting deadline?

Re: [RFC] Versioning Extension

[Robert Houghton]

Hello Geode Developers,

The expected review data for the RFC[1] on implementing Versioning Extensions 
for Geode has passed. We have implemented feedback from the community into our 
work-in-progress PR, and are ready to submit it for PR[2] review and testing.

Thank you,
-Robert Houghton

[1] https://cwiki.apache.org/confluence/display/GEODE/Versioning+Extension
[2] https://github.com/apache/geode/pull/7074

From: Jacob Barrett 
Date: Wednesday, November 3, 2021 at 12:00 PM
To: dev@geode.apache.org 
Subject: [Suspected Spam] Re: [RFC] Versioning Extension
A revision was made based on feedback. The getDetails method now returns a map.

See updated PR #7074 [1] for POC.

[1] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7074data=04%7C01%7Crhoughton%40vmware.com%7Cb2932c9cb3d844c7ec2408d99efc3a88%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637715628423191919%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=2TIvSklJzfwCS%2Ft7FvzD74ddJZnP%2F1YoMumGFLkaPew%3Dreserved=0


On Nov 2, 2021, at 12:45 PM, Jacob Barrett 
mailto:jabarr...@vmware.com>> wrote:

Devs,

Please review the new RFC for Versioning Extension [1]. For an example please 
find a POC of the RFC as PR #7071 [2].

Thanks,
Jake

[1] https://cwiki.apache.org/confluence/display/GEODE/Versioning+Extension
[2] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7071data=04%7C01%7Crhoughton%40vmware.com%7Cb2932c9cb3d844c7ec2408d99efc3a88%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637715628423201859%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=azMByMSrIjcunGrYK9MrUDQX9TW21JcC3jIeKalHfv8%3Dreserved=0

Re: Apache Geode CI upgrade

[Robert Houghton]

We are upgrading the Apache Geode Concourse deployment. Pipelines will be 
paused as we roll over to the new backend.

Thanks you,
-Robert Houghton

From: Robert Houghton 
Date: Thursday, November 4, 2021 at 3:24 PM
To: geode 
Subject: [Suspected Spam] Apache Geode CI upgrade
Hello Geode Developers,

Next week, Wednesday, 10 November 2021, we are planning to upgrade the Apache 
Geode Concourse deployment from version 6.7.4 to 7.6.0. We will also be moving 
the backend for Concourse from Bosh to k8s. We will deploy all production 
pipelines (Apache repo, develop and support/* branches). Developer or personal 
pipelines will not be deployed. We are not able to bring job history along.

What this means to you:
Please anticipate some downtime in the develop pipelines, but PR checks should 
continue to run as normal. Links from GitHub PRs to specific Concourse jobs in 
the old deployment will break. If this happens to you, change the domain from 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fdata=04%7C01%7Crhoughton%40vmware.com%7C06f5aeffa9714b6815df08d99fe1e9c0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637716614930386785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=oQkhAIq%2FkBTgh6SO6SIkOxBSKL3TlaUResjO1i84ESM%3Dreserved=0
 to 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse-calcite.apachegeode-ci.info%2Fdata=04%7C01%7Crhoughton%40vmware.com%7C06f5aeffa9714b6815df08d99fe1e9c0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637716614930396731%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tgajkCnfpMZbiFn0dQWrZYyvctRWyQfSU6YqGD8vBew%3Dreserved=0
 to reach the broken link. The team is brainstorming on how to not break these 
links in the future.


Thank you,
-Robert Houghton

Re: @TestOnly or @VisibleForTesting

[Robert Houghton]

Rather than change an in-production class scope for testing reasons, I would 
like to see a new pass-through method with required scope and the @TestOnly 
annotation. This leaves the in-use method alone with correct 
encapsulation/visibility, and makes a clear, concise way to access it for test 
that is un-ambiguous as to its intent (testing only, non-production).

As a consumer of Geode, I think the broader @VisibleForTesting is worse, in 
that I no longer know the scope from which I am “supposed” to consume the 
method. Whereas for @TestOnly, I know the answer is “nowhere”

-Robert Houghton

From: Mark Hanson 
Date: Friday, November 5, 2021 at 10:11 AM
To: dev@geode.apache.org 
Subject: Re: @TestOnly or @VisibleForTesting
I generally see @VisibleForTesting used to change permissions of a private 
method, which is inaccessible to an external object, into a public method to 
allow a test to reach in and set or get fields within a class.

I see it as an invaluable bridge between the reality of where we are now with 
our less than perfect codebase and where we want to go. I know a number of 
classes could be rewritten to enable better dependency injection, but I see 
this as a necessary and beneficial stop gap to that end.

With regard to @TestOnly, I have yet to really have a need to use this as 
@VisibleForTesting, generally, seems broader than @TestOnly. If @TestOnly 
causes methods to be optimized out and thus reduces object size, I think there 
can be real benefit there. But I don't see much harm in having a few extra 
methods on some of our main objects which occur infrequently, such as 
DistributedMember or PoolImpl.

Unless there is more substantial benefit just as reduce object size, I would 
suggest that we just use @VisibleForTesting and avoid adding dependencies.

Thanks,
Mark

On 11/5/21, 9:32 AM, "Dale Emery"  wrote:

Kirk and I often use @VisibleForTesting as part of a technique for making 
complex code more testable.

Many classes have “hidden” dependencies that make that class hard to test. 
A useful technique is to add a constructor parameter to allow a test to inject 
a more controllable, more observable instance.

But we don’t want to force every use of the existing constructor to have to 
create that instance. So we leave the old constructor’s signature as is, create 
a new constructor that accepts that new parameter, and make the old constructor 
call the new one.

This new, intermediate constructor is called only by the old constructor 
and by tests. In order for tests to call it, we have to make it visible, so we 
mark it as @VisibleForTesting.

Dale


From: Alexander Murmann 
Date: Thursday, November 4, 2021 at 5:02 PM
To: dev@geode.apache.org 
Subject: Re: @TestOnly or @VisibleForTesting
Another +1 to Eric's point. What are others seeing as valid use cases for 
those annotations?

From: Patrick Johnson 
Sent: Thursday, November 4, 2021 15:55
To: dev@geode.apache.org 
Subject: Re: @TestOnly or @VisibleForTesting

I agree with Eric. Maybe rather than standardizing on one, we should stop 
adding anymore @VisibleForTesting or @TestOnly to the codebase. Possibly 
deprecate @VisibleForTesting.

> On Nov 4, 2021, at 3:30 PM, Eric Zoerner  wrote:
>
> My opinion is that @VisibleForTesting is a code smell and either 
indicates that there is refactoring needed or there are tests that are 
unnecessary. If there is functionality in a private method that needs to be 
tested independently, then that code should be extracted and placed in a 
separate class that has a wider visibility that can be tested on its own.
>
> The same could probably be said for @TestOnly unless there are actually 
static analysis tools that need it for some reason.
>
> From: Kirk Lund 
> Date: Thursday, November 4, 2021 at 15:13
> To: dev@geode.apache.org 
> Subject: Re: @TestOnly or @VisibleForTesting
> As everyone thinks about how we want to use these annotations, please keep
> this in mind that both *@VisibleForTesting* and *@TestOnly* can be used on
> Types (Class/Interface/Enum), Constructors, Methods and Fields. (not just
> Methods)
>
> On Thu, Nov 4, 2021 at 3:09 PM Kirk Lund  wrote:
>
>> Hey all,
>>
>> We're introducing a mess to the codebase. It's a small problem, but
>> several small problems become a big problem and one of my missions is to
>> clean up and improve the codebase.
>>
>> I recently started seeing lots of pull requests with usage of @TestOnly.
>> Sometimes it's used instead of @VisibleForTesting, while sometimes I see
>> both annotations added to the same method.
>>
>> Before we start using @TestOnly, I think we need some guidelines for when
>> to use

Apache Geode CI upgrade

[Robert Houghton]

Hello Geode Developers,

Next week, Wednesday, 10 November 2021, we are planning to upgrade the Apache 
Geode Concourse deployment from version 6.7.4 to 7.6.0. We will also be moving 
the backend for Concourse from Bosh to k8s. We will deploy all production 
pipelines (Apache repo, develop and support/* branches). Developer or personal 
pipelines will not be deployed. We are not able to bring job history along.

What this means to you:
Please anticipate some downtime in the develop pipelines, but PR checks should 
continue to run as normal. Links from GitHub PRs to specific Concourse jobs in 
the old deployment will break. If this happens to you, change the domain from 
https://concourse.apachegeode-ci.info to 
https://concourse-calcite.apachegeode-ci.info to reach the broken link. The 
team is brainstorming on how to not break these links in the future.


Thank you,
-Robert Houghton

Re: Apache Geode PR pipeline

[Robert Houghton]

This should be resolved now. Thanks!

From: Robert Houghton 
Date: Tuesday, October 26, 2021 at 12:07 PM
To: geode 
Subject: Apache Geode PR pipeline
A concourse variable change for the PR pipeline has gotten us into a weird 
state of non-running. We’re working on it.
-Robert

Apache Geode PR pipeline

[Robert Houghton]

A concourse variable change for the PR pipeline has gotten us into a weird 
state of non-running. We’re working on it.
-Robert

Re: Geode-9621: Spotless overstepping its boundary of formatter

[Robert Houghton]

I would not call it a “surge” of them, but I perceived an uptick in them.

From: Udo Kohlmeyer 
Date: Tuesday, October 12, 2021 at 1:00 PM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
I had never explicitly asked this, but what was the main driver for this 
ticket? Was there a surge in explicit version definitions in gradle files? I 
mean we as committers have agreed to using BOM constraints.

--Udo

From: Robert Houghton 
Date: Wednesday, October 13, 2021 at 4:32 AM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
Again, Spotless is not changing code. That was a bad change, as you pointed 
out, and we modified Spotless to throw an exception instead of stripping out 
the version string indiscriminately.

We surely could make Spotless only apply to the main source set. The cost would 
be that tests could use different versions than production. That tests could 
themselves specify multiple versions at once, and that tests would be 
out-of-sync with each other, and with the production code. So I would strongly 
vote against that.

-Robert

From: Udo Kohlmeyer 
Date: Monday, October 11, 2021 at 5:32 PM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
Hi there Robert,

Yes, you did work with me, but the solution to add my custom versions of jars 
that I want to test with, into the `DependencyConstraints` file just feels like 
the wrong approach. The prior art you refer to, is a specific version of  
tomcat we want each module to use. It is no different to specifying the exact 
artifact version we want for production code. My case is that I don’t want my 
artifact version dependencies to leak outside of the test module.

I think, the fact that the decision to elevate spotless from cosmetic formatter 
to actually having a significant impact, was done unilaterally. Please don’t 
get me wrong, I love the fact that we want to fix the problem where developers 
are not following good practices. But I think I am digging in here, because I 
believe we missed an opportunity to get more voices in this decision.

I also firmly believe that spotless is overstepping the “cosmetic” boundary. I 
think many developers would feel upset if their code were to be changed. If 
spotless suddenly replaces a for-loop with a stream implementation.

I think it is one thing to decide if gradle should be formatted with 2 spaces 
vs 4, or when and how lines wrap, and it’s another for spotless to just remove 
version information and force one consistent version even if the version was 
specifically overriden.

I wonder if we can maybe reduce the area of effect of this change and really 
only force the version formatter to activate for the ‘main’ configuration and 
leave every other configuration type (test, distributedTest, integrationTest, 
acceptanceTest, upgradeTest) alone.

--Udo

From: Robert Houghton 
Date: Tuesday, October 12, 2021 at 10:38 AM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
Currently there is no way to exclude certain code blocks from the malicious 
code formatter. Therefore, my tests that require a specific hardcoded version 
of Spring to be used, will always either fail the validation or tests will fail 
due to spotless changing my testing scenario by affecting the version of the 
library that we have to use.


Udo,

I have worked with you to accomplish your goal of not automatically “fixing” 
the inclusion of dependency versions in the build files, as your work case 
wants to do something special there. The formatter now throws when there is a 
naked version-string on the dependency line, as we discussed.

If you want to continue to work around this, there are many cases of prior art 
in Geode, such as extensions/geode-modules-tomcat9/build.gradle, where specific 
versions are allowed.

Good luck.
-Robert Houghton

Re: Geode-9621: Spotless overstepping its boundary of formatter

[Robert Houghton]

Again, Spotless is not changing code. That was a bad change, as you pointed 
out, and we modified Spotless to throw an exception instead of stripping out 
the version string indiscriminately.

We surely could make Spotless only apply to the main source set. The cost would 
be that tests could use different versions than production. That tests could 
themselves specify multiple versions at once, and that tests would be 
out-of-sync with each other, and with the production code. So I would strongly 
vote against that.

-Robert

From: Udo Kohlmeyer 
Date: Monday, October 11, 2021 at 5:32 PM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
Hi there Robert,

Yes, you did work with me, but the solution to add my custom versions of jars 
that I want to test with, into the `DependencyConstraints` file just feels like 
the wrong approach. The prior art you refer to, is a specific version of  
tomcat we want each module to use. It is no different to specifying the exact 
artifact version we want for production code. My case is that I don’t want my 
artifact version dependencies to leak outside of the test module.

I think, the fact that the decision to elevate spotless from cosmetic formatter 
to actually having a significant impact, was done unilaterally. Please don’t 
get me wrong, I love the fact that we want to fix the problem where developers 
are not following good practices. But I think I am digging in here, because I 
believe we missed an opportunity to get more voices in this decision.

I also firmly believe that spotless is overstepping the “cosmetic” boundary. I 
think many developers would feel upset if their code were to be changed. If 
spotless suddenly replaces a for-loop with a stream implementation.

I think it is one thing to decide if gradle should be formatted with 2 spaces 
vs 4, or when and how lines wrap, and it’s another for spotless to just remove 
version information and force one consistent version even if the version was 
specifically overriden.

I wonder if we can maybe reduce the area of effect of this change and really 
only force the version formatter to activate for the ‘main’ configuration and 
leave every other configuration type (test, distributedTest, integrationTest, 
acceptanceTest, upgradeTest) alone.

--Udo

From: Robert Houghton 
Date: Tuesday, October 12, 2021 at 10:38 AM
To: dev@geode.apache.org 
Subject: Re: Geode-9621: Spotless overstepping its boundary of formatter
Currently there is no way to exclude certain code blocks from the malicious 
code formatter. Therefore, my tests that require a specific hardcoded version 
of Spring to be used, will always either fail the validation or tests will fail 
due to spotless changing my testing scenario by affecting the version of the 
library that we have to use.


Udo,

I have worked with you to accomplish your goal of not automatically “fixing” 
the inclusion of dependency versions in the build files, as your work case 
wants to do something special there. The formatter now throws when there is a 
naked version-string on the dependency line, as we discussed.

If you want to continue to work around this, there are many cases of prior art 
in Geode, such as extensions/geode-modules-tomcat9/build.gradle, where specific 
versions are allowed.

Good luck.
-Robert Houghton

Re: Geode-9621: Spotless overstepping its boundary of formatter

[Robert Houghton]

Currently there is no way to exclude certain code blocks from the malicious 
code formatter. Therefore, my tests that require a specific hardcoded version 
of Spring to be used, will always either fail the validation or tests will fail 
due to spotless changing my testing scenario by affecting the version of the 
library that we have to use.


Udo,

I have worked with you to accomplish your goal of not automatically “fixing” 
the inclusion of dependency versions in the build files, as your work case 
wants to do something special there. The formatter now throws when there is a 
naked version-string on the dependency line, as we discussed.

If you want to continue to work around this, there are many cases of prior art 
in Geode, such as extensions/geode-modules-tomcat9/build.gradle, where specific 
versions are allowed.

Good luck.
-Robert Houghton

Re: Access to the CI pipelines using fly

[Robert Houghton]

Sure thing, Udo. I have updated the concourse instance with your github name.

-Robert

From: Udo Kohlmeyer 
Date: Thursday, July 1, 2021 at 1:44 PM
To: geode 
Subject: Access to the CI pipelines using fly
Hi there Apache Geode Infra,

Would it be possible to please grant me access to fly access of a CI pipeline. 
I’m trying to root-cause a problem which I see happen only in the CI-pipeline.

--Udo

Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

[Robert Houghton]

I for one do not like revisionist history (rebase) but understand that linear 
history is easier for bisect.


From: Blake Bender 
Date: Monday, June 28, 2021 at 2:24 PM
To: dev@geode.apache.org 
Subject: RE: Fw: [DISCUSS] Rebase and Squash Options on Github develop
+1, only because I only get one vote.  Merge commits in develop are a scourge, 
IMO, and should be avoided in almost all instances.

Blake


-Original Message-
From: Donal Evans 
Sent: Monday, June 28, 2021 2:22 PM
To: dev@geode.apache.org
Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

I definitely approve of this proposal. I accidentally merged (rather than 
squashed and merged) a PR a while back because my GitHub session had expired 
and refreshing the page after trying to squash and merge resulted in the 
default merge option being used, much to my frustration. I've yet to see any 
explanation of why we would need to merge PRs rather than squash and 
merge/rebase and merge, so removing it as an option just seems like a good idea.

From: Nabarun Nag 
Sent: Monday, June 28, 2021 1:06 PM
To: Owen Nichols ; dev@geode.apache.org 

Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

Just a clarification.

The options I am proposing to be kept in the PRs are:

  *   Squash and merge
  *   Rebase and merge

Regards,
Nabarun

From: Owen Nichols 
Sent: Monday, June 28, 2021 1:03 PM
To: Nabarun Nag ; dev@geode.apache.org 
Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

Sounds like a good idea. I can't find any example of an intentional merge 
commit on develop...but plenty of accidental ones.

Upon releases we do a command line merge commit to the main trunk, which should 
still be fine as this proposal only applies to PRs.

I agree with still allowing rebase commits. I have seen several PRs use them to 
good effect.

You can disable merge commits through .asf.yaml, so changing the setting is as 
easy as opening a PR (and, if we don't like it, reverting is just as easy)


---
Sent from Workspace ONE 
Boxer

On June 28, 2021 at 12:38:50 PM PDT, Nabarun Nag  wrote:
Hi all,

Last year, we had a discussion on rebase and squash merge PRs to the develop 
branch,

  *   to have linear git history
  *   help with bisecting
  *   easier root cause analysis of failures
  *   easier to backport changes

However, many developers have reached out mentioning that sometimes they have 
clicked the merge button in hurry when they meant to rebase/squash. Once 
clicked there is no going back. All of them suggested that to have the GitHub 
setting on develop branch to rebase and squash merge option and hence there is 
no room for mistakes to occur.

I would like to propose to that we set the GitHub setting on develop PR to 
rebase and squash buttons.

Please do note that this is reversible and can be changed back, hence there is 
no harm in trying it out.


Regards
Nabarun Nag



From: Owen Nichols (Pivotal) 
Sent: Thursday, April 9, 2020 11:45 AM
To: dev@geode.apache.org 
Subject: Re: [REQUEST] Squash merge please

I've noticed quite a few PRs in the last week that were merged with "Merge" 
rather than "Squash and Merge".

While the community consensus was to continue to allow all merge options, 
please try to default to "Squash and Merge" whenever you can to keep history as 
linear as possible. GitHub will save the last method you used in a cookie, 
which helps, but then it's easy to miss when it resets itself back to the 
default of "Merge" some months later because you cleared cookies, changed 
browsers, etc.

> On Oct 22, 2019, at 5:12 PM, Nabarun Nag  wrote:
>
> Hi Geode Committers,
>
> A kind request for using squash commit instead of using merge.
> This will really help us in our bisect operations when a regression/flakiness 
> in the product is introduced. We can automate and go through fewer commits 
> faster, avoiding commits like "spotless fix" and "re-trigger precheck-in" or 
> other minor commits in the merged branch.
>
> Also, please use the commit format : (helps us to know who worked on it, what 
> is the history)
> GEODE-: 
>
> * explanation line 1
> * explanation line 2
>
> This is not a rule or anything, but a request to help out your fellow 
> committers in quickly detecting a problem.
>
> For inspiration, we can look into Apache Kafka / Spark where they have a 
> complete linear graph for their main 

Re: [VOTE] Apache Geode 1.13.3.RC2

[Robert Houghton]

+1
I like the build. I verified the output of the RC check jobs, and matched 
against the source SHA we are trying to publish.


From: Owen Nichols 
Date: Wednesday, June 23, 2021 at 11:56 AM
To: dev@geode.apache.org 
Subject: [Suspected Spam] [VOTE] Apache Geode 1.13.3.RC2
Hello Geode Dev Community,

This is a release candidate for Apache Geode version 1.13.3.RC2.
Thanks to all the community members for their contributions to this release!

Please do a review and give your feedback, including the checks you performed.

Expedited voting deadline:
3PM PST Thu, June 24 2021.

Please note that we are voting upon the source tag:
rel/v1.13.3.RC2

Release notes:
https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.13.3

Source and binary distributions:
https://dist.apache.org/repos/dist/dev/geode/1.13.3.RC2/

Maven staging repo:
https://repository.apache.org/content/repositories/orgapachegeode-1086

GitHub:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Ftree%2Frel%2Fv1.13.3.RC2data=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=dfnMLCKjve7vyaMZJyE6aAe70FPU8NT04I05fePqJ9c%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-examples%2Ftree%2Frel%2Fv1.13.3.RC2data=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=uKcpcjIrfEgr7Hfb5qVn28v3MdUNISbzRtENapC0Fak%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Ftree%2Frel%2Fv1.13.3.RC2data=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=dBthKohpFT%2BemN5pxQifpccj%2BdJmgdFgByiatSkOFw4%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-benchmarks%2Ftree%2Frel%2Fv1.13.3.RC2data=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=oHp%2B2QTu3r2P9WSjQxbvS3B8EStFpeidmciANXa9mcw%3Dreserved=0

Pipelines:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fteams%2Fmain%2Fpipelines%2Fapache-support-1-13-maindata=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=UY%2FEWf7l8tfJW%2BU%2BbiD3a%2BqfnDijHIICWQ5MVa6Y6es%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fteams%2Fmain%2Fpipelines%2Fapache-support-1-13-rcdata=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=7Q%2Bj4ZF1zzc0syizXDJ5b1BdKjeAi1PXUnUcwfRnL9c%3Dreserved=0

Geode's KEYS file containing PGP keys we use to sign the release:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2FKEYSdata=04%7C01%7Crhoughton%40vmware.com%7C0292ff4088f3455ba36a08d9367889de%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637600713602836651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=nOBSS%2Fo3Ld5cZlUujvQ%2Fa8NmSifuUE6Jie1xOl1fCnc%3Dreserved=0

Command to run geode-examples:
./gradlew 
-PgeodeReleaseUrl=https://dist.apache.org/repos/dist/dev/geode/1.13.3.RC2 
-PgeodeRepositoryUrl=https://repository.apache.org/content/repositories/orgapachegeode-1086
 build runAll

Regards
Owen Nichols

Re: [FYI] PR checks

[Robert Houghton]

Thanks, Owen. I was sitting down to write this.

For more context:
With the merge of GEODE-9298 to remove concourse deprecations from our 
pipeline, many of the concourse jobs have been re-named. This may cause issues 
with in-flight PRs not correctly updating their status with GitHub, which could 
lead to the merge button not being enabled for even passing PRs. If this 
happens to you,  please either re-trigger the specific job, or push a new 
(empty) commit to your PR branch.

Thank you,
-Robert Houghton

From: Owen Nichols 
Date: Friday, May 28, 2021 at 9:07 AM
To: dev@geode.apache.org 
Subject: [FYI] PR checks
Concourse job names have been changed to lowercase for conformity[1].

If your PR shows required checks with a status of "Expected -- Waiting for 
status to be reported", please push another commit.

[1] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse-ci.org%2Fconfig-basics.html%23schema.identifierdata=04%7C01%7Crhoughton%40vmware.com%7Cf857a3a4f6384aae491b08d921f29eed%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637578148215521004%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=R%2FjtMxdWShzwrMbIjEtEA1zozL1XTtwRzDabHS2m3PM%3Dreserved=0

Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

[Robert Houghton]

80 characters also feels arbitrary, especially with auto-formatters 
(clang-tidy?) of mangling some otherwise very-readable code.

From: Blake Bender 
Date: Monday, May 3, 2021 at 11:23 AM
To: dev@geode.apache.org 
Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide
My $0.02 on these:

Things I'd like to see us conform to Google style on:
* I'd be happy to move to C++ 17
* Would also be happy to remove forward declarations.  "I'm not a critic, but I 
know what I hate," as it were, and I hate forward declarations.
* I would also be happy with an 80-character line limit, though I don't feel 
strongly about it.  100 may be consistent with Geode, but it still feels 
arbitrary to me.
* I would be very pleased to remove all the macros from our code.  I've been 
bitten more than once in the past while debugging or refactoring our code, 
because of ill-formed macros.

Google things I disagree with:
* I don't like exceptions, but I don't even want to think about the amount of 
effort required to remove them from the codebase is, IMO, unreasonably high.  
Keep the exceptions, most of the time they're used pretty judiciously.
* I really, really, *really* (really?  Yes, really!) hate anything resembling 
Hungarian prefix notation, and have permanent scars from decades of reading it 
in Windows code.  Please don't ask me to put a random 'k' in from of my enums - 
ick.

One other note: in the past, we've had conversations about "style only" pull 
requests to fix some of these things, and the guidance we ended up with has 
been to only fix this sort of thing while you're in the code working on a fix 
or a feature.  I, for one, would welcome some PRs that just, say, renamed a ton 
of member variables to replace "m_" prefix with a simple trailing "_", perhaps 
fixed some of the more egregious and weird abbreviations, etc.  My preference 
for bug fixes and feature work is that all of the code changes be focused on 
stuff that's relevant to the fix/feature, and mixing it with random style guide 
refactoring, I feel, muddies the waters for future maintainers.

Thanks,

Blake

-Original Message-
From: Jacob Barrett 
Sent: Saturday, May 1, 2021 9:21 AM
To: dev@geode.apache.org
Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

Great call outs!

> On May 1, 2021, at 7:57 AM, Mario Salazar de Torres 
>  wrote:
>
>  1.  Member variables names as of Google style guide requires a '_' char to 
> be added at the end so it can be identified. Should we also adopt that?
> For example, imagine you have a region mutex, so, should we name it as 
> 'regionMutex_' ?

I didn’t mention this one out in my review of differences because we are 
following it but I suppose with the combination of the camelCase difference we 
should probably call it out more specifically. Perhaps in our documentation we 
should show examples of both local and member variables. Do you think that will 
make it more clear?

>  2.  Also, I would like to point out that macros are dis-recommended but 
> every C++ committee member I know.
> What do you think about adding a notice saying: "Macros should be avoided and 
> only used when there is no alternative”?

I think that is called out in various ways in a few places in the Google guide 
but I am more than happy for us to include strong or clearer language around 
this. Between constexpr and templates there are very cases for macros anymore.
We mostly use macros only to handle non-standard attributes. When we move to 
C++17 a lot of these will go away.

Thanks,
Jake

Re: [VOTE] Requiring final keyword on every parameter and local variable

[Robert Houghton]

Does this really "add noise" instead of "add clarity and precision" ?

-Robert

On 4/14/21, 4:25 PM, "Jason Huynh"  wrote:

Just to confirm for myself... final doesn't necessarily make object 
immutable.. the reference is immutable but if it refers to say a list, the list 
is still mutable.

So if I see a final keyword, I should only expect the reference not to 
change, but it won't guarantee that the object isn't changing.

I'm a -1 on forcing the issue because I believe we currently have too many 
"edge cases" where we can't stick a final on.  So we end up adding complexity 
in reading without the benefit guaranteeing anything...  I'd dislike seeing 
something like this make it partway and then get abandoned...

Also are we only discussing local variables, class variables and method 
parameters?  Ignoring final on methods and classes?


On 4/14/21, 2:18 PM, "Mark Hanson"  wrote:

+1 To Kirk’s approach.
-1 To final everywhere.

I support Kirk’s approach of adding final where there is benefit. If we 
want to go further, we can have intellij find all of them and change them if we 
want. Final is not always a developer’s explicit statement. Sometimes something 
becomes final after the fact.

If we manually touch any code, I would much rather we deal with the use 
of raw types than have final everywhere. This strikes me as a minor issue 
compared to the rampant use of raw types.

E.g.
RegionFactory regionFactory = 
getCache().createRegionFactory(RegionShortcut.PARTITION_REDUNDANT);
Region region = regionFactory.create (“testRegion”);
Region.put(int, String);`

Thanks,
Mark

From: Bill Burcham 
Date: Wednesday, April 14, 2021 at 1:53 PM
To: dev@geode.apache.org 
Subject: Re: [VOTE] Requiring final keyword on every parameter and 
local variable
+1

I am 100% for putting final everywhere it is possible to put it. Call 
this
the "hard final" position.

What I've been advocating for, intermittently, in PRs (for example, in 
PR
#6310 
)
 is
more of a "soft final" position which I can formulate as:

The final keyword is valuable everywhere. When we encounter it in code, 
we
assume that the author put it there intentionally to communicate that 
the
variable will not change. Since the code compiled before our change, we
know that the variable does not change (the compiler has proven it!) 
When
modifying code, we do not remove the final keyword unless we have to (to
get the code to compile).

So that's the "soft final" position. But I'm all for Kirk's "hard 
final" if
others agree. I just don't see any downside—only upside on this one.

The final keyword is a vehicle, like types, for programmers to 
communicate
their intentions. In both cases, the compiler can provide valuable
validation. There is, unfortunately, some visual noise entailed (lots of
occurrences of "final" in the code). But this makes non-final (mutable)
variables much more apparent. Since they are (or should be) in the
minority, this is an aid to maintainers. This becomes apparent in 
longer,
more complex methods.

On Wed, Apr 14, 2021 at 12:56 PM Kirk Lund  wrote:

> Our coding standard and our Design Decisions does NOT require using 
final
> on parameters and local variables.
>
> Please do NOT request that I add or keep final on parameters or local
> variables unless the community votes and decides that every parameter 
and
> local variable should require the final keyword. I coded this way in 
the
> past and I found that it resulted in noisy code with no benefit. We 
can
> argue about using this keyword all you want but the fact is I'm 
against it,
> and I will not embrace it unless this community decides that we need 
to use
> it.
>
> Using final on instance or class fields does have a concurrency 
benefit and
> I support that only.
>
> If you want to add final to every single parameter and local var in 
the
> Geode codebase, then now is your chance to vote on it. Please vote.
>
> Thanks,
> Kirk
>

Re: code-owners seems to have some problems

[Robert Houghton]

Hi Mark,

Yes, each area of ownership only needs one review-by-owner.
Your following question, I do not understand, but I'll try: Any PR that needs 
review, will have text in the "reviewers needed and status-required" area, that 
states who is still needed to review. However, there is not a direct way to see 
if a co-owner has already reviewed a file you are looking at.

-Robert

On 2/25/21, 10:36 AM, "Mark Hanson"  wrote:

Hi Owen, 

Two more questions.
Is it the case that only one of the code owners for that area has to review 
the PR?
Also, is there a way to tell that a code owner has reviewed an area, so you 
don't have to as one of the other code owners?

Thanks,
Mark

On 2/25/21, 10:31 AM, "Owen Nichols"  wrote:

GitHub does not add reviewers from CODEOWNERS to PRs created as draft 
PRs (until you mark it not-draft by clicking Ready For Review).  However if you 
subsequently change it back to a draft, GitHub will not remove any reviewers.

On 2/25/21, 10:28 AM, "Mark Hanson"  wrote:

Hi Owen,

Is there a way to ensure that draft mode PRs aren't requesting 
reviews from code owners?

Thanks,
Mark

On 2/19/21, 11:44 AM, "Owen Nichols"  wrote:

GitHub provides some tools to answer this kind of question.

Step 1: Under the PR's "Files Changed" tab, click File Filter > 
Your CODEOWNER files
Step 2: Hover over the keyhole/shield icon (to the left of the 
red/green changecount graphic to the left of each filename) to see who else is 
also owner.
Step 3: If the ownership doesn't seem quite right, click on the 
keyhole/shield icon to see the exact line in CODEOWNERS that was applied.

Please note, if a file has NO codeowner, it will still show up 
in the "Your CODEOWNER files" (but no keyhole/shield indicator will be 
displayed).  It would be fantastic to find owners for these remaining unowned 
files, but until then owned-by-no-one == owned-by-everyone.

Improvements to CODEOWNERS can always be submitted via PR.

Some files in your list below have no owner, but several are 
owned by someone else so they shouldn't be showing up in your list.  Perhaps 
you copy this list from the Jump To dropdown?  If so, it seems that a 
File Filter selection does not filter the Jump To dropdown, only the display 
below.

On 2/19/21, 11:30 AM, "Bruce Schuchardt"  
wrote:

I was pulled in to PR 5989 but can’t figure out how that 
happened with the current CODEOWNERS file.  These all seem out of my area:

boms/geode-all-bom/src/test/resources/expected-pom.xml
  
buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy
 extensions/geode-modules-session/build.gradle
 extensions/geode-modules/build.gradle
  
extensions/geode-modules/src/test/resources/expected-pom.xml
 extensions/session-testing-war/build.gradle
  
...embly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatInstall.java
   
geode-assembly/src/acceptanceTest/java/org/apache/geode/modules/DeployJarAcceptanceTest.java
 
...butedTest/java/org/apache/geode/management/internal/rest/DeployToMultiGroupDUnitTest.java
  
...tedTest/java/org/apache/geode/management/internal/rest/DeploymentManagementDUnitTest.java
 
.../java/org/apache/geode/management/internal/rest/DeploymentManagementRedployDUnitTest.java
  
...java/org/apache/geode/management/internal/rest/DeploymentSemanticVersionJarDUnitTest.java
  
geode-assembly/src/integrationTest/resources/assembly_content.txt
 
geode-assembly/src/integrationTest/resources/dependency_classpath.txt
 
.../java/org/apache/geode/connectors/jdbc/internal/cli/CreateDataSourceCommandDUnitTest.java
 geode-core/build.gradle
  
...a/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
  
geode-core/src/integrationTest/java/org/apache/geode/internal/ClassPathLoaderDeployTest.java

Re: [VOTE] Apache Geode 1.12.1.RC4

[Robert Houghton]

+1

I verified the version.properties file contents to match the pipeline version 
that we've tested and verified.
-Robert Houghton


On 2/21/21, 12:08 PM, "Owen Nichols"  wrote:

Hello Geode Dev Community,

This is a release candidate for Apache Geode version 1.12.1.RC4.
Thanks to all the community members for their contributions to this release!

Please do a review and give your feedback, including the checks you 
performed.

Voting deadline:
3PM PST Thu, February 25 2021.

Please note that we are voting upon the source tag:
rel/v1.12.1.RC4

Release notes:

https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.12.1

Source and binary distributions:
https://dist.apache.org/repos/dist/dev/geode/1.12.1.RC4/

Maven staging repo:
https://repository.apache.org/content/repositories/orgapachegeode-1075

GitHub:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Ftree%2Frel%2Fv1.12.1.RC4data=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=yBvhGM48yiWnZ0Gz3Q9mI%2B3J2ddoAoBGh4Oi3Jmo1Mw%3Dreserved=0

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-examples%2Ftree%2Frel%2Fv1.12.1.RC4data=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=92jZ7B2TeRyIUgcHSToUKC8sW%2Bh%2BRRt8sSTMTAKS%2Fj0%3Dreserved=0

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Ftree%2Frel%2Fv1.12.1.RC4data=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tJ7gap56xlCUH7vlWzebu1bss%2BTRmBJMBM6AYZOrqKg%3Dreserved=0

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-benchmarks%2Ftree%2Frel%2Fv1.12.1.RC4data=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=b1aVxqH8Kynl9BkHk3%2Fgxin5GDNaxuP684sAtqy3zD0%3Dreserved=0

Pipelines:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fteams%2Fmain%2Fpipelines%2Fapache-support-1-12-maindata=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=oeLPQLMuJYX3h%2FQTVex61x6s0jdaXywWJlWIgZU%2Ftzw%3Dreserved=0

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fteams%2Fmain%2Fpipelines%2Fapache-support-1-12-rcdata=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=fLNXbsgn%2BuX44vumuIMB2mluKjb5Uz7k9%2BemWLcQH70%3Dreserved=0

Geode's KEYS file containing PGP keys we use to sign the release:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2FKEYSdata=04%7C01%7Crhoughton%40vmware.com%7Cf221b519b16b4ebf161008d8d6a46188%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637495348793363365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=X%2BGCSmUWwk8INBHRgBFq0biDO58gZMcVtZRagPptByo%3Dreserved=0

Command to run geode-examples:
./gradlew 
-PgeodeReleaseUrl=https://dist.apache.org/repos/dist/dev/geode/1.12.1.RC4 
-PgeodeRepositoryUrl=https://repository.apache.org/content/repositories/orgapachegeode-1075
 build runAll

Regards
Owen Nichols

Re: Need Access to apachegeode/geode-native-build on docker hub

[Robert Houghton]

The apachegeode-ci pipeline is storing images on a GCR project aptly named 
"apachegeode-ci". It is currently a private repo, hosted on Google, paid by 
VMware.

-Robert

On 2/22/21, 9:06 AM, "Anthony Baker"  wrote:

Where are we hosting images for the geode (not the geode-native) pipeline?  
While it might make sense to use the same repo across the board, the images for 
geode-native and geode-site can be useful beyond just CI.  I know I use the 
geode-native-build image to vet releases.

Anthony


> On Feb 22, 2021, at 8:12 AM, Jacob Barrett  wrote:
> 
> This docker image has been there for years and is not related to a 
release but to the LGTM and Travis builds. I have access and can bump the image 
for you Mike.
> 
> -Jake
> 
>> On Feb 17, 2021, at 1:35 PM, Owen Nichols  wrote:
>> 
>> apachegeode images on dockerhub correspond to releases. To make a 
change, the change must first come to appropriate release branch, then a Geode 
release made.
>> 
>> Perhaps trying to use release images for CI is not quite the right 
strategy?
>> 
>> 
>> ---
>> Sent from Workspace ONE 
Boxer
>> 
>> On February 17, 2021 at 1:24:04 PM PST, Mike Martell 
 wrote:
>> Please provide access to geode-native images on docker hub. Need to 
update our latest docker image to support our new open source CI for 
geode-native. In particular, need to upgrade to use cmake 3.18.
>> 
>> Thanks,
>> Mike
> 

Re: Concourse Access

[Robert Houghton]

Confirmed you are on the committer list. Done.

On 2/5/21, 9:08 AM, "Jacob Barrett"  wrote:

May I please have my Concourse access restored.

GitHub: pivotal-jbarrett

Thanks,
Jake

Re: Need Concourse Access

[Robert Houghton]

Done.

On 2/4/21, 11:06 AM, "Mike Martell"  wrote:

Please provide concourse access to:
github user: mmartell

Re: GEODE-8883 - Enable code-owner reviews on PRs

[Robert Houghton]

For all interested developers, I have submitted a PR to modify Geode’s GitHub 
permissions to require a code-owner review to all PRs against geode/develop, 
using the .asf.yaml file. Apache runs a bot that scans that file for repository 
rules, in order to keep down the number of INFRA tickets that projects need to 
file.

I have tried to make sure that the existing repository rules are kept. Please 
review, and if I missed a rule we can fix it.

Thanks,
-Robert Houghton

[DISCUSS] CODEOWNERS and .asf.yml

[Robert Houghton]

Hello fellow Geode developers.

The CODEOWNERS file has been merged to Geode develop for a few days now,
and we are starting to see automatic reviewers being added to code changes.
My thought is that we'll give this a week or so, to make sure that we like
the behavior we are seeing, and then to add the .asf.yml change that will
make the review of one owner mandatory for PR merge.

I don't think this needs a formal vote. But please do let me know what you
think.

-Robert Houghton

Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

[Robert Houghton]

To be honest, I hadn’t thought of the mess that might make. I guess, we can 
either make changes directly to the feature branch, which will eventually have 
a pull-request back to [develop], or we can modify Owen’s existing PR. At the 
end of the day, it all has to go through the normal PR process back to the 
develop branch.

-Robert

From: Jens Deppe 
Date: Monday, December 7, 2020 at 11:43 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
Should we just be adding to Owen's PR?

If everyone creates their own PR we'll probably end up with a bunch of merge 
conflicts.

--Jens

On 12/7/20, 10:07 AM, "Robert Houghton"  wrote:

I haven’t seen any negative responses, and a big THANK YOU to @onichols for 
making the first PR to self-volunteer into ownership of some code. Lets give 
people another week to nominate, or self-nominate, as PRs against 
[feature/introduce-codeowners], and plan to merge this feature into develop on 
14 December?




From: Owen Nichols 
Date: Tuesday, December 1, 2020 at 8:56 PM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
It looks like the discussion period ended Nov 25 with all comments in 
support, so next phase is populating the proposed codeowners file.  @Robert how 
long do we have to populate this before you plan to formally propose activating 
it on develop?

As prescribed I’ve submitted a PR[1] against the 
feature/introduce-codeowners branch to nominate myself as a code owner for 
areas I feel qualified to own.

[1] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5802data=04%7C01%7Crhoughton%40vmware.com%7Ce548ca1d6b5440cf3b1008d89ae86bc4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637429670315071059%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=qUKX575Q1I3q2OXEO7BJvr5ms4yVM7nG%2BAF2X7panXM%3Dreserved=0

From: Xiaojian Zhou 
Date: Friday, November 20, 2020 at 11:06 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
+1

I saw the template of splitting the geode code. Can someone nominate a few 
codeowners in the file as examples?

On 11/20/20, 7:32 AM, "Alexander Murmann"  wrote:

+1

I agree with Owen's point that this will improve the experience for new 
contributors. It also helps people new to the community to have confidence that 
they got the type of review they need to feel confident to merge. I might get 
to reviews that are both from great committers who can review for things like 
coding style, test coverage etc. However, I might be unaware that neither of 
them know the area I am modifying particularly well. This solves this problem. 
I can merge with more confidence, once I got the review from the owner.

From: Anthony Baker 
Sent: Thursday, November 19, 2020 17:55
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

+1

I think we as a project will need to iterator on the code owners as 
well as the process for code owners.  But this is a model that has been adopted 
by a number of OSS projects both within and outside of Apache.  I like that it 
provides visibility to PR authors and associates motivated experts to review 
and merge changes.

Anthony


> On Nov 19, 2020, at 10:46 AM, Ernie Burghardt  
wrote:
>
> Perfect, then let's give this a try.
> +1
>
    > On 11/19/20, 10:45 AM, "Robert Houghton"  wrote:
>
>Hi Ernie,
>
>DRAFT PRs do not get reviewers by default, but when the draft 
transitions to ‘ready’, then the owners are requested to review.
>
>
>From: Ernie Burghardt 
>Date: Thursday, November 19, 2020 at 9:56 AM
>To: dev@geode.apache.org 
>Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
>Does GitHub allow us to limit this automated action to non-DRAFT 
PRs?
>
>On 11/18/20, 8:28 PM, "Owen Nichols"  wrote:
>
>+1 This will greatly improve the experience for contributors.  
Instead of an intimidating empty list of reviewers when you submit a PR (and no 
ability to add reviewers, if you’re not a committer), it will be great to 
already have at least two reviewers automagically assigned.
>
>I have a small concern that initially populating this file via 
a flurry of PRs may result in a lot of merge conflicts with anyone else that 
volunteers on the same or an adjacent line.  Also, since you _must_ be a 
committer to be a code owner, is a PR even necessary…would directly committing 
changes to the featu

Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

[Robert Houghton]

I haven’t seen any negative responses, and a big THANK YOU to @onichols for 
making the first PR to self-volunteer into ownership of some code. Lets give 
people another week to nominate, or self-nominate, as PRs against 
[feature/introduce-codeowners], and plan to merge this feature into develop on 
14 December?




From: Owen Nichols 
Date: Tuesday, December 1, 2020 at 8:56 PM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
It looks like the discussion period ended Nov 25 with all comments in support, 
so next phase is populating the proposed codeowners file.  @Robert how long do 
we have to populate this before you plan to formally propose activating it on 
develop?

As prescribed I’ve submitted a PR[1] against the feature/introduce-codeowners 
branch to nominate myself as a code owner for areas I feel qualified to own.

[1] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5802data=04%7C01%7Crhoughton%40vmware.com%7C8d82983774b843bcfee808d8967eb03e%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637424818157270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=peSSJwhlpb%2BTwpfpsVRECFYsQ29G4uyOTQ00C1pQuj8%3Dreserved=0

From: Xiaojian Zhou 
Date: Friday, November 20, 2020 at 11:06 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
+1

I saw the template of splitting the geode code. Can someone nominate a few 
codeowners in the file as examples?

On 11/20/20, 7:32 AM, "Alexander Murmann"  wrote:

+1

I agree with Owen's point that this will improve the experience for new 
contributors. It also helps people new to the community to have confidence that 
they got the type of review they need to feel confident to merge. I might get 
to reviews that are both from great committers who can review for things like 
coding style, test coverage etc. However, I might be unaware that neither of 
them know the area I am modifying particularly well. This solves this problem. 
I can merge with more confidence, once I got the review from the owner.

From: Anthony Baker 
Sent: Thursday, November 19, 2020 17:55
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

+1

I think we as a project will need to iterator on the code owners as well as 
the process for code owners.  But this is a model that has been adopted by a 
number of OSS projects both within and outside of Apache.  I like that it 
provides visibility to PR authors and associates motivated experts to review 
and merge changes.

Anthony


> On Nov 19, 2020, at 10:46 AM, Ernie Burghardt  
wrote:
>
> Perfect, then let's give this a try.
> +1
>
> On 11/19/20, 10:45 AM, "Robert Houghton"  wrote:
>
>Hi Ernie,
>
>DRAFT PRs do not get reviewers by default, but when the draft 
transitions to ‘ready’, then the owners are requested to review.
>
>
>From: Ernie Burghardt 
>Date: Thursday, November 19, 2020 at 9:56 AM
>To: dev@geode.apache.org 
>Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
>Does GitHub allow us to limit this automated action to non-DRAFT PRs?
>
>On 11/18/20, 8:28 PM, "Owen Nichols"  wrote:
>
>+1 This will greatly improve the experience for contributors.  
Instead of an intimidating empty list of reviewers when you submit a PR (and no 
ability to add reviewers, if you’re not a committer), it will be great to 
already have at least two reviewers automagically assigned.
>
>I have a small concern that initially populating this file via a 
flurry of PRs may result in a lot of merge conflicts with anyone else that 
volunteers on the same or an adjacent line.  Also, since you _must_ be a 
committer to be a code owner, is a PR even necessary…would directly committing 
changes to the feature/introduce-codeowners branch be acceptable?  If not, who 
needs to review and who can merge the PRs against the ‘introduce’ branch?
>
>What happens if you are the only owner for an area, can you 
approve your own PR?  Even if the goal is two owners per area, does that mean 
PRs by either owner cannot be merged if the only other owner is on vacation or 
otherwise unavailable?
>
>Can we submit PRs against the ‘introduce’ branch now and they just 
won’t be merged before Nov 26, or do we all just need to be patient until this 
review period has concluded?
>
>From: Robert Houghton 
>Date: Wednesday, November 18, 2020 at 2:07 PM
>To: dev@geode.apache.org 
>Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode
>Hello Devs.
>

Re: [DISCUSS] Geode 1.14

[Robert Houghton]

@alexander, I agree that stabilizing on 1.14 will be much easier to do, than to 
manage the fixes while develop features move along apace. I disagree that 
shifting attention to 1.14 will mean that develop does not get better. We can 
port the fixes forward, just as easily as we backport them. Same effort, but a 
more-quickly-stable release branch.

-Robert

From: Alexander Murmann 
Date: Monday, November 30, 2020 at 11:57 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Geode 1.14
Hi all,

Thanks, Owen for reminding us all of this topic!

I wonder how we feel about the state of develop right now. If we cut 1.14 right 
now, it will make it easier to stabilize and ship it. However, I see 21 open 
JIRA tickets affecting 1.14.0. It might be better to have an all-hands effort 
to address as much as possible on develop and then cut 1.14. If we shift all 
attention to 1.14, develop will likely never get better. I'd love to get closer 
to an always shippable develop branch. That should vastly reduce future release 
pain and make everyday development better as well.

Thoughts?

From: Jens Deppe 
Sent: Wednesday, November 25, 2020 20:11
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Geode 1.14

Hi Owen,

Thanks for starting this conversation and especially for volunteering as 
Release Manager!

Since we're already a couple of quarters 'behind', in terms of releases, I'd 
prefer cutting the 1.14 branch ASAP. Leaving it until Feb means we'll have 9 
months of changes to stabilize. How long might that take to finally get 
shipped? (rhetorical).

--Jens

On 11/25/20, 6:05 PM, "Owen Nichols"  wrote:

The trigger in @Alexander’s July 28 proposal to postpone 1.14 has been met 
(we shipped 1.13).
It’s time to discuss when we want to cut the 1.14 branch.  I will volunteer 
as Release Manager.

Below are all release dates since Geode adopted a time-based release 
cadence.

Minor releases:
1.13   branch cut May 4 2020, 1.13.0 shipped Sep 9 2020
1.12   branch cut Feb 4 20201.12.0 shipped Mar 31 2020
1.11   branch cut Nov 4 2019   1.11.0 shipped Dec 31 2019
1.10   branch cut Aug 2 2019   1.10.0 shipped Sep 26 2019
1.9  branch cut Feb 19 2019 1.9.0 shipped Apr 24 2019
1.8  branch cut Nov 1 2018   1.8.0 shipped Dec 12 2018

Patch releases:
1.13.1shipped Nov 18 2020
1.9.2  shipped Nov 14 2019
1.9.1  shipped Sep 6 2019

I’ll toss out an initial suggestion: Let’s cut the support/1.14 branch on 
the next date in the original quarterly cadence: Feb 1 2021.

-Owen

From: Alexander Murmann 
Date: Tuesday, July 28, 2020 at 4:34 PM
To: dev@geode.apache.org 
Subject: [PROPOSAL] Postpone Geode 1.14
Hi all,

As mentioned on the previous discuss thread, I propose to hold off cutting
1.14 until we have shipped 1.13.

Once we have shipped 1.13, we should discuss when we want to cut the 1.14
release. The actual ship date for Geode 1.13 is important information for
that conversation. Thus we cannot have that conversation before then.

Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

[Robert Houghton]

Hi Ernie,

DRAFT PRs do not get reviewers by default, but when the draft transitions to 
‘ready’, then the owners are requested to review.


From: Ernie Burghardt 
Date: Thursday, November 19, 2020 at 9:56 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
Does GitHub allow us to limit this automated action to non-DRAFT PRs?

On 11/18/20, 8:28 PM, "Owen Nichols"  wrote:

+1 This will greatly improve the experience for contributors.  Instead of 
an intimidating empty list of reviewers when you submit a PR (and no ability to 
add reviewers, if you’re not a committer), it will be great to already have at 
least two reviewers automagically assigned.

I have a small concern that initially populating this file via a flurry of 
PRs may result in a lot of merge conflicts with anyone else that volunteers on 
the same or an adjacent line.  Also, since you _must_ be a committer to be a 
code owner, is a PR even necessary…would directly committing changes to the 
feature/introduce-codeowners branch be acceptable?  If not, who needs to review 
and who can merge the PRs against the ‘introduce’ branch?

What happens if you are the only owner for an area, can you approve your 
own PR?  Even if the goal is two owners per area, does that mean PRs by either 
owner cannot be merged if the only other owner is on vacation or otherwise 
unavailable?

Can we submit PRs against the ‘introduce’ branch now and they just won’t be 
merged before Nov 26, or do we all just need to be patient until this review 
period has concluded?

From: Robert Houghton 
Date: Wednesday, November 18, 2020 at 2:07 PM
To: dev@geode.apache.org 
Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode
Hello Devs.

I would like to improve the quality of the pull-request reviews we see for
critical parts of the Apache Geode project. In discussions with other
committers, a (not the) big hurdle to that is getting the right eyes to
look at a given PR. To that end, I propose the adoption of GitHub's
CODEOWNERS functionality for the Apache Geode code repository.

A discussion-document of this issue has been written up
by @upthewaterspout. Thanks Dan!

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfiledata=04%7C01%7Crhoughton%40vmware.com%7C53e36a75a03c4910b4f508d88cb4791b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414054040266773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=xXvg59MWDSLaQsC3Ik5ikZ1MFkCKnlpJaoEQ5EgwB4E%3Dreserved=0

I have tested the feature with fellow Geode committers @upthewaterspout
and @onichols-pivotal, and found it to meet our expectations.  Please
review the document, and comment or reply to this thread, by 25 November,
so we might start the task of nominating and applying for ownership.

    -Robert Houghton

Re: [DISCUSS] Adding CODEOWNERS to Apache Geode

[Robert Houghton]

@onichols, to answer your questions:

  *   If there is only one owner, and the owner is on vacation, then a PR will 
be delayed
  *   If you are an owner, and the only other owner is on vacation, then the PR 
will be delayed
  *   If one is the sole owner, then the owner-review requirement is not 
enforced (you can’t review your own PR)
  *   Feel free to start PRs or changes to `features/introduce_codeowners`



From: Owen Nichols 
Date: Wednesday, November 18, 2020 at 8:28 PM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
+1 This will greatly improve the experience for contributors.  Instead of an 
intimidating empty list of reviewers when you submit a PR (and no ability to 
add reviewers, if you’re not a committer), it will be great to already have at 
least two reviewers automagically assigned.

I have a small concern that initially populating this file via a flurry of PRs 
may result in a lot of merge conflicts with anyone else that volunteers on the 
same or an adjacent line.  Also, since you _must_ be a committer to be a code 
owner, is a PR even necessary…would directly committing changes to the 
feature/introduce-codeowners branch be acceptable?  If not, who needs to review 
and who can merge the PRs against the ‘introduce’ branch?

What happens if you are the only owner for an area, can you approve your own 
PR?  Even if the goal is two owners per area, does that mean PRs by either 
owner cannot be merged if the only other owner is on vacation or otherwise 
unavailable?

Can we submit PRs against the ‘introduce’ branch now and they just won’t be 
merged before Nov 26, or do we all just need to be patient until this review 
period has concluded?

From: Robert Houghton 
Date: Wednesday, November 18, 2020 at 2:07 PM
To: dev@geode.apache.org 
Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode
Hello Devs.

I would like to improve the quality of the pull-request reviews we see for
critical parts of the Apache Geode project. In discussions with other
committers, a (not the) big hurdle to that is getting the right eyes to
look at a given PR. To that end, I propose the adoption of GitHub's
CODEOWNERS functionality for the Apache Geode code repository.

A discussion-document of this issue has been written up
by @upthewaterspout. Thanks Dan!
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfiledata=04%7C01%7Crhoughton%40vmware.com%7C78b888ac11b9497a0ebf08d88c437d63%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413568802236824%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=h7uZXWAqmYrcyry6q5%2BYH4PRrMdSi9egPc3KsD9lPX8%3Dreserved=0

I have tested the feature with fellow Geode committers @upthewaterspout
and @onichols-pivotal, and found it to meet our expectations.  Please
review the document, and comment or reply to this thread, by 25 November,
so we might start the task of nominating and applying for ownership.

-Robert Houghton

[DISCUSS] Adding CODEOWNERS to Apache Geode

[Robert Houghton]

Hello Devs.

I would like to improve the quality of the pull-request reviews we see for
critical parts of the Apache Geode project. In discussions with other
committers, a (not the) big hurdle to that is getting the right eyes to
look at a given PR. To that end, I propose the adoption of GitHub's
CODEOWNERS functionality for the Apache Geode code repository.

A discussion-document of this issue has been written up
by @upthewaterspout. Thanks Dan!
https://cwiki.apache.org/confluence/display/GEODE/Introduce+Codeowners+file

I have tested the feature with fellow Geode committers @upthewaterspout
and @onichols-pivotal, and found it to meet our expectations.  Please
review the document, and comment or reply to this thread, by 25 November,
so we might start the task of nominating and applying for ownership.

-Robert Houghton

Utilizing GitHub .CODEOWNERS files

[Robert Houghton]

Thanks to @udo and @nabarunnag the conversations and ideas around having
better review processes and review quality.

TL;DR - Adding GitHub CODEOWNERS[1] to geode-examples, eye toward adding to
GEODE

GitHub provides a mechanism of assigning code ownership to specific
owners/committers based on file pattern-matching (similar to .gitignore).
Owners are automatically added as reviewers to pull-requests against their
described areas. This, coupled with our review requirements[2], could
really help us get the right eyes on prospective code changes.

I intend to add this file to the apache/geode-examples repository, tagging
myself and a few volunteers to different code areas, and checking the
results. If we like it, we can move on to doing the same to Geode.

Will anyone volunteer to get code-reviews for geode-examples changes (that
we won't merge) ?

Thanks,
-Robert Houghton

[1]
https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/about-code-owners
[2]
https://cwiki.apache.org/confluence/display/INFRA/git+-+.asf.yaml+features#git.asf.yamlfeatures-BranchProtection

Re: PR process and etiquette

[Robert Houghton]

There are some pieces of Apache infrastructure we can control without needing 
tickets: Specifically
required_pull_request_reviews:
dismiss_stale_reviews: true
require_code_owner_reviews: true

I think these specific controls could go a long way towards helping keep our PR 
quality high, while reducing cognitive load for the reviewers.

Full documentation 
here



From: Bruce Schuchardt 
Date: Wednesday, October 28, 2020 at 8:10 AM
To: dev@geode.apache.org 
Subject: Re: PR process and etiquette
-1
While I often use the Draft option I don't see why we want to add even more 
rules about how we use github.  I think it's enough to put in a PR and then add 
reviewers when you're ready for comments.  Getting the stink-eye for putting up 
a non-Draft PR is just going to make it more difficult to attract and retain 
new contributors.

On 10/27/20, 5:41 PM, "Udo Kohlmeyer"  wrote:

Dear Apache Geode Devs,
It is really great going through all the PRs that been submitted. As Josh 
Long is known to say: "I work for PRs".
Whilst going through some of the PRs I do see that there are many PRs that 
have multiple commits against the PR.
I know that the PR submission framework kicks off more testing than we do 
on our own local machines. It is extremely uncommon to submit a PR the first 
time and have all tests go green. Which h means we invariably iterate over 
commits to make the build go green.
In this limbo time period, it is hard for any reviewer to know when the 
ticket is ready to be reviewed.
I want to propose that when submitting a PR, it is initially submitted as a 
DRAFT PR. This way, all test can still be run and work can be done to make sure 
"green" is achieved. Once "green" status has been achieved, the draft can then 
be upgraded to a final PR by pressing the "Ready For Review" button. At this 
point all commits on the branch can then once again be squashed into a single 
commit.
Now project committers will now know that the PR is in a state that it can 
be reviewed for completeness and functionality.
In addition, it will help tremendously helpful if anyone submitting a PR 
monitors their PR for activity. If there is no activity for a few days, please 
feel free to ping the Apache Geode Dev community for review. If review is 
request, please prioritize some time to address the feedback, as reviewers 
spend time reviewing code and getting an understanding what the code is doing. 
If too much time goes by, between review and addressing the review comments, 
not only does the reviewer lose context, possibly requiring them to spend time 
again to understand what the code was/is supposed to do, but also possibly lose 
interest, as the ticket has now become cold or dropped down the list of PRs.
There are currently many PRs that are in a cold state, where the time 
between review and response has been so long, that both parties (reviewer and 
submitter) have forgotten about the PR.
In the case that the reviews will take more time to address than expected, 
please downgrade the PR to draft status again. At this point, it does not mean 
that reviewers will not be able to help anymore, as you can request a reviewer 
to help with feedback and comments, until one feels that the PR is back in a 
state of final submission.
So, what I'm really asking from the Dev Community:
If you submit a PR, it would be great if you can nudge the 
community if there is no review on the PR. If feedback is provided on a PR, 
please address it as soon as possible. This not only will help the PR progress 
faster, but it will shorten the feedback loop.
Finally, start with a DRAFT PR and then upgrade to a final PR when 
you feel it is in a "good" state. If more work is required, it is ok to 
downgrade back to a draft, until one feels the PR is in a good state again.
Managing the PR process in this manner, will be hugely beneficial for all 
community members. As reviewers will know when a PR is ready to be reviewed. 
Reviewers will also feel more engaged in this process, due to timely response 
to their review comments. PR submitters will feel happier, in the knowledge 
that the code they spent so long meticulously crafting will possibly make it 
into the project.
Any thoughts?
--Udo

Re: [Suspected Spam] Access to Geode concourse pipeline

[Robert Houghton]

Hi Sara. This got thrown into my junk list (thanks, Outlook). I’ll get on this.


From: Sarah Abbey 
Date: Wednesday, September 30, 2020 at 9:07 AM
To: dev@geode.apache.org 
Subject: [Suspected Spam] Access to Geode concourse pipeline
Hello!

I would like to request access to the Geode concourse 
pipeline.

Thank you,
Sarah

Re: Please give me access to debug concourse runs

[Robert Houghton]

Hi! I'll set that up first thing tomorrow

On Aug 18, 2020 16:49, Darrel Schneider  wrote:
I would like to use "fly" to login to concourse and look at some test runs that 
are hanging. Could I be given access please?
Thanks

Re: [PROPOSAL] one-time cleanup of stray and obsolete tags in geode repo

[Robert Houghton]

+1

I dug in more. My thoughts on the `highwater` tag were out of date.



From: Mark Bretl 
Date: Thursday, July 30, 2020 at 12:47 PM
To: geode 
Subject: Re: [PROPOSAL] one-time cleanup of stray and obsolete tags in geode 
repo
+1

--Mark

On Thu, Jul 30, 2020 at 1:30 PM Robert Houghton 
wrote:

> Highwater isn't used by benchmarks?
>
> On Jul 30, 2020 12:02, Dave Barnes  wrote:
> +1
>
> On Thu, Jul 30, 2020 at 8:45 AM Owen Nichols  wrote:
>
> > Hi Donal, I can confirm that develop/highwater hasn't been updated in a
> > year and is no longer in use by any pipelines (all pipelines use the last
> > release tag as the benchmark baseline now).
> >
> > On 7/30/20, 5:48 AM, "Donal Evans"  wrote:
> >
> > I may be mistaken, but I think the develop/highwater tag was used by
> > the geode-benchmarks project. Can we get confirmation that it's no longer
> > in use?
> >
> > +1 conditional on that
> >
> > Get Outlook for Android<
> >
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Faka.ms%2Fghei36data=02%7C01%7Crhoughton%40vmware.com%7Cfbb521db9056434e376f08d834c16e71%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637317352668622083sdata=IS0DXT0bGqxezj7cOZlHq9qfDe8DCUJ7cF3LyV5Y6XM%3Dreserved=0
> > >
> >
> > 
> > From: Ju@N 
> > Sent: Thursday, July 30, 2020 3:10:44 AM
> > To: dev@geode.apache.org 
> > Subject: Re: [PROPOSAL] one-time cleanup of stray and obsolete tags
> in
> > geode repo
> >
> > +1
> >
> > On Thu 30 Jul 2020 at 08:21, Owen Nichols 
> wrote:
> >
> > > Tags in the rel/ namespace should be created by the Geode release
> > manager
> > > as part of an official Geode release only, yet we seem to have some
> > extra
> > > ones somehow.
> > > Further, I don't see any value in keeping RC tags forever long
> after
> > the
> > > release is final.
> > >
> > > Please vote +1 in favor of trimming the current list of 110 tags
> > down to
> > > 20 to make it nice and easy for newcomers (as well as the rest of
> > us) to
> > > find and checkout a specific version of Geode; otherwise vote -1.
> > If the
> > > majority vote is +1 at 3PM PDT Fri Aug 7, I will proceed to archive
> > and
> > > delete the 90 unnecessary tags as detailed below.
> > >
> > > I propose to KEEP only the following tags, corresponding to
> official
> > Geode
> > > releases:
> > >
> > > rel/v1.12.0
> > > rel/v1.11.0
> > > rel/v1.10.0
> > > rel/v1.9.2
> > > rel/v1.9.1
> > > rel/v1.9.0
> > > rel/v1.8.0
> > > rel/v1.7.0
> > > rel/v1.6.0
> > > rel/v1.5.0
> > > rel/v1.4.0
> > > rel/v1.3.0
> > > rel/v1.2.1
> > > rel/v1.2.0
> > > rel/v1.1.1
> > > rel/v1.1.0
> > > rel/v1.0.0-incubating
> > > rel/v1.0.0-incubating.M3
> > > rel/v1.0.0-incubating.M2
> > > rel/v1.0.0-incubating.M1
> > >
> > > I propose a one-time cleanup to DELETE all other tags,
> specifically:
> > >
> > > develop/highwater
> > > modules-pre-merge
> > > rel/v1.0.0-incubating.M1.RC1
> > > rel/v1.0.0-incubating.M1.RC2
> > > rel/v1.0.0-incubating.M2.RC1
> > > rel/v1.0.0-incubating.M2.RC2
> > > rel/v1.0.0-incubating.M3.RC1
> > > rel/v1.0.0-incubating.M3.RC2
> > > rel/v1.0.0-incubating.M3.RC3
> > > rel/v1.0.0-incubating.M3.RC4
> > > rel/v1.0.0-incubating.M3.RC5
> > > rel/v1.0.0-incubating.M3.RC6
> > > rel/v1.0.0-incubating.M3.RC7
> > > rel/v1.0.0-incubating.RC1
> > > rel/v1.0.0-incubating.RC2
> > > rel/v1.1.0.RC1
> > > rel/v1.1.0.RC2
> > > rel/v1.1.0manualrev-2017-02-16
> > > rel/v1.1.0manualrev-2017-10-19
> > > rel/v1.1.1.RC1
> > > rel/v1.1.1.RC2
> > > rel/v1.10.0.1
> > > rel/v1.10.0.1.RC1
> > > rel/v1.10.0.2
> > > rel/v1.10.0.RC1
> > > rel/v1.10.0.RC2
> > > rel/v1.11.0.1
> > > rel/v1.11.0.2
> > > rel/v1.11.0.23755
> > > rel/v1.11.0.23755_2
> > > rel/v1.11.0.23755_3
> > > rel/v1.11.0.3
> > > rel/v1.11.0.4
> > > rel/v1.11.0.5

Re: [PROPOSAL] one-time cleanup of stray and obsolete tags in geode repo

[Robert Houghton]

Highwater isn't used by benchmarks?

On Jul 30, 2020 12:02, Dave Barnes  wrote:
+1

On Thu, Jul 30, 2020 at 8:45 AM Owen Nichols  wrote:

> Hi Donal, I can confirm that develop/highwater hasn't been updated in a
> year and is no longer in use by any pipelines (all pipelines use the last
> release tag as the benchmark baseline now).
>
> On 7/30/20, 5:48 AM, "Donal Evans"  wrote:
>
> I may be mistaken, but I think the develop/highwater tag was used by
> the geode-benchmarks project. Can we get confirmation that it's no longer
> in use?
>
> +1 conditional on that
>
> Get Outlook for Android<
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Faka.ms%2Fghei36data=02%7C01%7Crhoughton%40vmware.com%7C399c0ee0d7824ffc4e5008d834bb2a3e%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637317325761204283sdata=xmzDLn4YsP4KyZl9vWEbY2pOzWHJDKef7dMtgtEHy3E%3Dreserved=0
> >
>
> 
> From: Ju@N 
> Sent: Thursday, July 30, 2020 3:10:44 AM
> To: dev@geode.apache.org 
> Subject: Re: [PROPOSAL] one-time cleanup of stray and obsolete tags in
> geode repo
>
> +1
>
> On Thu 30 Jul 2020 at 08:21, Owen Nichols  wrote:
>
> > Tags in the rel/ namespace should be created by the Geode release
> manager
> > as part of an official Geode release only, yet we seem to have some
> extra
> > ones somehow.
> > Further, I don't see any value in keeping RC tags forever long after
> the
> > release is final.
> >
> > Please vote +1 in favor of trimming the current list of 110 tags
> down to
> > 20 to make it nice and easy for newcomers (as well as the rest of
> us) to
> > find and checkout a specific version of Geode; otherwise vote -1.
> If the
> > majority vote is +1 at 3PM PDT Fri Aug 7, I will proceed to archive
> and
> > delete the 90 unnecessary tags as detailed below.
> >
> > I propose to KEEP only the following tags, corresponding to official
> Geode
> > releases:
> >
> > rel/v1.12.0
> > rel/v1.11.0
> > rel/v1.10.0
> > rel/v1.9.2
> > rel/v1.9.1
> > rel/v1.9.0
> > rel/v1.8.0
> > rel/v1.7.0
> > rel/v1.6.0
> > rel/v1.5.0
> > rel/v1.4.0
> > rel/v1.3.0
> > rel/v1.2.1
> > rel/v1.2.0
> > rel/v1.1.1
> > rel/v1.1.0
> > rel/v1.0.0-incubating
> > rel/v1.0.0-incubating.M3
> > rel/v1.0.0-incubating.M2
> > rel/v1.0.0-incubating.M1
> >
> > I propose a one-time cleanup to DELETE all other tags, specifically:
> >
> > develop/highwater
> > modules-pre-merge
> > rel/v1.0.0-incubating.M1.RC1
> > rel/v1.0.0-incubating.M1.RC2
> > rel/v1.0.0-incubating.M2.RC1
> > rel/v1.0.0-incubating.M2.RC2
> > rel/v1.0.0-incubating.M3.RC1
> > rel/v1.0.0-incubating.M3.RC2
> > rel/v1.0.0-incubating.M3.RC3
> > rel/v1.0.0-incubating.M3.RC4
> > rel/v1.0.0-incubating.M3.RC5
> > rel/v1.0.0-incubating.M3.RC6
> > rel/v1.0.0-incubating.M3.RC7
> > rel/v1.0.0-incubating.RC1
> > rel/v1.0.0-incubating.RC2
> > rel/v1.1.0.RC1
> > rel/v1.1.0.RC2
> > rel/v1.1.0manualrev-2017-02-16
> > rel/v1.1.0manualrev-2017-10-19
> > rel/v1.1.1.RC1
> > rel/v1.1.1.RC2
> > rel/v1.10.0.1
> > rel/v1.10.0.1.RC1
> > rel/v1.10.0.2
> > rel/v1.10.0.RC1
> > rel/v1.10.0.RC2
> > rel/v1.11.0.1
> > rel/v1.11.0.2
> > rel/v1.11.0.23755
> > rel/v1.11.0.23755_2
> > rel/v1.11.0.23755_3
> > rel/v1.11.0.3
> > rel/v1.11.0.4
> > rel/v1.11.0.5
> > rel/v1.11.0.6
> > rel/v1.11.0.7
> > rel/v1.11.0.7565
> > rel/v1.11.0.8
> > rel/v1.11.0.RC1
> > rel/v1.11.0.RC2
> > rel/v1.11.0.RC3
> > rel/v1.11.0.RC4
> > rel/v1.12.0.1
> > rel/v1.12.0.1234
> > rel/v1.12.0.2
> > rel/v1.12.0.23755
> > rel/v1.12.0.3
> > rel/v1.12.0.4
> > rel/v1.12.0.5
> > rel/v1.12.0.6
> > rel/v1.12.0.RC1
> > rel/v1.12.0.RC2
> > rel/v1.12.0.RC3
> > rel/v1.12.0.RC4
> > rel/v1.14.0.23755
> > rel/v1.2.0.RC1
> > rel/v1.2.0.RC2
> > rel/v1.2.1.RC1
> > rel/v1.2.1.RC2
> > rel/v1.2.1.RC3
> > rel/v1.2.1.RC4
> > rel/v1.2.1manualrev-2017-10-19
> > rel/v1.3.0.RC1
> > rel/v1.3.0.RC2
> > rel/v1.3.0.RC3
> > rel/v1.3.0.RC4
> > rel/v1.4.0.RC1
> > rel/v1.4.0.RC2
> > rel/v1.5.0.RC1
> > rel/v1.5.0.RC2
> > rel/v1.6.0.RC1
> > rel/v1.7.0.RC1
> > rel/v1.7.0.RC2
> > rel/v1.8.0.RC1
> > rel/v1.8.0.RC2
> > rel/v1.9.0.1
> > rel/v1.9.0.1.RC1
> > rel/v1.9.0.2
> > rel/v1.9.0.RC1
> > rel/v1.9.0.RC2
> > rel/v1.9.0.RC3
> > rel/v1.9.0.RC4
> > rel/v1.9.1-nordix
> > rel/v1.9.1.RC1
> > rel/v1.9.1.RC2
> > rel/v1.9.1.RC3
> > rel/v1.9.2.RC1
> > sga2-core
> > v1.1.0manualrev-2017-10-19
> > v9.0.0-beta.1
> >
> --
> Ju@N
>
>

Re: Fate of master branch

[Robert Houghton]

We are presumably following the `git-flow` release process, of loosely. That 
process specifies that there is a branch which is the latest main (not 
supporting) release. If you want to call is `release` then fine. A rose by any 
other name, and all that. But having that reference is useful for the working 
model that we purport to follow.

From: Owen Nichols 
Sent: Friday, July 24, 2020 1:45 AM
To: dev@geode.apache.org 
Subject: Re: Fate of master branch

@Robert, would you care to elaborate on the case for keeping a branch (by any 
name) that this discussion thread overwhelmingly felt:
* "isn’t really in use for anything vital"
* "always a source of confusion"
* "don't see the need for it"
* "no good reason to keep it"
* "always a little unclear...what master was doing"

And if there is value in keeping it, why not pick a meaningful name like 
"latest_release" or "stable"?  If the goal is just to be a "symlink" to latest 
release tag, why not just explain in the README how to check out the tag for 
the release you want (which might not always be the latest release).

On 7/23/20, 7:53 PM, "Robert Houghton"  wrote:

I would not delete the branch without a new branch 'main' in its place

On Jul 23, 2020 17:50, Owen Nichols  wrote:
Now that geode-examples' default branch has been changed to develop, and 
nothing further has been added to this discussion in a while, would anyone like 
to call for a vote to eliminate master branch from all geode projects?

I would suggest holding this vote under 'code modification' rules[1] since 
we would be deleting code.  Even though master should be substantially 
equivalent to latest release tag (currently rel/v1.12.0), git diff shows a few 
small differences.

[1] a timeframe of at least 72 hours and a single -1 can veto, see 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.apache.org%2Ffoundation%2Fvoting.htmldata=02%7C01%7Crhoughton%40vmware.com%7C777db553b8ab4b233dcc08d82fadee8a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637311771371808600sdata=c60eG80iwUxt1YMX6mQIxu%2BsNaOz4oLCexOLFERQ8IQ%3Dreserved=0

On 7/7/20, 6:33 PM, "Owen Nichols"  wrote:

Since the branch proposed for deletion is the default branch in 
geode-examples, you will need to file an ASF INFRA ticket to change that 
default.  This is a great discussion thread, but ASF will require a [VOTE] 
thread to be cited.

I am concerned about keeping it easy for someone who has just cloned 
geode to identify the most stable branch for their purpose.  Before, they could 
always be assured `git checkout master` would give the flagship release.  Now, 
new users will be immediately forced into some daunting detective work to sift 
through hundreds of haphazard tags and branches (a task even veteran committers 
frequently fail).  I would strongly encourage an aggressive cleanup of 
unhelpful branches and tags, as Jacob proposed last month, before getting rid 
of the latest_release concept.

On 7/7/20, 8:24 AM, "Blake Bender"  wrote:

Just to follow up on this: I've filed GEODE-8335 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8335data=02%7C01%7Crhoughton%40vmware.com%7C777db553b8ab4b233dcc08d82fadee8a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637311771371818600sdata=lggG4kZu37SgNdlWaMKOTB4ulTtpbaDqrueUItiHK1E%3Dreserved=0)
 to track, respectfully (cowardly __ ) deferring to individuals who regularly 
contribute to the various Geode repos to handle it as they see fit.  I'll take 
care of the several Geode Native associated repos.

Thanks,

Blake


On 6/26/20, 12:21 PM, "Dave Barnes"  wrote:

+1 if we can override git’s ‘master’ default and establish 
‘develop’ in its place. Otherwise renaming to ‘main’ would solve the problem 
with the negative connotations.

NB: Mark, I think the 3-vote convention is just for back-ports 
to the release-branch. I don’t think of it as applying to a general discussion 
like this one.

> On Jun 26, 2020, at 9:42 AM, Anthony Baker 
 wrote:
>
> By just do it, I assume you mean:
>
> - Contact delete master where not needed
> - Rename master to main when needed
> - Contact INFRA to change the default branch
> - Update README and CI jobs as needed
>
> Across *all* geode repos.
>
>
> Anthony
>
>
>> On Jun 26, 2020, at 9:38 AM, Mark Hanson 
 wrote:
>>
>&

Re: Fate of master branch

[Robert Houghton]

I would not delete the branch without a new branch 'main' in its place

On Jul 23, 2020 17:50, Owen Nichols  wrote:
Now that geode-examples' default branch has been changed to develop, and 
nothing further has been added to this discussion in a while, would anyone like 
to call for a vote to eliminate master branch from all geode projects?

I would suggest holding this vote under 'code modification' rules[1] since we 
would be deleting code.  Even though master should be substantially equivalent 
to latest release tag (currently rel/v1.12.0), git diff shows a few small 
differences.

[1] a timeframe of at least 72 hours and a single -1 can veto, see 
https://www.apache.org/foundation/voting.html

On 7/7/20, 6:33 PM, "Owen Nichols"  wrote:

Since the branch proposed for deletion is the default branch in 
geode-examples, you will need to file an ASF INFRA ticket to change that 
default.  This is a great discussion thread, but ASF will require a [VOTE] 
thread to be cited.

I am concerned about keeping it easy for someone who has just cloned geode 
to identify the most stable branch for their purpose.  Before, they could 
always be assured `git checkout master` would give the flagship release.  Now, 
new users will be immediately forced into some daunting detective work to sift 
through hundreds of haphazard tags and branches (a task even veteran committers 
frequently fail).  I would strongly encourage an aggressive cleanup of 
unhelpful branches and tags, as Jacob proposed last month, before getting rid 
of the latest_release concept.

On 7/7/20, 8:24 AM, "Blake Bender"  wrote:

Just to follow up on this: I've filed GEODE-8335 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8335data=02%7C01%7Crhoughton%40vmware.com%7Ce8af678cedaa48c3f19408d82f6b846d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637311486111409381sdata=K%2Fs68rCpCgsc6Duzey9Iodf9gdUyE0AMAfGFzr2y4VA%3Dreserved=0)
 to track, respectfully (cowardly __ ) deferring to individuals who regularly 
contribute to the various Geode repos to handle it as they see fit.  I'll take 
care of the several Geode Native associated repos.

Thanks,

Blake


On 6/26/20, 12:21 PM, "Dave Barnes"  wrote:

+1 if we can override git’s ‘master’ default and establish 
‘develop’ in its place. Otherwise renaming to ‘main’ would solve the problem 
with the negative connotations.

NB: Mark, I think the 3-vote convention is just for back-ports to 
the release-branch. I don’t think of it as applying to a general discussion 
like this one.

> On Jun 26, 2020, at 9:42 AM, Anthony Baker  
wrote:
>
> By just do it, I assume you mean:
>
> - Contact delete master where not needed
> - Rename master to main when needed
> - Contact INFRA to change the default branch
> - Update README and CI jobs as needed
>
> Across *all* geode repos.
>
>
> Anthony
>
>
>> On Jun 26, 2020, at 9:38 AM, Mark Hanson  
wrote:
>>
>> +1 to delete. No good reason to keep it that I know of. I think 
I am the third +1 with no -1s so just do it.
>>
>> Thanks,
>> Mark
>>
>>> On Jun 26, 2020, at 9:13 AM, Alexander Murmann 
 wrote:
>>>
>>> +1 to deleting. Given we tag everything properly on the other 
branches, I
>>> don't see the need for it either.
>>>
>>> On Fri, Jun 26, 2020 at 9:03 AM Alberto Bustamante Reyes
>>>  wrote:
>>>
 +1 for deleting master branch. An also for updating the wiki 
page about
 branching that Alberto pointed out.
 
 De: Bruce Schuchardt 
 Enviado: viernes, 26 de junio de 2020 17:37
 Para: dev@geode.apache.org 
 Asunto: Re: Fate of master branch

 Let's just delete it.  I need to do that in my own repos as 
well.

 On 6/26/20, 8:05 AM, "Blake Bender"  wrote:

  Apologies if this has been addressed already and I missed it. 
 In
 keeping with other OSS projects, I believe it’s time we did 
something about
 removing the insensitive term master from Geode repositories.

  One choice a lot of projects appear to be going with is a 
simple
 rename from master • main.  In our own case, however, master 
isn’t really
 in use for anything vital.  We track releases with a tag and a 
branch to
 backport fixes to, and the develop branch is the “source of 
truth”
 

Re: about Liberica JDK

[Robert Houghton]

Only partially tongue-in-cheek, but isn’t the promise of the JDK, that it is 
the same everywhere, on all versions, builds, and platforms?

From: Kirk Lund 
Date: Wednesday, July 8, 2020 at 10:23 AM
To: dev@geode.apache.org 
Subject: Re: about Liberica JDK
In the future, I think we should discuss and propose changing the JDK on
this dev-list before making the changes.

On Tue, Jul 7, 2020 at 9:01 AM Anthony Baker  wrote:

> Liberica is an OpenJDK distribution like AdoptOpenJDK, Oracle, RedHat,
> Amazon, Azul, etc.  I have yet to find a behavioral difference between the
> distributions—it’s mostly about ease of acquiring binaries and LTS support.
> Just for simplicity I would prefer to use a single JDK distribution across
> versions in our CI pipelines.
>
>
> Anthony
>
>
> > On Jul 7, 2020, at 2:21 AM, Alberto Bustamante Reyes
>  wrote:
> >
> > Hi devs,
> >
> > I have seen in develop branch this commit that changes openjdk by
> Liberica JDK (
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5312data=02%7C01%7Crhoughton%40vmware.com%7C3b7d69b430bc4bd4faf408d823639d41%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637298258028125419sdata=SH3ASp6W1EjfWUccN5ssm1EmNihh9T8FK%2F1NsHxmS7k%3Dreserved=0
> ), although it was reverted later so I suppose there are still issues to be
> solved.
> >
> > I didn't know Liberica and I'm curious about the change. Why is this
> change being implemented?
> >
> > BR/
> >
> > Alberto B.
> >
>
>

Re: Proposal to bring GEODE-8315 (shiro upgrade) to support branches

[Robert Houghton]

+1

From: Dick Cavender 
Date: Tuesday, June 30, 2020 at 9:14 AM
To: dev@geode.apache.org 
Subject: RE: Proposal to bring GEODE-8315 (shiro upgrade) to support branches
+1

-Original Message-
From: Ju@N 
Sent: Tuesday, June 30, 2020 9:12 AM
To: dev@geode.apache.org
Subject: Re: Proposal to bring GEODE-8315 (shiro upgrade) to support branches

+1

On Tue, 30 Jun 2020 at 17:03, Owen Nichols  wrote:

> Recently shiro-1.5.2.jar is getting flagged for critical security
> vulnerability CVE-2020-11989.
>
> Analysis shows that Geode does not use Shiro in a manner that would
> expose this vulnerability.
>
> The risk of bringing GEODE-8315 is very low (difference between Shiro
> 1.5.2 and 1.5.3 is bugfix only).  GEODE-8315 has been on develop for 2
> days and passed the pipeline.
>
> This fix is critical to avoid false positives in automated
> vulnerability scans, so it would be nice to bring before 1.13.0 release.
>


--
Ju@N

Re: Docker on Windows

[Robert Houghton]

@Jacob Barrett<mailto:jabarr...@vmware.com> @Dale 
Emery<mailto:dem...@vmware.com> I am excited to see progress on this. What I do 
not know, is what Junit5 buys us in terms of test isolation and parallelism 
compared to what we have now.

From: Jacob Barrett 
Date: Monday, June 29, 2020 at 11:16 AM
To: dev@geode.apache.org 
Subject: Re: Docker on Windows
Awesome! I will take a quick stab at rebasing the changes and see if anything 
has improved.

I will figure out a good place to setup a shared branch for collaboration.

-Jake

On Jun 29, 2020, at 11:06 AM, Dale Emery 
mailto:dem...@vmware.com>> wrote:

Here are my notes from my most recent attempts:

• November
• Added JUnit 5 to geode-junit API
• Configured geode-junit and geode-core to run all tests via JUnit 5
• CI failures on JDK 11
• NPE thrown apparently by Gradle
• December
• Ran tests in JDK 11 on my Mac
• Failures do not occur on my Mac
• Set gradle’s log level to info
• Results in CI logs so enormous that Chrome cannot display them in a bearable 
timeframe
• Then the whole PR CI pipeline became unusable
• So I was able to gain no further information about the cause of the gradle 
NPEs

Here is the branch the way I left things: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdemery-pivotal%2Fgeode%2Ftree%2Fold%2Fgeode-junit5data=02%7C01%7Crhoughton%40vmware.com%7C09249e9ba58844c7563308d81c588743%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637290513849574069sdata=fPzImBwZ2zivCSdP6mPCGBFQNrpaahQuhZNpP3I4%2BIY%3Dreserved=0

About the PR CI pipeline becoming unusable… I don’t have any notes about what 
“unusable” means. I suspect it was unrelated to my PR, but I’m not sure.

Cheers,
Dale


On Jun 29, 2020, at 10:51 AM, Jacob Barrett 
mailto:jabarr...@vmware.com><mailto:jabarr...@vmware.com>>
 wrote:

Dale,

Sorry I thought it was Kirk. Do you have a branch somewhere with your work so 
far? Can you refresh us all on the issues you hit and what you think the next 
steps would be?

Thanks,
Jake


On Jun 29, 2020, at 10:23 AM, Kirk Lund 
mailto:kl...@apache.org><mailto:kl...@apache.org>> wrote:

It was Dale who worked on migrating Geode to use JUnit 5. I know he ran
into some issues but I don't recall what they were. I'm definitely up for
helping on the JUnit 5 front!

On Fri, Jun 26, 2020 at 11:30 AM Jacob Barrett 
mailto:jabarr...@vmware.com><mailto:jabarr...@vmware.com>>
 wrote:

If the effort to do both is less than the sum of each individually then I
say lets do it.

Kirk, I recall you putting some effort into JUnit 5 at some point.

-Jake


On Jun 26, 2020, at 11:13 AM, Jens Deppe 
mailto:jde...@vmware.com><mailto:jde...@vmware.com>> wrote:

A bigger effort (but I think more correct and sustainable) would be to
switch to junit 5 where something like this could more easily be
implemented.

--Jens

On 6/26/20, 9:11 AM, "Robert Houghton" 
mailto:rhough...@vmware.com><mailto:rhough...@vmware.com>>
 wrote:

The plugin code that spawns junit test workers on containers needs
some serious help. Aside from the benefit we would get on windows, we also
are blocked on getting to the next major version of Gradle with the current
tool. I really think it might be easier to write our own Gradle plugin at
this point.


From: Jacob Barrett 
mailto:jabarr...@vmware.com><mailto:jabarr...@vmware.com>>
Date: Thursday, June 25, 2020 at 12:26 PM
To: 
dev@geode.apache.org<mailto:dev@geode.apache.org><mailto:dev@geode.apache.org> 
mailto:dev@geode.apache.org><mailto:dev@geode.apache.org>>
Subject: Docker on Windows

On Jun 25, 2020, at 12:23 PM, Jens Deppe 
mailto:jde...@vmware.com><mailto:jde...@vmware.com>> wrote:
It's been a couple of years since Sai and I tried (but failed) to
dockerize the tests. I'm sure docker support has improved and it might be
worth trying that again.

Docker on windows has improved a lot but wasn’t the major issue the
docker plugin for Gradle needed some serious work?

I have recently been experimenting with the Docker/Kubernetes for
Windows experience. Perhaps we can take another stab at this issue.

-Jake





—
Dale Emery
dem...@vmware.com<mailto:dem...@vmware.com><mailto:dem...@vmware.com<mailto:dem...@vmware.com%3e%3cmailto:dem...@vmware.com>>

Re: Docker on Windows

[Robert Houghton]

The plugin code that spawns junit test workers on containers needs some serious 
help. Aside from the benefit we would get on windows, we also are blocked on 
getting to the next major version of Gradle with the current tool. I really 
think it might be easier to write our own Gradle plugin at this point.


From: Jacob Barrett 
Date: Thursday, June 25, 2020 at 12:26 PM
To: dev@geode.apache.org 
Subject: Docker on Windows

> On Jun 25, 2020, at 12:23 PM, Jens Deppe  wrote:
> It's been a couple of years since Sai and I tried (but failed) to dockerize 
> the tests. I'm sure docker support has improved and it might be worth trying 
> that again.

Docker on windows has improved a lot but wasn’t the major issue the docker 
plugin for Gradle needed some serious work?

I have recently been experimenting with the Docker/Kubernetes for Windows 
experience. Perhaps we can take another stab at this issue.

-Jake

Re: Is "java" available to acceptanceTests?

[Robert Houghton]

The images used for these tests do have Java installed, but not on PATH. You 
need to pick which $JAVA_HOME (jdk8 or jdk11) you wish to run with. These 
values are set on the Gradle invocation line for the test type, depending on 
platform/job-name.

From: Kirk Lund 
Date: Saturday, June 6, 2020 at 5:18 AM
To: dev@geode.apache.org 
Subject: Is "java" available to acceptanceTests?
Does the image(s) we use for running acceptanceTest include "java"? I've
written some new AcceptanceTests for the LocatorLauncher and ServerLauncher
which pass locally but fail in precheckin in the cloud with
*java.io.IOException:
Cannot run program "java"*.

> Task :geode-assembly:acceptanceTest

org.apache.geode.launchers.LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest
> locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins FAILED
java.io.IOException: Cannot run program "java" (in directory
"/tmp/junit5795014123591460975"): error=2, No such file or directory
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1128)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1071)
at
org.apache.geode.launchers.LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins(LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java:164)

Caused by:
java.io.IOException: error=2, No such file or directory
at java.lang.ProcessImpl.forkAndExec(Native Method)
at java.lang.ProcessImpl.(ProcessImpl.java:340)
at java.lang.ProcessImpl.start(ProcessImpl.java:271)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1107)
... 2 more

Re: LGTM check failed

[Robert Houghton]

I’m looking at the logs, and doing some digging.

From: Mario Kevo 
Date: Friday, May 29, 2020 at 3:42 AM
To: dev@geode.apache.org 
Subject: LGTM check failed
Hi all,

LGTM analysis: Java check failed for last six opened PRs.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5182data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565949790sdata=iyGw1RZmwBahoKqOJKwuQYPoXaXXVbQUwfcFueIE%2F6o%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5181data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565959783sdata=uA2xsWRzyZrHENilhuk5JHt2jS1ipJMiab%2BGrIGbqyA%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5180data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565959783sdata=bTNodLY9BJLA58zXS4TRPcd7PNFbgyCUoShsDTkM90M%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5179data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565959783sdata=O2sQjP6w4DEI%2BBH4M8GtOGU8JhJn54Yu1Kon6hVNpH0%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5176data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565959783sdata=qsSWsMRHzz59XnrP13qXMvTSbFliZCt1yAtyZTPJB%2Fg%3Dreserved=0
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5175data=02%7C01%7Crhoughton%40vmware.com%7C61c908bdfa0644b8333c08d803bcff8b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637263457565959783sdata=6ZDC8jIlnUINz3n24uZK2CPlnRvHVojXybqqVdmpg%2Fk%3Dreserved=0

[2020-05-29 00:08:56] [analysis] [EVALUATION 115/177] [FAIL] Error running 
query semmlecode-queries/Security/CWE/CWE-022/TaintedPath.ql: OutOfMemory
Query evaluation ran out of memory (maximum allowed memory: 3012MB).

I take a look on the last few merged commit but don't think that they caused 
this failure.
Please can someone, who is more familiar with this, take a look to see if it is 
problem with this check?

BR,
Mario

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

[Robert Houghton]

INFRA has added the status to the required list for merging PRs to
geode/develop. Thanks everyone.

On Fri, May 15, 2020 at 3:03 PM Robert Houghton 
wrote:

> Please refer to https://issues.apache.org/jira/browse/INFRA-20270
>
> On Fri, May 15, 2020 at 2:58 PM Robert Houghton 
> wrote:
>
>> Support seems to be unanimously good, and folks voted in this thread. I'm
>> going to make the ASF-INFRA request.
>>
>> On Fri, May 15, 2020 at 11:47 AM Anthony Baker  wrote:
>>
>>> Barry and I tossed up a draft PR to fix a problem in session state
>>> replication with Tomcat.  If we can get this completed I’d like to include
>>> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
>>> after 9.0.21.
>>>
>>> Anthony
>>>
>>>
>>> > On May 15, 2020, at 1:27 AM, Ju@N  wrote:
>>> >
>>> > +1
>>> >
>>> > On Thu, 14 May 2020 at 22:19, Mark Hanson  wrote:
>>> >
>>> >> +1. The more we can automate this types of checks the better.
>>> >>
>>> >> Thanks,
>>> >> Mark
>>> >>
>>> >>
>>> >
>>> > --
>>> > Ju@N
>>>
>>>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

[Robert Houghton]

Please refer to https://issues.apache.org/jira/browse/INFRA-20270

On Fri, May 15, 2020 at 2:58 PM Robert Houghton 
wrote:

> Support seems to be unanimously good, and folks voted in this thread. I'm
> going to make the ASF-INFRA request.
>
> On Fri, May 15, 2020 at 11:47 AM Anthony Baker  wrote:
>
>> Barry and I tossed up a draft PR to fix a problem in session state
>> replication with Tomcat.  If we can get this completed I’d like to include
>> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
>> after 9.0.21.
>>
>> Anthony
>>
>>
>> > On May 15, 2020, at 1:27 AM, Ju@N  wrote:
>> >
>> > +1
>> >
>> > On Thu, 14 May 2020 at 22:19, Mark Hanson  wrote:
>> >
>> >> +1. The more we can automate this types of checks the better.
>> >>
>> >> Thanks,
>> >> Mark
>> >>
>> >>
>> >
>> > --
>> > Ju@N
>>
>>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

[Robert Houghton]

Support seems to be unanimously good, and folks voted in this thread. I'm
going to make the ASF-INFRA request.

On Fri, May 15, 2020 at 11:47 AM Anthony Baker  wrote:

> Barry and I tossed up a draft PR to fix a problem in session state
> replication with Tomcat.  If we can get this completed I’d like to include
> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
> after 9.0.21.
>
> Anthony
>
>
> > On May 15, 2020, at 1:27 AM, Ju@N  wrote:
> >
> > +1
> >
> > On Thu, 14 May 2020 at 22:19, Mark Hanson  wrote:
> >
> >> +1. The more we can automate this types of checks the better.
> >>
> >> Thanks,
> >> Mark
> >>
> >>
> >
> > --
> > Ju@N
>
>

Re: Update of SSLParameterExtension interface

[Robert Houghton]

There is an opportunity to take @Jacob Barrett s
advice on using a real configuration class instead of an opaque
'Properties' class here. See https://github.com/apache/geode/pull/5115,
where we test the API breaking change with an exemption file.

On Thu, May 7, 2020 at 8:52 AM Jacob Barrett  wrote:

> I have some concerns with using Properties in public APIs. The use of
> Properties is not strongly typed. I can’ tell from one property to the next
> what the type is. I can’t get compile time errors is the type is wrong. I
> don’t know what goes into a Property based on the interface or any magic
> and IDE could produce. I can’t get compile time failures because I am
> missing or using an invalid property. The Property object is synchronized,
> so I am using this object to get values frequently I am not serializing all
> threads through this object.
>
> Let’s take this time, where we are already fixing a broken API to do it
> right. Build into the API a configuration class that has what we think we
> need right now. We can add to that class over time as needed.
>
> -Jake
>
>
> > On May 4, 2020, at 12:00 PM, Bruce Schuchardt 
> wrote:
> >
> > I guess that would have to go into the 1.13 branch as well.  This
> changes the public API but I think we should do it.  The current API isn't
> usable since it refers to a non-public interface.
> >
> > On 5/4/20, 9:31 AM, "Mario Ivanac"  wrote:
> >
> >Hi all,
> >
> >after comments that SSLParameterExtension interface has an init()
> method that takes a DistributionConfig as an argument (which is internal
> class),
> >new solution is proposed to replace DistributionConfig with
> Properties.
> >
> >
> >New PR is created with new proposal
> https://github.com/apache/geode/pull/5040,
> >and also RFC is updated with new proposal
> https://cwiki.apache.org/confluence/display/GEODE/Introduction+of+SSL+Parameter+Extension
> >Introduction of SSL Parameter Extension - Geode - Apache Software
> Foundation<
> https://cwiki.apache.org/confluence/display/GEODE/Introduction+of+SSL+Parameter+Extension
> >
> >Hit enter to search. Help. Online Help Keyboard Shortcuts Feed
> Builder What’s new
> >cwiki.apache.org
> >
> >Should we cherry-picked this PR into the support/1.12 branch?
> >
> >Regards,
> >Mario
> >
> >
> >
>
>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

[Robert Houghton]

@donal There is a violation filter in place for major version changes that
would currently allow any change. That can be narrowed down to only allow
deletion of already-deprecated signatures.

On Wed, May 13, 2020 at 5:18 PM Owen Nichols  wrote:

> If ApiChecker fails on your PR, and you merge anyway, it will fail on
> develop, so there’s no question it should be a required PR check.
>
> We left some checks optional to avoid blocking merges on unrelated flaky
> failures.  ApiChecker should never be flaky.
>
> If/when Geode decides it’s time for a 2.0 release, which would allow
> breaking backward compatibility, we should be able to provide a rule to
> allow that in the ApiChecker.
>
> > On May 13, 2020, at 3:49 PM, Donal Evans  wrote:
> >
> > Given that this job takes less than 10 minutes to run, pass or fail, I
> > don't see it adding any additional friction to the PR process in terms of
> > having to wait for things to finish. I am curious if there are any
> > circumstances we could envisage where skipping or bypassing this check
> > would be warranted though, and what the procedure would be in those
> cases.
> > For example, how would it handle the removal of a deprecated method from
> an
> > API? It's not something that happens often, but it will happen eventually
> > (I would hope).
> >
> > On Wed, May 13, 2020 at 2:32 PM Robert Houghton 
> > wrote:
> >
> >> We have enabled this job on the develop and support 1.13 branches, and
> it
> >> is going well. I would like this to be a blocking job for our pull
> >> requests.
> >>
> >> Are there any issues around this test that we want to address, or
> reasons
> >> to *not* have it be a blocking job in the PR pipeline?
> >>
> >> To seed the conversation, there is an issue I am working on with @Mario
> >> Ivanac  regarding exemptions to the Gradle task.
> >>
> >> I would like to have a [VOTE] by end of week on this.
> >>
>
>

[DISCUSS] enable GitHub PR blocking on API breaking changes

[Robert Houghton]

We have enabled this job on the develop and support 1.13 branches, and it
is going well. I would like this to be a blocking job for our pull
requests.

Are there any issues around this test that we want to address, or reasons
to *not* have it be a blocking job in the PR pipeline?

To seed the conversation, there is an issue I am working on with @Mario
Ivanac  regarding exemptions to the Gradle task.

I would like to have a [VOTE] by end of week on this.

Re: [PROPOSAL] Merge PR 5103 (GEODE-8083) to support/1.13

[Robert Houghton]

Thanks all.

On Wed, May 13, 2020 at 8:34 AM Joris Melchior  wrote:

> +1
> 
> From: Robert Houghton 
> Sent: May 12, 2020 18:57
> To: dev@geode.apache.org ; Dave Barnes (Pivotal) <
> dbar...@pivotal.io>
> Subject: [PROPOSAL] Merge PR 5103 (GEODE-8083) to support/1.13
>
> This is a squash of GEODE-8083 and 8087, to bring the Java  API comparison
> jobs from Gradle and Concourse to the support branch. Currently runs
> cleanly from my terminal, and has been green on develop for a week or so.
>
> @Dave Barnes  , as release manager, what say you?
>

[PROPOSAL] Merge PR 5103 (GEODE-8083) to support/1.13

[Robert Houghton]

This is a squash of GEODE-8083 and 8087, to bring the Java  API comparison
jobs from Gradle and Concourse to the support branch. Currently runs
cleanly from my terminal, and has been green on develop for a week or so.

@Dave Barnes  , as release manager, what say you?

Re: Proposal to bring GEODE-8016 to support branches

[Robert Houghton]

Yes please! +1

On Mon, May 11, 2020 at 2:46 AM Darrel Schneider 
wrote:

> +1
>
> On Sun, May 10, 2020 at 10:05 PM Dick Cavender 
> wrote:
>
> > +1
> >
> > On Sat, May 9, 2020 at 6:42 PM Owen Nichols  wrote:
> >
> > > Last week develop was successfully migrated from publishing -SNAPSHOT
> to
> > > publishing -build.nnn, as discussed on the dev list.
> > >
> > > I propose that we bring the same change to support/1.13 and
> support/1.12
> > > for consistency.  This will allow downstream projects to change over
> the
> > > new model “everywhere" rather than having to maintain support for both
> > ways
> > > and constantly try to remember which branches do it which way.
> > >
> > > The specific changes to be cherry-picked from develop are a4c8b <
> > >
> >
> https://github.com/apache/geode/commit/a4c8b9ed8bbea584f798164fa5308d236e9b6048
> > >
> > > and 39c52 <
> > >
> >
> https://github.com/apache/geode/commit/39c522e340196cb30d55d81d93c63028938cd782
> > >.
> > > I have prepared PR #5089 
> for
> > > support/1.13 and PR #5090 
> > for
> > > support/1.12 due to the version number differences on each branch.
> >
>

Re: Discussion - Change Public API Before Initial Release

[Robert Houghton]

+1

On Fri, May 8, 2020, 18:26 Jacob Barrett  wrote:

> Hey Ya’ll,
>
> We have a new API going into 1.13 that has an inconsistency I want to
> address before we are stuck with it. The class SniSocketFactory should be
> renamed SniProxySocketFactory. The class in question produces objects of
> type SniProxySocket. An "SNI socket" isn’t a thing but an SNI proxy is a
> thing. The factory class that produces proxy socket factories (yes a meta
> factory) ProxySocketFactories uses the “ProxySocket” name as well. It fells
> very inconsistent with all the other classes that make up with API for
> SniSocketFactory to not have a proxy in it and be called
> SniProxySocketFactory.
>
> To be very clear, this API has not made it out of development yet but is
> in the 1.13 release branch. Can I get a few thumbs up to open an ticket and
> pick it into the 1.13 release branch before it goes out?
>
> Thanks,
> Jake
>
>

API check results

[Robert Houghton]

Hello all,

This is a call for help.

We have recently enabled an API check job on pull requests on Concourse.
You have likely seen it, since it is failing while comparing changes on
develop (not yet on support/1.13) to our latest release, v1.12.0.

I have configuration changes to the tool itself to fix most of the reported
errors, but am stuck on what to do for
'org.apache.geode.net.SSLParameterExtension', which has seen a change to
its default and signatures. Should I add an exemption to this particular
class since it is new? Do we need to revert the change for the sake of 1.13
and 1.12 compatibility?

Thanks for any guidance,
-Robert Houghton

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

I have no current plans on backporting, though it might make the continued
Concourse testing of those branches easier.

There is a PR about this available:
https://github.com/apache/geode/pull/5057

On Thu, Apr 30, 2020 at 9:25 PM Owen Nichols  wrote:

> It sounds like the only impact is that downstream projects that consume
> -SNAPSHOT builds will need to make one small change to their maven config;
> downstream projects that consume release versions will be unaffected.  So
> just let us know when to make that change.
>
> Anytime soon after support/1.13 is cut would be great to bring this change
> to develop.  Assuming it goes smoothly on develop, would you envision
> backporting to support/1.13 and support/1.12 as well?
>
> > On Apr 30, 2020, at 8:38 PM, Robert Houghton 
> wrote:
> >
> > I don't read any strong negatives to this change. I would like to start
> > polishing a PR on this work, unless anyone has strong opposing feelings.
> > Does anyone feel that this change requires a VOTE?
> >
> > On Tue, Apr 28, 2020 at 10:36 AM Jacob Barrett 
> wrote:
> >
> >> Probably for a separate discussion but I would opt for a future where
> the
> >> release manager validates and signs a specific build from produced by
> the
> >> CI. This takes a lot of error away in the what the release manager
> >> currently does, which has resulted in incorrect artifacts being voted
> on.
> >>
> >> In this model the release manager would be certifying a release with
> full
> >> version number, or whatever we choose for the release version number.
> >>
> >> Until then the release manager could just use the short number. In
> >> Gradle/Maven versioning a version without qualifier is always newer than
> >> one with. So 1.13.0 is newer than any 1.13.0-build.123.
> >>
> >> -Jake
> >>
> >>
> >>> On Apr 28, 2020, at 10:24 AM, Anthony Baker  wrote:
> >>>
> >>> Note that I’m asking about building on my local system.  Will the
> >> version listed in gradle.properties use the full 1.13.0-build.123
> syntax?
> >> How would it get bumped?
> >>>
> >>> Would a release manager end up using just 1.13.0?  Hoping for yes :-)
> >>>
> >>> Anthony
> >>>
> >>>> On Apr 28, 2020, at 9:24 AM, Robert Houghton 
> >> wrote:
> >>>>
> >>>> @anthony
> >>>> /develop would say 1.13.0-build.230 (as of this morning).
> >>>> Once cut, /release/1.13 would say 1.13.0-build.231, and /develop would
> >>>> switch to 1.14.0-build.1.
> >>>>
> >>>> gfsh would return that full value. That is the artifact version.
> >>>>
> >>>> On Mon, Apr 27, 2020 at 4:38 PM Anthony Baker 
> >> wrote:
> >>>>
> >>>>> If I build the /develop branch and run `gfsh version` what will it
> >> print?
> >>>>>
> >>>>> If I build the soon-to-be /release/1.13 branch and run `gfsh version`
> >> what
> >>>>> will it print?
> >>>>>
> >>>>> Anthony
> >>>>>
> >>>>>
> >>>>> On 4/27/20, 4:32 PM, "Robert Houghton" 
> wrote:
> >>>>>
> >>>>>  The artifact would change from "1.13.0-SNAPSHOT" to
> >> "1.13.0-build.123".
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

I don't read any strong negatives to this change. I would like to start
polishing a PR on this work, unless anyone has strong opposing feelings.
Does anyone feel that this change requires a VOTE?

On Tue, Apr 28, 2020 at 10:36 AM Jacob Barrett  wrote:

> Probably for a separate discussion but I would opt for a future where the
> release manager validates and signs a specific build from produced by the
> CI. This takes a lot of error away in the what the release manager
> currently does, which has resulted in incorrect artifacts being voted on.
>
> In this model the release manager would be certifying a release with full
> version number, or whatever we choose for the release version number.
>
> Until then the release manager could just use the short number. In
> Gradle/Maven versioning a version without qualifier is always newer than
> one with. So 1.13.0 is newer than any 1.13.0-build.123.
>
> -Jake
>
>
> > On Apr 28, 2020, at 10:24 AM, Anthony Baker  wrote:
> >
> > Note that I’m asking about building on my local system.  Will the
> version listed in gradle.properties use the full 1.13.0-build.123 syntax?
> How would it get bumped?
> >
> > Would a release manager end up using just 1.13.0?  Hoping for yes :-)
> >
> > Anthony
> >
> >> On Apr 28, 2020, at 9:24 AM, Robert Houghton 
> wrote:
> >>
> >> @anthony
> >> /develop would say 1.13.0-build.230 (as of this morning).
> >> Once cut, /release/1.13 would say 1.13.0-build.231, and /develop would
> >> switch to 1.14.0-build.1.
> >>
> >> gfsh would return that full value. That is the artifact version.
> >>
> >> On Mon, Apr 27, 2020 at 4:38 PM Anthony Baker 
> wrote:
> >>
> >>> If I build the /develop branch and run `gfsh version` what will it
> print?
> >>>
> >>> If I build the soon-to-be /release/1.13 branch and run `gfsh version`
> what
> >>> will it print?
> >>>
> >>> Anthony
> >>>
> >>>
> >>> On 4/27/20, 4:32 PM, "Robert Houghton"  wrote:
> >>>
> >>>   The artifact would change from "1.13.0-SNAPSHOT" to
> "1.13.0-build.123".
> >>>
> >>>
> >>>
> >>>
> >>>
> >
>
>

Re: [DISCUSS] etiquette around breaking the pipeline

[Robert Houghton]

I am very pro-revert for breaking changes. The Benchmark or Windows tests
being a culprit is unfortunate, since the PR pipeline cannot tell us about
those, but thats the cost of our excellence :)

On Wed, Apr 29, 2020 at 4:06 PM Owen Nichols  wrote:

> There are many ways a commit can break the Geode CI pipeline[1] despite
> our required PR checks, such as:
> - merge a PR that failed some non-required PR checks
> - merge a PR that last ran checks awhile ago and now interacts
> unexpectedly with other recent changes
> - merge a PR that fails Windows tests
> - merge a PR that fails Benchmarks tests
>
> When a bad commit gets through for whatever reason, what should happen
> next?  For example:
> - bring it up on the dev list
> - someone should revert the change as soon as possible, allowing the
> pipeline to remain green while the issue is addressed
> - a reasonable amount of time should be allowed for someone to make a
> quick fix, otherwise revert.
> - the pipeline should remain broken for as long as it takes to fix, as
> long as there is clear communication that someone is working on it
> - everyone look the other way and hope it fixes itself
>
> I’m sure there are other ideas as well.  A related question is *who* can
> or should revert a bad commit?  Only the person that merged the PR?  Only
> the original author of the PR?  The first person to notice the problem?
>
> What is a reasonable amount of time for this to happen?  2 hours? 2 days?
> 2 weeks? Does it depend whether the bad commit is also affecting PR checks
> for everyone else vs only depriving downstream consumers of new Geode
> -SNAPSHOTs?
>
> Would you take offense if someone else reverted your commit?  Does is make
> a difference if your commit is exposing an existing issue (as opposed to
> introducing a new bug)?
>
> Is there a perception that reverts create a lot of extra work? (they
> shouldn’t--just start your rework PR with a revert of the revert, then add
> additional commits that resolve the issue, so reviewers can easily see what
> was missing the first time)
>
> This is a discussion thread, not a vote.  We trust committers to do what’s
> best.  Would embracing a “anyone can revert, no shame”
> revert-first-then-fix culture benefit our community, or is our current
> easygoing approach working just fine?
>
> [1]
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main

Re: geodeOldVersionInstalls

[Robert Houghton]

Pretty sure it is generated by Gradle in geode-assembly

On Tue, Apr 28, 2020, 16:09 Kirk Lund  wrote:

> Does anyone know how geodeOldVersionInstalls.txt gets created?
>
> When I try to run
> TomcatSessionBackwardsCompatibilityTomcat8WithOldModuleCanDoPutsTest
> locally, it fails with the following stack trace.
>
> I've searched for geodeOldVersionInstalls.txt and grepped for code that
> involves the file, but I can't figure out how this file gets created or how
> to run that test.
>
> Anyone have ideas how to debug this test or how to
> ensure geodeOldVersionInstalls.txt gets created?
>
> java.lang.InternalError: VersionManager: unable to locate
> geodeOldVersionInstalls.txt in class-path
>
> at
>
> org.apache.geode.test.version.VersionManager.checkForLoadFailure(VersionManager.java:150)
> at
>
> org.apache.geode.test.version.VersionManager.getVersionsWithoutCurrent(VersionManager.java:141)
> at
>
> org.apache.geode.session.tests.TomcatSessionBackwardsCompatibilityTestBase.data(TomcatSessionBackwardsCompatibilityTestBase.java:55)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
>
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
>
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at
>
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
> at
>
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> at
>
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
> at org.junit.runners.Parameterized.allParameters(Parameterized.java:280)
> at org.junit.runners.Parameterized.(Parameterized.java:248)
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
> at
>
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at
>
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
> at
>
> org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
> at
>
> org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
> at
>
> org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
> at
>
> org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
> at
>
> org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
> at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:33)
> at
>
> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:49)
> at
>
> com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
> at
>
> com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
> at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

@anthony
/develop would say 1.13.0-build.230 (as of this morning).
Once cut, /release/1.13 would say 1.13.0-build.231, and /develop would
switch to 1.14.0-build.1.

gfsh would return that full value. That is the artifact version.

On Mon, Apr 27, 2020 at 4:38 PM Anthony Baker  wrote:

> If I build the /develop branch and run `gfsh version` what will it print?
>
> If I build the soon-to-be /release/1.13 branch and run `gfsh version` what
> will it print?
>
> Anthony
>
>
> On 4/27/20, 4:32 PM, "Robert Houghton"  wrote:
>
> The artifact would change from "1.13.0-SNAPSHOT" to "1.13.0-build.123".
>
>
>
>
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

@donal I do not have an example of a Java project that builds using a
Gradle multi-module build, that cares as much as we do about re-running CI
tests with identical inputs. I wish I did, I would crib from their answer
sheet without shame or remorse.

On Mon, Apr 27, 2020 at 4:37 PM Donal Evans  wrote:

> Do we know if this is an issue that other open source projects have dealt
> with? And if so, is this proposed solution similar to what they might have
> done to remedy it?
> ____
> From: Robert Houghton 
> Sent: Monday, April 27, 2020 4:31 PM
> To: dev@geode.apache.org 
> Subject: Re: [DISCUSS] Publish Builds, not Snapshots
>
> The artifact would change from "1.13.0-SNAPSHOT" to "1.13.0-build.123".
>
> The number after the "build" slug is auto-incremented by our CI system
> anyway, as the "geode-build-version" semver resource. We are actually doing
> *more* work in Gradle to truncate that number from the current "SNAPSHOT"
> value.
>
> On Mon, Apr 27, 2020 at 3:41 PM Anthony Baker  wrote:
>
> > @Robert, can you show some examples of what the build number would be
> > under this proposal?  Does 1.13.0-SNAPSHOT become 1.13.0.N where N
> > increments every build?
> >
> > Seems reasonable.  Since the consumers of pre-release artifacts are
> either
> > a) this project or b) close related projects for
> > integration-testing-purposes-only I’m not super worried about the ugly
> > syntax.
> >
> >
> > Anthony
> >
> >
> > > On Apr 27, 2020, at 3:25 PM, Jacob Barrett 
> wrote:
> > >
> > > It is unfortunate that the Maven/Gradle community hasn’t addressed this
> > glaring issue with SNAPSHOT for decades now (well maybe not decades but
> > certainly decade). It is also unfortunate that the Maven version
> coordinate
> > is ugly. Aside from that I am totally onboard. Yay for reproducible
> builds
> > and predictable downstream builds!
> > >
> > > With SNAPSHOTS in a repo the repository automatically prunes back old
> > builds. Do we have any concerns about having a plethora of builds filling
> > up this new pre-release repository?
> > >
> > > -Jake
> > >
> > >> On Apr 27, 2020, at 3:21 PM, Robert Houghton 
> > wrote:
> > >>
> > >> Hello to the community,
> > >>
> > >> tl;dr - Lets publish builds, not snapshots, for repeatable CI builds,
> as
> > >> GEODE-8016[1]. Communicate desired artifact version via the existing
> > >> 'UpdatePassingTokens' job.
> > >>
> > >> I have been working on the Geode build and CI systems for a long time,
> > and
> > >> it has irked me that the geode-examples pipeline[2] does not build and
> > test
> > >> against the latest artifacts from the develop pipeline. Some work has
> > been
> > >> done already to allow this via "composite" builds for local testing
> > without
> > >> needing to publish Geode to your local Maven repository.
> > >>
> > >> From a Concourse CI perspective, composite builds are costly due to
> the
> > >> rebuild of the upstream artifacts. They allow repeatable builds, but
> > only
> > >> by rebuilding those dependencies. Better would be to point to upstream
> > >> artifacts as concrete build versions. SNAPSHOT builds can and do roll
> > >> (invisibly) as new versions are published. Discrete, numbered builds
> do
> > >> not. Downstream consumers can use greedy version specifiers to get
> their
> > >> current behavior of "latest".
> > >>
> > >> Gradle: 'org.apache.geode:geode-core:1.13.0+'
> > >> Maven: 'org.apache.geode'
> > >>'geode-core'
> > >>'[1.13.0,1.14.0)'
> > >>
> > >> What do you all think? Discuss!
> > >> -Robert Houghton
> > >>
> > >> [1]
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8016data=02%7C01%7Cdoevans%40vmware.com%7Cf1b924b4bfe04438773a08d7eb033b08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637236271422916825sdata=gRQFRLGueu5x8FxRIsdXeKM5PmZLBT6uW8Dgh7FAx2s%3Dreserved=0
> > >> [2]
> > >>
> >
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fconcourse.apachegeode-ci.info%2Fteams%2Fmain%2Fpipelines%2Fapache-develop-examplesdata=02%7C01%7Cdoevans%40vmware.com%7Cf1b924b4bfe04438773a08d7eb033b08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637236271422916825sdata=g6mfjOLQrkbpS9UWC9QBQL37rcFASIUo5PG0rCs1eAU%3Dreserved=0
> > >
> >
> >
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

But the "maven-resource" type is still non-repeatable for SNAPSHOTs.
Resource pinning will not work, and identifying the actual GEODE build
causing the failure won't work.

On Mon, Apr 27, 2020 at 4:39 PM Owen Nichols  wrote:

> That sounds like a problem with geode-examples regardless of whether we
> use -SNAPSHOT or -build.123 … we should replace the 24hr trigger with a
> maven-resource trigger (nulldriver, linkyard, or similar)
>
> > On Apr 27, 2020, at 4:34 PM, Robert Houghton 
> wrote:
> >
> > @Owen: What geode-examples is doing, is a perfect case study of the wrong
> > behaviors predicated on 'SNAPSHOT' with Concourse.
> >
> > Concourse CI is built on repeatable builds with defined input. The
> current
> > CI for examples is using a 24hr trigger because it has no valid
> dependency
> > on the Geode develop pipeline.
> >
> >
> > On Mon, Apr 27, 2020 at 3:55 PM Owen Nichols 
> wrote:
> >
> >> Sounds good to me, based on the repeatability argument.  Will the build
> >> number output by gfsh version --full match the maven artifact spec?
> >>
> >> I am unclear what geode-examples has to do with this.  Aside from
> >> repeatability, there is no reason geode-examples should be having any
> >> problem with -SNAPSHOT.  As far as I can tell, geode-examples develop is
> >> correctly resolving geodeDistribution from
> >> org.apache.geode:apache-geode:1.13.0-SNAPSHOT, and geode-examples
> master is
> >> correctly resolving geodeDistribution from
> >> org.apache.geode:apache-geode:1.12.0
> >>
> >>> On Apr 27, 2020, at 3:41 PM, Anthony Baker  wrote:
> >>>
> >>> @Robert, can you show some examples of what the build number would be
> >> under this proposal?  Does 1.13.0-SNAPSHOT become 1.13.0.N where N
> >> increments every build?
> >>>
> >>> Seems reasonable.  Since the consumers of pre-release artifacts are
> >> either a) this project or b) close related projects for
> >> integration-testing-purposes-only I’m not super worried about the ugly
> >> syntax.
> >>>
> >>>
> >>> Anthony
> >>>
> >>>
> >>>> On Apr 27, 2020, at 3:25 PM, Jacob Barrett 
> wrote:
> >>>>
> >>>> It is unfortunate that the Maven/Gradle community hasn’t addressed
> this
> >> glaring issue with SNAPSHOT for decades now (well maybe not decades but
> >> certainly decade). It is also unfortunate that the Maven version
> coordinate
> >> is ugly. Aside from that I am totally onboard. Yay for reproducible
> builds
> >> and predictable downstream builds!
> >>>>
> >>>> With SNAPSHOTS in a repo the repository automatically prunes back old
> >> builds. Do we have any concerns about having a plethora of builds
> filling
> >> up this new pre-release repository?
> >>>>
> >>>> -Jake
> >>>>
> >>>>> On Apr 27, 2020, at 3:21 PM, Robert Houghton 
> >> wrote:
> >>>>>
> >>>>> Hello to the community,
> >>>>>
> >>>>> tl;dr - Lets publish builds, not snapshots, for repeatable CI builds,
> >> as
> >>>>> GEODE-8016[1]. Communicate desired artifact version via the existing
> >>>>> 'UpdatePassingTokens' job.
> >>>>>
> >>>>> I have been working on the Geode build and CI systems for a long
> time,
> >> and
> >>>>> it has irked me that the geode-examples pipeline[2] does not build
> and
> >> test
> >>>>> against the latest artifacts from the develop pipeline. Some work has
> >> been
> >>>>> done already to allow this via "composite" builds for local testing
> >> without
> >>>>> needing to publish Geode to your local Maven repository.
> >>>>>
> >>>>> From a Concourse CI perspective, composite builds are costly due to
> the
> >>>>> rebuild of the upstream artifacts. They allow repeatable builds, but
> >> only
> >>>>> by rebuilding those dependencies. Better would be to point to
> upstream
> >>>>> artifacts as concrete build versions. SNAPSHOT builds can and do roll
> >>>>> (invisibly) as new versions are published. Discrete, numbered builds
> do
> >>>>> not. Downstream consumers can use greedy version specifiers to get
> >> their
> >>>>> current behavior of "latest".
> >>>>>
> >>>>> Gradle: 'org.apache.geode:geode-core:1.13.0+'
> >>>>> Maven: 'org.apache.geode'
> >>>>>  'geode-core'
> >>>>>  '[1.13.0,1.14.0)'
> >>>>>
> >>>>> What do you all think? Discuss!
> >>>>> -Robert Houghton
> >>>>>
> >>>>> [1] https://issues.apache.org/jira/browse/GEODE-8016
> >>>>> [2]
> >>>>>
> >>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-examples
> >>>>
> >>>
> >>
> >>
>
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

@Owen: What geode-examples is doing, is a perfect case study of the wrong
behaviors predicated on 'SNAPSHOT' with Concourse.

Concourse CI is built on repeatable builds with defined input. The current
CI for examples is using a 24hr trigger because it has no valid dependency
on the Geode develop pipeline.


On Mon, Apr 27, 2020 at 3:55 PM Owen Nichols  wrote:

> Sounds good to me, based on the repeatability argument.  Will the build
> number output by gfsh version --full match the maven artifact spec?
>
> I am unclear what geode-examples has to do with this.  Aside from
> repeatability, there is no reason geode-examples should be having any
> problem with -SNAPSHOT.  As far as I can tell, geode-examples develop is
> correctly resolving geodeDistribution from
> org.apache.geode:apache-geode:1.13.0-SNAPSHOT, and geode-examples master is
> correctly resolving geodeDistribution from
> org.apache.geode:apache-geode:1.12.0
>
> > On Apr 27, 2020, at 3:41 PM, Anthony Baker  wrote:
> >
> > @Robert, can you show some examples of what the build number would be
> under this proposal?  Does 1.13.0-SNAPSHOT become 1.13.0.N where N
> increments every build?
> >
> > Seems reasonable.  Since the consumers of pre-release artifacts are
> either a) this project or b) close related projects for
> integration-testing-purposes-only I’m not super worried about the ugly
> syntax.
> >
> >
> > Anthony
> >
> >
> >> On Apr 27, 2020, at 3:25 PM, Jacob Barrett  wrote:
> >>
> >> It is unfortunate that the Maven/Gradle community hasn’t addressed this
> glaring issue with SNAPSHOT for decades now (well maybe not decades but
> certainly decade). It is also unfortunate that the Maven version coordinate
> is ugly. Aside from that I am totally onboard. Yay for reproducible builds
> and predictable downstream builds!
> >>
> >> With SNAPSHOTS in a repo the repository automatically prunes back old
> builds. Do we have any concerns about having a plethora of builds filling
> up this new pre-release repository?
> >>
> >> -Jake
> >>
> >>> On Apr 27, 2020, at 3:21 PM, Robert Houghton 
> wrote:
> >>>
> >>> Hello to the community,
> >>>
> >>> tl;dr - Lets publish builds, not snapshots, for repeatable CI builds,
> as
> >>> GEODE-8016[1]. Communicate desired artifact version via the existing
> >>> 'UpdatePassingTokens' job.
> >>>
> >>> I have been working on the Geode build and CI systems for a long time,
> and
> >>> it has irked me that the geode-examples pipeline[2] does not build and
> test
> >>> against the latest artifacts from the develop pipeline. Some work has
> been
> >>> done already to allow this via "composite" builds for local testing
> without
> >>> needing to publish Geode to your local Maven repository.
> >>>
> >>> From a Concourse CI perspective, composite builds are costly due to the
> >>> rebuild of the upstream artifacts. They allow repeatable builds, but
> only
> >>> by rebuilding those dependencies. Better would be to point to upstream
> >>> artifacts as concrete build versions. SNAPSHOT builds can and do roll
> >>> (invisibly) as new versions are published. Discrete, numbered builds do
> >>> not. Downstream consumers can use greedy version specifiers to get
> their
> >>> current behavior of "latest".
> >>>
> >>> Gradle: 'org.apache.geode:geode-core:1.13.0+'
> >>> Maven: 'org.apache.geode'
> >>>   'geode-core'
> >>>   '[1.13.0,1.14.0)'
> >>>
> >>> What do you all think? Discuss!
> >>> -Robert Houghton
> >>>
> >>> [1] https://issues.apache.org/jira/browse/GEODE-8016
> >>> [2]
> >>>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-examples
> >>
> >
>
>

Re: [DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

The artifact would change from "1.13.0-SNAPSHOT" to "1.13.0-build.123".

The number after the "build" slug is auto-incremented by our CI system
anyway, as the "geode-build-version" semver resource. We are actually doing
*more* work in Gradle to truncate that number from the current "SNAPSHOT"
value.

On Mon, Apr 27, 2020 at 3:41 PM Anthony Baker  wrote:

> @Robert, can you show some examples of what the build number would be
> under this proposal?  Does 1.13.0-SNAPSHOT become 1.13.0.N where N
> increments every build?
>
> Seems reasonable.  Since the consumers of pre-release artifacts are either
> a) this project or b) close related projects for
> integration-testing-purposes-only I’m not super worried about the ugly
> syntax.
>
>
> Anthony
>
>
> > On Apr 27, 2020, at 3:25 PM, Jacob Barrett  wrote:
> >
> > It is unfortunate that the Maven/Gradle community hasn’t addressed this
> glaring issue with SNAPSHOT for decades now (well maybe not decades but
> certainly decade). It is also unfortunate that the Maven version coordinate
> is ugly. Aside from that I am totally onboard. Yay for reproducible builds
> and predictable downstream builds!
> >
> > With SNAPSHOTS in a repo the repository automatically prunes back old
> builds. Do we have any concerns about having a plethora of builds filling
> up this new pre-release repository?
> >
> > -Jake
> >
> >> On Apr 27, 2020, at 3:21 PM, Robert Houghton 
> wrote:
> >>
> >> Hello to the community,
> >>
> >> tl;dr - Lets publish builds, not snapshots, for repeatable CI builds, as
> >> GEODE-8016[1]. Communicate desired artifact version via the existing
> >> 'UpdatePassingTokens' job.
> >>
> >> I have been working on the Geode build and CI systems for a long time,
> and
> >> it has irked me that the geode-examples pipeline[2] does not build and
> test
> >> against the latest artifacts from the develop pipeline. Some work has
> been
> >> done already to allow this via "composite" builds for local testing
> without
> >> needing to publish Geode to your local Maven repository.
> >>
> >> From a Concourse CI perspective, composite builds are costly due to the
> >> rebuild of the upstream artifacts. They allow repeatable builds, but
> only
> >> by rebuilding those dependencies. Better would be to point to upstream
> >> artifacts as concrete build versions. SNAPSHOT builds can and do roll
> >> (invisibly) as new versions are published. Discrete, numbered builds do
> >> not. Downstream consumers can use greedy version specifiers to get their
> >> current behavior of "latest".
> >>
> >> Gradle: 'org.apache.geode:geode-core:1.13.0+'
> >> Maven: 'org.apache.geode'
> >>'geode-core'
> >>'[1.13.0,1.14.0)'
> >>
> >> What do you all think? Discuss!
> >> -Robert Houghton
> >>
> >> [1] https://issues.apache.org/jira/browse/GEODE-8016
> >> [2]
> >>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-examples
> >
>
>

[DISCUSS] Publish Builds, not Snapshots

[Robert Houghton]

Hello to the community,

tl;dr - Lets publish builds, not snapshots, for repeatable CI builds, as
GEODE-8016[1]. Communicate desired artifact version via the existing
'UpdatePassingTokens' job.

I have been working on the Geode build and CI systems for a long time, and
it has irked me that the geode-examples pipeline[2] does not build and test
against the latest artifacts from the develop pipeline. Some work has been
done already to allow this via "composite" builds for local testing without
needing to publish Geode to your local Maven repository.

>From a Concourse CI perspective, composite builds are costly due to the
rebuild of the upstream artifacts. They allow repeatable builds, but only
by rebuilding those dependencies. Better would be to point to upstream
artifacts as concrete build versions. SNAPSHOT builds can and do roll
(invisibly) as new versions are published. Discrete, numbered builds do
not. Downstream consumers can use greedy version specifiers to get their
current behavior of "latest".

Gradle: 'org.apache.geode:geode-core:1.13.0+'
Maven: 'org.apache.geode'
 'geode-core'
 '[1.13.0,1.14.0)'

What do you all think? Discuss!
-Robert Houghton

[1] https://issues.apache.org/jira/browse/GEODE-8016
[2]
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-examples

Re: [DISCUSS] should we add some Windows PR checks?

[Robert Houghton]

I would like to *not* include them, until they can take advantage of our
run-in-docker behavior, and not be dog slow. Plus, they are *very*
expensive instances.

On Mon, Apr 27, 2020 at 1:25 PM Donal Evans  wrote:

> Having just hit this issue myself, and seen it crop up a few times when I
> was CIO, I think it would definitely be valuable to add some level of
> Windows testing to PR pre-checkin. It's always better to catch issues early
> rather than late, and anything that helps keep the pipeline green gets my
> approval.
>
> Related to this, I personally don't know how to debug a failure and verify
> a fix for failing Windows tests locally, so being able to push to a PR
> branch and have that all done for me would be very helpful as well.
> 
> From: Owen Nichols 
> Sent: Monday, April 27, 2020 1:00 PM
> To: dev@geode.apache.org 
> Subject: [DISCUSS] should we add some Windows PR checks?
>
> Currently we have zero Windows tests in the PR pipeline, which has led to
> more than a handful of times this year where a PR passed all its checks
> then failed the main pipeline once merged.
>
> Adding these 3 jobs to the PR pipeline would have caught the majority of
> them:
> WindowsUnitTestOpenJDK11
> WindowsIntegrationTestOpenJDK11
> WindowsAcceptanceTestOpenJDK11
>
> The longest of these 3 jobs is about 1/2 hour longer than the current
> longest PR check.
>
> This is a discussion, not a vote thread.  This might also be a good chance
> to reflect on whether more or fewer PR checks should be “required”, whether
> there are other checks we do not have currently but would be nice, the
> balance between JDK8 and JDK11 checks, or even brainstorm ideas to create
> additional checks that might not apply to all PRs but could be requested
> (or attached automatically based on area of code touched).
>

Re: [VOTE] Using Github issues and wiki for geode-kafka-connector project

[Robert Houghton]

Looks like we can use the `.asf.yml` file in our repositories to enable
`issues` and `wiki` behavior ourselves if we decide we like the feature :)

On Fri, Apr 24, 2020 at 7:46 AM Dave Barnes  wrote:

> +1
> I like the look of the samples.
> I also want to echo Anthony and Blake: Accessibility is huge, not only
> because it smooths the way for adding content, but for pruning outdated
> material.
>
> On 4/24/20, 7:18 AM, "Blake Bender"  wrote:
>
> +1 - Wow, research - nice work, Naba, this is great!
>
> Just want to emphasize a larger point I failed to make in my response
> re:
> Markdown.  My experience with Wikis has been that it's very difficult
> to
> convince people to contribute entries/edits, and an active community of
> contributors makes a big difference between a good and less-good Wiki.
> Better content on the Wiki naturally leads to more people reading, so
> anything we can do to eliminate barriers to entry for contributing to
> our
> wiki is a good thing.  Having the wiki right there on GitHub removes a
> big
> impediment, we should def do this.
>
> Thanks,
>
> Blake
>
>
> On Thu, Apr 23, 2020 at 8:14 PM Nabarun Nag  wrote:
>
> > Hi Anthony!
> >
> > Sorry for the late reply but I was doing some research. The issues
> and wiki
> > section as of now has been used by few engineers only and Confluent
> has not
> > yet entered any issues as they are still reviewing the project. I
> went
> > ahead and looked into all projects in the Apache domain using issues
> and
> > the extra features they enable.
> > *JIRA vs Issues:*
> >
> >- There are a sizable number of Apache projects who are using
> GitHub
> >issues
> >- One clear advantage is the automatic linking of PRs and Issues.
> Issues
> >can be closed automatically once the PR is merged.
> >- It can also enable a feature to delete the feature branch
> >automatically once the PRs is merged (we have lot unused
> > feature/GEODE-
> >branches in origin which were not deleted after merging PRs)
> >- It enables us to use Github Project management(Github version of
> >PivotalTracker)  which is integrated with Github issues and PRs
> and all
> > the
> >movement from "To-do", "In-progress", "resolved" and "closed" are
> > automated
> >depending on if a PR is opened, requires reviews, reviewed and
> merged
> > state.
> >
> > *Github Wiki vs Confluence Wiki:*
> >
> >- As you have mentioned that visibility is more important, we can
> follow
> >other open-source products like Greenplum, Hystrix and we can use
> the
> > wiki
> >page to explain stuff like how to contribute, basic architecture,
> > internal
> >knowledge, i.e information that is needed to contribute to Geode.
> >- A signification advantage is the colocation of code and wiki.
> Any
> >developer can find Geode GitHub repo and that person now has all
> the
> > tools
> >needed to start contributing.
> >
> >
> > A few examples of well-written wikis on GitHub:
> >
> >-
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fd3%2Fd3%2Fwikidata=02%7C01%7Cdaveba%40vmware.com%7Cb17baa964f014fcecc5108d7e85a6b55%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637233347359128192sdata=Yq%2Bi%2FuU2%2B5JiQ1jd%2BasQ0%2F%2BTPrK4vdxm%2FK6Faw4UW3M%3Dreserved=0
> >-
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FNetflix%2FHystrix%2Fwikidata=02%7C01%7Cdaveba%40vmware.com%7Cb17baa964f014fcecc5108d7e85a6b55%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637233347359128192sdata=1c5DQgA%2BWzGFZW5v%2FJdJeeyyELsNyWuFD3JNC2C5VhI%3Dreserved=0
> >-
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fhelix%2Fwikidata=02%7C01%7Cdaveba%40vmware.com%7Cb17baa964f014fcecc5108d7e85a6b55%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637233347359128192sdata=89fieZFbngwXwD2uXCkEg0PTWtw1nbtU2xVn%2BpPm%2FFU%3Dreserved=0
> >
> >
> > ASF: word on the street is that it was mentioned in ApacheCon, that
> they
> > support the use of Github wiki and issues in ASF projects, and this
> can
> > also be seen in multiple INFRA tickets mentioning enabling wiki.
> >
> > I am also looking into ZenHub to improve our workflow. ZenHub is a
> very
> > robust project management tools used by Apache Contributors and
> > corporations like VMware.
> >
> > Regards
> > Nabarun Nag
> >
> >
> > On Thu, Apr 23, 2020 at 2:40 PM Anthony Baker 
> wrote:
> >
> > > Having used pretty every style of wiki, I care less about the wiki
> tech
> > > and more about making the content easily accessible and
> discoverable for
> > > our users and contributors.  Our current wiki has a lot of useful
> > > 

Re: Reconfiguring our notifications and more

[Robert Houghton]

+1

On Tue, Apr 21, 2020, 08:54 Anthony Baker  wrote:

> I’d like a quick round of feedback so we can stop the dev@ spamming [1].
>
> ASF has implemented a cool way to give projects control of lots of things
> [2].  In short, you provide a .asf.yml in each and every GitHub repo to
> control lots of interesting things.  For us the most immediately relevant
> are:
>
> notifications:
>   commits: comm...@geode.apache.org
>   issues:  iss...@geode.apache.org
>   pullrequests:  notificati...@geode.apache.org
>   jira_options: link label comment
>
> I’d like to commit this to /develop and cherry-pick over to /master ASAP.
> Later on we can explore the website and GitHub sections.  Let me know what
> you think.
>
> Anthony
>
>
> [1] https://issues.apache.org/jira/browse/INFRA-20156
> [2]
> https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Notificationsettingsforrepositories

Re: [PROPOSAL]: GEODE-7940 to support/1.12

[Robert Houghton]

Conditional +1 from me too, pending a few days on develop with happy
results :)

Thanks Juan!

On Fri, Apr 17, 2020 at 1:41 AM Ju@N  wrote:

> Hello devs,
>
> I'd like to propose bringing *GEODE-7940 [1]* to the *support/1.12* branch.
> The bug is not new, seems quite old actually, but still seems pretty
> critical as it can lead to data loss in WAN topologies.
> Long story short: when there are multiple parallel *gateway-senders*
> attached
> to the same region and the user decides to stop, detach and *destroy* one
> of the senders (the destroy part is what actually causes the problem), the
> rest of the senders attached will silently stop replicating events to the
> the remote clusters and all these events will be lost.
> The fix has been merged into develop through commit *bfbb398 [2]*.
> Best regards.
>
> [1]: https://issues.apache.org/jira/browse/GEODE-7940
> [2]:
>
> https://github.com/apache/geode/commit/bfbb398891c5d96fa3a5975365b29d71bd849ad6
>
> --
> Ju@N
>

Re: [Discuss] Cache.close synchronous is not synchronous, but code still expects it to be....

[Robert Houghton]

The time out is increased in the shared Jinja variables file

On Thu, Apr 16, 2020, 10:48 Kirk Lund  wrote:

> I revived my branch by rebasing it on develop and filed a new draft PR:
>
> https://github.com/apache/geode/pull/4963
>
> Unfortunately, IntegrationTest exceeds timeout every time I trigger it. The
> cause does not appear to be a specific test or hang. I
> think IntegrationTest has already been running very close to the timeout
> and is exceeding it fairly often even without my changes in #4963.
>
> Should we increase the timeout for IntegrationTest? (Anyone know how to
> increase it?)
>
> On Tue, Apr 14, 2020 at 5:06 PM Kirk Lund  wrote:
>
> > When I previously submitted a PR to change concurrent calls to
> > Cache.close() to not return until everything is completely stopped
> > (including the DistributedSystem), the change was down-voted by Anil. His
> > reasoning was that the current behavior is expected by users and is de
> > facto correct. Changing it would change the behavior and result in
> > long-time users being surprised by the new behavior.
> >
> > Below are some stack traces from my notes about what causes the thread
> > calling Cache.close() to return prematurely before the DistributedSystem
> is
> > disconnected. Thread #3 (junit main thread) invokes Cache.close() as seen
> > in stack #3 below. DiskStoreImpl reacts to the situation by once again
> > invoking Cache.close() once or twice (stack #1 and stack #2) and one of
> > those invocations wins the race against the thread in stack #3. The
> thread
> > in stack #3 then returns prematurely before the DistributedSystem is
> > disconnected. If thread #3 then attempts to do anything like create a new
> > cache (which quite a few tests do), it can fail and throw
> > DistributedSystemDisconnectedException from cache create.
> >
> > There are two early-outs in GemFireCacheImpl.close() which allows a
> > calling thread to return before any actual work has been completed after
> > closing nothing but the SecurityService.
> >
> > if (isClosed()) {
> >   return;
> > }
> >
> > And "isClosed()" returns true when isClosing flag is true (which is set
> > true when closing starts):
> >
> >   public boolean isClosed() {
> > return isClosing;
> >   }
> >
> > Failed creation of a persistent region is one way DiskStoreImpl can cause
> > multiple threads trying to close the cache to trip all over each other.
> >
> > DiskStoreImpl is problematic at best in the way it's implemented and it's
> > not currently unit tested (or unit testable without lots of refactoring),
> > and I don't plan to revisit this change. I would however be happy to
> review
> > proposals and PRs related to this. My change was focused on Cache.close()
> > and adding a CountDownLatch to close() -- perhaps the next attempt to
> "fix"
> > this should focus on DiskStoreImpl -- one could easily argue that closing
> > the cache is NOT a valid responsibility for DiskStoreImpl. But changing
> the
> > behavior of the persistent "layer" of Geode might require more research
> > (and a LOT more refactoring) than I had time for since my reason for
> > working on this was to fix some flaky dunit tests caused by this race
> > condition.
> >
> > This bug appears to be caused when creation of a persistent region fails
> > and DiskStoreImpl.lambda$handleDiskAccessException forks a new Thread to
> > close the Cache which succeeds in closing the Cache before the main test
> > thread closes it. The main test thread then early outs because the
> > DiskStore thread is already closing the Cache. The main test thread then
> > tries to create a Cache which collides with the DiskStore thread which is
> > still closing the Cache and DistributedSystem.
> >
> > java.lang.Throwable: KIRK GemFireCacheImpl closed 632230948
> >   at
> org.apache.geode.internal.cache.GemFireCacheImpl.close(GemFireCacheImpl.java:2365)
> >   at
> org.apache.geode.internal.cache.GemFireCacheImpl.close(GemFireCacheImpl.java:1917)
> >   at
> org.apache.geode.internal.cache.DiskStoreImpl.lambda$handleDiskAccessException$2(DiskStoreImpl.java:3380)
> >   at java.lang.Thread.run(Thread.java:748)
> >
> > java.lang.Throwable: KIRK InternalDistributedSystem closed 306674056
> >   at
> org.apache.geode.distributed.internal.InternalDistributedSystem.disconnect(InternalDistributedSystem.java:1637)
> >   at
> org.apache.geode.distributed.internal.InternalDistributedSystem.disconnect(InternalDistributedSystem.java:1225)
> >   at
> org.apache.geode.internal.cache.GemFireCacheImpl.close(GemFireCacheImpl.java:2351)
> >   at
> org.apache.geode.internal.cache.GemFireCacheImpl.close(GemFireCacheImpl.java:1917)
> >   at
> org.apache.geode.internal.cache.DiskStoreImpl.lambda$handleDiskAccessException$2(DiskStoreImpl.java:3380)
> >   at java.lang.Thread.run(Thread.java:748)
> >
> > java.lang.Throwable: KIRK DiskStoreImpl closing cache 1555793073
> >   at
> 

Re: RFC: Add C Bindings to Geode Native Client

[Robert Houghton]

Quick note: ccache is a C/C++ compiler cache. Examples using 'ccache' as
the name are confusing.

On Tue, Mar 31, 2020 at 3:56 PM Jacob Barrett  wrote:

>
>
> > On Mar 31, 2020, at 3:06 PM, Blake Bender  wrote:
> >
> > We in this instance means the native client team.  As far as specific
> > comments, I'm going to suggest we not go down that road, because this
> feels
> > a little more adversarial to me than it needs to be already.
>
> Sorry it feels adversarial. From below I think there is a misunderstanding
> of my preferences.
>
> >  Suffice to
> > say that from my own perspective, in both what you wrote and what I got
> > from our in-person conversation on Monday, your answer to the general
> > question "Should the C bindings be part of the native client?" appeared
> to
> > be no, thus a separate repository seemed a perfectly reasonable
> > assumption.  I had hoped to do this in the geode-native repo to begin
> with,
> > so your assent to this makes life easier.
>
> This may be the point of confusion because I have never intended to give
> the impression that the C-bindings should be separate from the geode-native
> repo. My examples even integrate it with the geode-native project. I do
> believe it should be separate sources and separate includes. I would not
> want to be doing a C++ project and have C functions clouding my IDE or
> vice versa. Perhaps that is where the confusion comes from.
>
> > As far as my concerns about inefficiency, what I meant is essentially yes
> > we have multiple copies of the same code in the release, and this always
> > makes me a little uneasy.  I've seen a lot of compatibility bugs in my
> > career having to do with different products having different versions of
> > the same code, so my natural inclination is to avoid it when possible.
> > Having both C++ and C-style functions exported from the same library
> > doesn't give me any heartburn at all, so simply compiling the C bindings
> > into the existing shared library just means one less copy of the code in
> > the installation.  I fear I am in the minority in this opinion, however,
> > and it's not something I'm really doctrinaire about, so I'll defer.
> I would really like to understand your concerns but I don’t understand how
> combining the symbols into a single file resolves the versioning issue? Can
> your help me understand what the different produces with different versions
> means and how it would apply to this case?
>
> If the C and C++ symbols are both exported from the same shared library
> would you have a common include directory as well or would you spit the
> includes? I could live with a combine library but not a combined include
> headers.
>
> > So are we good here?  I'd like to get the RFC wrapped up and move on to
> > building this.
>
> Do you feel there is a consensus? I feel like there is a lot left that
> isn’t either in the original RFC, hasn’t been discussed here or is still a
> sticking point. You could update the RFC with what we have discussed and
> see if consensus is reached.
>
> Sticking points:
> * Single or split shared libraries
> * Single or split includes
> * Single or split source repository
>
> Undefined:
> * Exception handling (I gave one example but didn’t get feedback or
> consensus)
> * Namespacing/Prefixing
> * Pattern and naming conventions for new, delete and class methods.
> * Handling of (de)serialization hand off or callbacks into non-C code
>
> Agreed:
> * Strong types with opaque struct pointers
> * No complete structs in the API/ABI
>
> -Jake
>
>

Re: RFC: Add C Bindings to Geode Native Client

[Robert Houghton]

+1
YES.

Having a set of clean C headers also allows for using a broad set of code
generators for additional languages (Swig, for example).



On Tue, Mar 24, 2020 at 2:20 PM Blake Bender  wrote:

> Hello everyone,
>
> We'd like to add C bindings to the native client, in preparation for the
> larger task of adding support for new languages, e.g. .net core.  Please
> have a look at the proposal below and let me know what you think.
>
> Thanks,
>
> Blake
>
>
>
> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
>

Re: [ANNOUNCE] Github Wiki and Issus are now activated on geode-kafka-connector

[Robert Houghton]

Woo! Now I just need to find an issue to file or something important to
document in the wiki...

Thanks, Naba!

On Tue, Mar 24, 2020 at 1:42 PM Nabarun Nag  wrote:

> Hi,
>
> Issues are wiki pages  are now active on the geode-kafka-connector
> repository. Thank you all for the kind votes.
>
> Regards
> Nabarun Nag
>

Re: [VOTE] Using Github issues and wiki for geode-kafka-connector project

[Robert Houghton]

+1!

On Sat, Mar 21, 2020, 17:17 Nabarun Nag  wrote:

> Hello team,
>
> We are planning to experiment with using Github issues and wiki for the
> Apache project *Geode-Kafka-Connector. *(not Apache Geode project). Please
> do give your vote on this as we need to send the vote link to infra to
> activate it.
>
> *Why are we doing this ? / Advantages* :
> 1. *Unified location* to have documentation, code and issue tracking.
> 2. Leverage Github tools like Github pages to create websites hosting
> information about the project.
> 3. No separate JIRA accounts or permission required to create issues.
> 4. This will have *no impact on the broader Geode community* as right now
> only 3-4 developers involved in this project.
> 5. *This is an experiment.* If things do not work out we can always revert
> back to the traditional way of having separate JIRA, documentation,
> websites etc.
>
> *Precedence*:
> 1. Kubernetes uses the github issues
> 2. RabbitMQ uses github issues.
>
>
> *NOTE: *- Please be cordial and do not use any condescending language and
> absolutely no bullying.
> - Please treat this email as a professional business email and maintain
> email etiquette while replying.
>
> Regards
> Nabarun
>

Re: Welcome!

[Robert Houghton]

And thank goodness! Plenty of work to share around. Welcome!

On Fri, Mar 20, 2020, 17:21 Anthony Baker  wrote:

> Over the last few months the Geode PMC has invited the following
> people as new committers and PMC members.  We’ve been remiss in
> officially recognizing your new status, but welcome aboard!  Thanks so
> much for your contributions to the project and we're looking forward
> to seeing more great things to come!
>
> New committers:
>Mario Kevo
>Aaron Lindsey
>Benjamin Ross
>Donal Evans
>
> New PMC members:
>Joris Melchior
>
>
> Anthony
>

Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest

[Robert Houghton]

gt;>> 43 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6190)
> > >>>>> 44 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5768)
> > >>>>> 44 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5772)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6191)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6218)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6219)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6225)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6244)
> > >>>>> 46 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6245)
> > >>>>> 48 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6129)
> > >>>>> 49 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5804)
> > >>>>> 51 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5877)
> > >>>>> 53 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5742)
> > >>>>> 54 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 5757)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6232)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6233)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6234)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6259)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6260)
> > >>>>> 66 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6278)
> > >>>>> 79 is too many changed tests to stress test. Allowing this job to
> > pass
> > >>>>> without stress testing.  (build 6481)
> > >>>>> 103 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6493)
> > >>>>> 106 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6206)
> > >>>>> 106 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6207)
> > >>>>> 115 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 5769)
> > >>>>> 118 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6226)
> > >>>>> 721 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6197)
> > >>>>> 722 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6217)
> > >>>>> 722 is too many changed tests to stress test. Allowing this job to
> > >>> pass
> > >>>>> without stress testing.  (build 6221)
> > >>>>>
> > >>>>> Please note, sometimes the number of files reported seems to be way
> > >>>> higher
> > >>>>> than it should be.  For example
> > >>>> https://github.com/apache/geode/pull/4691
> > >>>>> <https://github.com/apache/geode/pull/4691> shows exactly 1 test
> > file
> > >>>>> touched, but
> > >>>>>
> > >>>>
> > >>>
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> > >>>>> <
> > >>>>>
> > >>>>
> > >>>
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> > >>>>>
> > >>>>> thinks it touched 66 test files.
> > >>>>>
> > >>>>> There’s currently no good data to predict how long StressNew will
> > >>> take,
> > >>>> it
> > >>>>> just depends on the mix of tests touched.  I am aware of at least 4
> > >>> cases
> > >>>>> (with less than 25 files) where the timeout was hit.  In two of
> those
> > >>>> cases
> > >>>>> we re-ran with a longer timeout, and in two cases the PR author
> split
> > >>> up
> > >>>>> the PR into half a dozen smaller PRs to get around it.
> > >>>>>
> > >>>>>
> > >>>>>> On Mar 1, 2020, at 7:52 PM, Anthony Baker 
> > >>> wrote:
> > >>>>>>
> > >>>>>> What percentage of PR’s are currently subject to the 25 test file
> > >>> rule?
> > >>>>> How many would be subject to the concourse timeout?
> > >>>>>>
> > >>>>>> I’d like to understand the scope of the impact for this change.
> > >>>>>>
> > >>>>>> Anthony
> > >>>>>>
> > >>>>>>
> > >>>>>>> On Mar 1, 2020, at 8:58 AM, Owen Nichols 
> > >>> wrote:
> > >>>>>>>
> > >>>>>>> Impossible, no. Inconvenient, perhaps, but a small price to pay
> for
> > >>>>> being
> > >>>>>>> able to trust that green means green.
> > >>>>>>>
> > >>>>>>> With or without this proposed change, if anyone is having trouble
> > >>>>> getting
> > >>>>>>> their PR to pass StressNew, please bring it up on the dev list
> and
> > >>> we
> > >>>>> can
> > >>>>>>> discuss the appropriate solution on a case-by-case basis (e.g.
> > >>>>> increasing
> > >>>>>>> timeout, fixing the logic that identifies changed test files,
> > >>>> splitting
> > >>>>>>> into multiple PRs, authorizing an override, etc).
> > >>>>>>>
> > >>>>>>> On Sun, Mar 1, 2020 at 8:56 AM Dan Smith 
> > >>> wrote:
> > >>>>>>>
> > >>>>>>>> Won't this make it impossible to merge refactoring changes that
> > >>> touch
> > >>>>> a lot
> > >>>>>>>> of tests?
> > >>>>>>>>
> > >>>>>>>> -Dan
> > >>>>>>>>
> > >>>>>>>> On Sat, Feb 29, 2020, 12:37 PM Robert Houghton <
> > >>> rhough...@pivotal.io
> > >>>>>
> > >>>>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>>> Yes, as it should
> > >>>>>>>>>
> > >>>>>>>>> On Sat, Feb 29, 2020, 12:25 Dan Smith 
> wrote:
> > >>>>>>>>>
> > >>>>>>>>>> Doesn't the build fail when concourse times out?
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> >
>

Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest

[Robert Houghton]

stress testing.  (build 6483)
> > 43 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6190)
> > 44 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5768)
> > 44 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5772)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6191)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6218)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6219)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6225)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6244)
> > 46 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6245)
> > 48 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6129)
> > 49 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5804)
> > 51 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5877)
> > 53 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5742)
> > 54 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5757)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6232)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6233)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6234)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6259)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6260)
> > 66 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6278)
> > 79 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6481)
> > 103 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6493)
> > 106 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6206)
> > 106 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6207)
> > 115 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 5769)
> > 118 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6226)
> > 721 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6197)
> > 722 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6217)
> > 722 is too many changed tests to stress test. Allowing this job to pass
> > without stress testing.  (build 6221)
> >
> > Please note, sometimes the number of files reported seems to be way
> higher
> > than it should be.  For example
> https://github.com/apache/geode/pull/4691
> > <https://github.com/apache/geode/pull/4691> shows exactly 1 test file
> > touched, but
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> > <
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> >
> > thinks it touched 66 test files.
> >
> > There’s currently no good data to predict how long StressNew will take,
> it
> > just depends on the mix of tests touched.  I am aware of at least 4 cases
> > (with less than 25 files) where the timeout was hit.  In two of those
> cases
> > we re-ran with a longer timeout, and in two cases the PR author split up
> > the PR into half a dozen smaller PRs to get around it.
> >
> >
> > > On Mar 1, 2020, at 7:52 PM, Anthony Baker  wrote:
> > >
> > > What percentage of PR’s are currently subject to the 25 test file rule?
> > How many would be subject to the concourse timeout?
> > >
> > > I’d like to understand the scope of the impact for this change.
> > >
> > > Anthony
> > >
> > >
> > >> On Mar 1, 2020, at 8:58 AM, Owen Nichols  wrote:
> > >>
> > >> Impossible, no. Inconvenient, perhaps, but a small price to pay for
> > being
> > >> able to trust that green means green.
> > >>
> > >> With or without this proposed change, if anyone is having trouble
> > getting
> > >> their PR to pass StressNew, please bring it up on the dev list and we
> > can
> > >> discuss the appropriate solution on a case-by-case basis (e.g.
> > increasing
> > >> timeout, fixing the logic that identifies changed test files,
> splitting
> > >> into multiple PRs, authorizing an override, etc).
> > >>
> > >> On Sun, Mar 1, 2020 at 8:56 AM Dan Smith  wrote:
> > >>
> > >>> Won't this make it impossible to merge refactoring changes that touch
> > a lot
> > >>> of tests?
> > >>>
> > >>> -Dan
> > >>>
> > >>> On Sat, Feb 29, 2020, 12:37 PM Robert Houghton  >
> > >>> wrote:
> > >>>
> > >>>> Yes, as it should
> > >>>>
> > >>>> On Sat, Feb 29, 2020, 12:25 Dan Smith  wrote:
> > >>>>
> > >>>>> Doesn't the build fail when concourse times out?
> > >>>>
> > >>>>
> > >>>
> > >
> >
> >
>

Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest

[Robert Houghton]

Yes, as it should

On Sat, Feb 29, 2020, 12:25 Dan Smith  wrote:

> Doesn't the build fail when concourse times out?
>
> -Dan
>
> On Fri, Feb 28, 2020, 1:37 PM Donal Evans  wrote:
>
> > +1
> >
> > If the original reason for the 25 file limit was to prevent StressNewTest
> > taking too long, then the concourse timeout renders the restriction
> > redundant. Also, I'd think that if anything, the more files a PR changed,
> > the more interested we would be in running StressNewTest. Allowing it to
> > just be "green" when we get over 25 changed files seems counterintuitive
> > with that in mind.
> >
> > On Fri, Feb 28, 2020 at 12:04 PM Owen Nichols 
> wrote:
> >
> > > Currently, StressNewTest will turn green WITHOUT RUNNING ANY TESTS if
> it
> > > thinks (often wrongly) that more than 25 test files were touched.
> > >
> > > This is both dishonest (it can give a false green) AND totally
> > unnecessary
> > > (concourse already applies a timeout to StressNewTest, currently set
> at 6
> > > hours, so there is no reason not to at least try to run all changed
> > tests,
> > > they may easily complete in under 6 hours even if many files).
> > >
> > > *** I PROPOSE that we should remove this 25-file-free-pass loophole.
> ***
> > >
> > > As always, if anyone is having trouble getting their PR to pass
> > StressNew,
> > > please bring it up on the dev list and we can discuss the appropriate
> > > solution on a case-by-case basis (e.g. increasing timeout, fixing the
> > logic
> > > that identifies changes test files, splitting into multiple PRs, etc).
> > >
> > >
> >
>

Re: Redis PubSubTest started failing

[Robert Houghton]

We don't even have windows unit tests for PRs. Walk before we run...

On Wed, Feb 19, 2020, 09:00 Owen Nichols  wrote:

> Perhaps also worth considering: can we get WindowsStressNew added to the PR
> checks?
>
> On Wed, Feb 19, 2020 at 8:50 AM Udo Kohlmeyer  wrote:
>
> > Is this something that can be fixed in a short time (2hrs)? If not, can
> > be revert and get back to a clean pipeline?
> >
> > --Udo
> >
> > On 2/19/20 8:23 AM, Jens Deppe wrote:
> > > Thanks Kirk,
> > >
> > > We're working on fixing this.
> > >
> > > --Jens
> > >
> > > On Tue, Feb 18, 2020 at 3:23 PM Kirk Lund  wrote:
> > >
> > >> I just started seeing the Redis PubSubTest fail in IntegrationTest
> after
> > >> rebasing on develop this afternoon. Looks like I have Jens' latest
> > commit
> > >> for this test:
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> *commit 1befce17eaae2403828769840f86639e13fce81f (origin/develop,
> > >> origin/HEAD, develop)Author: Jens Deppe  > >> >Date:   Tue Feb 18 13:03:19 2020 -0800
> > GEODE-7798:
> > >> Fix flaky PubSub test (#4714)*
> > >>
> > >> *Authored-by: Jens Deppe >>
> > Task
> > >> :geode-redis:integrationTest*
> > >>
> > >> Here's the stack traces:
> > >>
> > >> org.apache.geode.redis.PubSubTest > testPatternWithoutAGlob FAILED
> > >>  redis.clients.jedis.exceptions.JedisConnectionException:
> > >> java.net.SocketTimeoutException: Read timed out
> > >>  at
> > >>
> > redis.clients.util.RedisInputStream.ensureFill(RedisInputStream.java:202)
> > >>  at
> > >> redis.clients.util.RedisInputStream.readByte(RedisInputStream.java:40)
> > >>  at redis.clients.jedis.Protocol.process(Protocol.java:151)
> > >>  at redis.clients.jedis.Protocol.read(Protocol.java:215)
> > >>  at
> > >>
> >
> redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:340)
> > >>  at
> > >> redis.clients.jedis.Connection.getIntegerReply(Connection.java:265)
> > >>  at redis.clients.jedis.Jedis.publish(Jedis.java:2690)
> > >>  at
> > >>
> >
> org.apache.geode.redis.PubSubTest.testPatternWithoutAGlob(PubSubTest.java:279)
> > >>
> > >>  Caused by:
> > >>  java.net.SocketTimeoutException: Read timed out
> > >>  at java.net.SocketInputStream.socketRead0(Native Method)
> > >>  at
> > >> java.net.SocketInputStream.socketRead(SocketInputStream.java:115)
> > >>  at
> > java.net.SocketInputStream.read(SocketInputStream.java:168)
> > >>  at
> > java.net.SocketInputStream.read(SocketInputStream.java:140)
> > >>  at
> > java.net.SocketInputStream.read(SocketInputStream.java:126)
> > >>  at
> > >>
> > redis.clients.util.RedisInputStream.ensureFill(RedisInputStream.java:196)
> > >>  ... 7 more
> > >>
> > >> org.apache.geode.redis.PubSubTest > testTwoSubscribersOneChannel
> FAILED
> > >>  org.awaitility.core.ConditionTimeoutException: Condition with
> > lambda
> > >> expression in org.apache.geode.redis.PubSubTest that uses
> > >> org.apache.geode.redis.mocks.MockSubscriber was not fulfilled within 1
> > >> 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:902)
> > >>  at
> > >> org.awaitility.core.ConditionFactory.until(ConditionFactory.java:860)
> > >>  at
> > org.apache.geode.redis.PubSubTest.waitFor(PubSubTest.java:296)
> > >>  at
> > >>
> >
> org.apache.geode.redis.PubSubTest.testTwoSubscribersOneChannel(PubSubTest.java:140)
> > >>
> > >> org.apache.geode.redis.PubSubTest >
> > >> testOneSubscriberSubscribingToTwoChannels FAILED
> > >>  org.awaitility.core.ConditionTimeoutException: Condition with
> > lambda
> > >> expression in org.apache.geode.redis.PubSubTest that uses
> > >> org.apache.geode.redis.mocks.MockSubscriber was not fulfilled within 1
> > >> 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:902)
> > >>  at
> > >> org.awaitility.core.ConditionFactory.until(ConditionFactory.java:860)
> > >>  at
> > org.apache.geode.redis.PubSubTest.waitFor(PubSubTest.java:296)
> > >>  at
> > >>
> >
> org.apache.geode.redis.PubSubTest.testOneSubscriberSubscribingToTwoChannels(PubSubTest.java:110)
> > >>
> > >> org.apache.geode.redis.PubSubTest > testPatternAndRegularSubscribe
> > FAILED
> > >>  

Re: [PROPOSAL] include GEODE-7796 in Release 1.12.0

[Robert Houghton]

+1

On Tue, Feb 18, 2020, 12:49 Bruce Schuchardt  wrote:

> This fix addresses the inability of a Locator to properly reconnect to the
> cluster after being kicked out.  Instead of rejoining or shutting down the
> Locator gets into a state where it no longer tries to reboot location
> services and hangs.
>
>

Re: Odg: acceptance test task failing?

[Robert Houghton]

I had only looked at develop, not the PR pipeline. More investigation is
required.

On Mon, Feb 17, 2020, 07:05 Mario Ivanac  wrote:

> Hi,
>
> but this commit was merged to develop, on saturday morning, and test are
> falling from friday.
>
> BR,
> Mario
> ------
> *Šalje:* Robert Houghton 
> *Poslano:* 17. veljače 2020. 16:02
> *Prima:* dev@geode.apache.org 
> *Predmet:* Re: acceptance test task failing?
>
> The test has been erroring consistently on develop since
>
> https://github.com/apache/geode/commit/1a0d9769e482f49e0c725c0d6adc75d324f88958
>
> On Mon, Feb 17, 2020, 06:39 Alberto Bustamante Reyes
>  wrote:
>
> > Hi,
> >
> > After just changing some typos on a PR, I got errors on
> > geode-connectors:acceptanceTest task, but the tests were working fine
> > before that change. I have seen that the acceptance test task in concur
> has
> > failed since last 14th February (15 executions since that day). and it
> > seems all of them got the same error.
> >
> > Is there any problem with the task or the develop branch?
> >
> > Thanks,
> >
> > Alberto B.
> >
> >
>

Re: acceptance test task failing?

[Robert Houghton]

The test has been erroring consistently on develop since
https://github.com/apache/geode/commit/1a0d9769e482f49e0c725c0d6adc75d324f88958

On Mon, Feb 17, 2020, 06:39 Alberto Bustamante Reyes
 wrote:

> Hi,
>
> After just changing some typos on a PR, I got errors on
> geode-connectors:acceptanceTest task, but the tests were working fine
> before that change. I have seen that the acceptance test task in concur has
> failed since last 14th February (15 executions since that day). and it
> seems all of them got the same error.
>
> Is there any problem with the task or the develop branch?
>
> Thanks,
>
> Alberto B.
>
>

Re: StressNewTest timeout exceeded

[Robert Houghton]

How did you change its timeout?

On Wed, Feb 5, 2020, 07:27 Owen Nichols  wrote:

> Hi Mario, I’ve re-triggered your PR check with a longer timeout, since I
> think you are only about an hour over the current timeout of 6 hours for
> StressNewTest.
>
> > On Feb 5, 2020, at 5:10 AM, Mario Kevo  wrote:
> >
> > Hi all,
> >
> > I'm working on https://issues.apache.org/jira/browse/GEODE-7309
> > PR: https://github.com/apache/geode/pull/4395
> > All tests passed except stressNewTest which failed with timeout exceeded.
> >
> > Can we somehow override it to pass this tests?
> >
> > BR,
> > Mario
>
>

Re: Old geode-benchmark PRs

[Robert Houghton]

@Alberto I like this idea. The link between Jira and the GitHub PR will be
a nice breadcrumb to follow going forward as well, assuming the PRs are
correctly labeled for cross-linking to occur.


On Thu, Jan 23, 2020 at 8:05 AM Alberto Bustamante Reyes
 wrote:

> What about closing the PRs and creating a Jira ticket (or tickets) for the
> review and update of the code?
> If someone finds time to spend on benchmarks, at least he/she will find
> the tickets in Jira.
>
>
> 
> De: Donal Evans 
> Enviado: jueves, 23 de enero de 2020 16:45
> Para: dev@geode.apache.org 
> Asunto: Re: Old geode-benchmark PRs
>
> @Alexander, I haven't looked at them in months and they never received any
> formal review on GitHub, so it's hard to know for sure if they're ready to
> merge or not, but as Jake said, they probably need some massaging to get
> the resource usage just right and minimize variance. If at this point
> there's no-one who knows enough about tuning benchmarks with the time to
> look at them, then it seems unlikely that they'll get merged any time soon.
>
> On Thu, Jan 23, 2020 at 6:42 AM Alexander Murmann 
> wrote:
>
> > Donal, are you still looking at these? If they aren't ready to merge and
> > not being worked on, should they be closed?
> >
> > On Wed, Jan 22, 2020 at 3:32 PM Donal Evans  wrote:
> >
> > > Two of those PRs are mine, so perhaps I can give a bit of context for
> > > people who might look at them. The oldest of the two, "Feature/Add
> > PdxType
> > > benchmark and additional framework flexibility" was an attempt to
> > quantify
> > > and maintain the improvement in performance for PdxType creation when
> > large
> > > numbers of PdxTypes already exist, and to allow the passing of
> additional
> > > system properties to the VMs hosting the servers in order to change the
> > log
> > > level and prevent the benchmark measuring how long it takes to log
> > PdxType
> > > creation rather than actual time taken to create new PdxTypes. This PR
> > has
> > > been open for a very long time, so it's possible that the changes
> > regarding
> > > passing additional system properties to the VMs are now outdated or
> > > unnecessary, but the actual benchmarks themselves still have some
> value.
> > >
> > > The second PR, "Added benchmarks for aggregate functions" contains 16
> new
> > > benchmarks related to aggregate OQL queries, (8 each for Partitioned
> and
> > > Replicated regions), which were added following work in that area by
> the
> > > Commons team. The build is currently marked as failing, but this is due
> > to
> > > a timeout rather than an actual build failure, as the number of
> > benchmarks
> > > added increased the total time to build beyond the currently configured
> > > timeout. Adding such a large number of additional benchmarks will
> > probably
> > > also noticeably increase the time it takes benchmarks to run, which
> bears
> > > consideration.
> > >
> > > I hope this helps shed some light for people who may look over those
> PRs.
> > >
> > > On Wed, Jan 22, 2020 at 11:36 AM Dan Smith  wrote:
> > >
> > > > Hi,
> > > >
> > > > I noticed we have some old outstanding PRs for the geode-benchmarks
> > > > project. Are any of these things we want to merge or should we close
> > them
> > > > out?
> > > >
> > > > https://github.com/apache/geode-benchmarks/pulls
> > > >
> > > > -Dan
> > > >
> > >
> >
>

Re: [DISCUSS] include geode-benchmarks in 1.12 release

[Robert Houghton]

Let's not vote until there is a call to vote, folks...



On Thu, Jan 16, 2020, 18:31 Jacob Barrett  wrote:

> I would characterize my vote as 0. I really don’t care either way. Just
> sharing I think they have no value in a release.
>
> > On Jan 16, 2020, at 6:08 PM, Owen Nichols  wrote:
> >
> > Geode PMC has 52 members.  If this were a vote, it looks like the
> results would have been:
> > +1: 2 (Anthony, Dan)
> > -1: 1 (Jake)
> >
> > If the next release manager were to go ahead and put geode-benchmarks in
> the Geode 1.12.0 source release, at least 3 PMC members would need to be
> willing to vote +1.  So it sounds like we need a few more of the other 49
> PMC members to weigh in on this discussion.
> >
> > To summarize so far:
> >
> > Proposal:
> > - add a geode-benchmarks-n.n.n-src.tgz artifact to all Geode releases
> going forward, starting with 1.12.0
> >
> > Arguments in favor:
> > - why not?
> > - it’s already public
> > - we should default to including all things
> > - it might be of interest to the user community
> > - it might encourage contributions back to further improve it
> > - it is required by CI, which is already included
> > - Apache mandates that source releases must include test code too
> >
> > Arguments against:
> > - doing nothing is less work
> > - it will burden PMC members with additional work to validate and vote
> on RCs
> > - nobody outside the dev community has asked for it to be included
> > - maybe it’s not ready
> > - maybe it’s not documented well enough
> > - it’s not needed to use Geode
> > - Apache's legal separation between dev stuff and public release stuff
> > - legal or license review may be not have been conducted yet
> >
> >
> >>> On Jan 16, 2020, at 4:48 PM, Dan Smith  wrote:
> >>>
> >>> If geode-benchmarks is included, that implies that an RC cannot be
> >> approved until reviewers can successfully run the benchmark suite from
> the
> >> geode-benchmarks source distribution.  Is that what we want?
> >>
> >> I think it would be sufficient to run the tests of the benchmarks, eg
> >> ./gradlew test
> >>
> >>> Deploying CI pipelines and running Benchmarks seems like a prime
> example
> >> of things we’d be happy to help others in the community with on the dev
> >> list — but not something we would expect questions about on the user
> list.
> >>
> >> I think it would be valuable to share our benchmarks with the geode user
> >> community. The benchmark framework itself (the harness module) is a
> fairly
> >> generic benchmarking framework than can be used to benchmark anything
> that
> >> can be spun up using java. The geode-benchmark module has geode
> benchmarks
> >> that could be used for testing specific hardware, for example.
> >>
> >> -Dan
> >>
> >>> On Thu, Jan 16, 2020 at 12:37 PM Owen Nichols 
> wrote:
> >>>
> >>> When voting on RC candidates, PMC members "are required to download the
> >>> signed source code package, compile it as provided, and test the
> resulting
> >>> executable on their own platform”.
> >>>
> >>> If geode-benchmarks is included, that implies that an RC cannot be
> >>> approved until reviewers can successfully run the benchmark suite from
> the
> >>> geode-benchmarks source distribution.  Is that what we want?
> >>>
> >>> Similarly, if CI is included, that seems to imply that an RC cannot be
> >>> approved until reviewers can stand up their own pipeline from the
> geode/ci
> >>> source distribution.  Is that what we want?
> >>>
> >>> So far there doesn’t seem to be consensus on what to include in a Geode
> >>> source release, but let’s keep in mind that anything we add to the
> release
> >>> becomes an Act Of The Foundation and is held to a higher standard.
> Apache
> >>> makes a clear distinction between between development activity and
> official
> >>> releases to the public.  Development activity is anything that should
> stay
> >>> within the dev list.  Deploying CI pipelines and running Benchmarks
> seems
> >>> like a prime example of things we’d be happy to help others in the
> >>> community with on the dev list — but not something we would expect
> >>> questions about on the user list.
> >>>
>  On Jan 16, 2020, at 10:23 AM, Dan Smith  wrote:
> 
>  We are supposed to be including all of the source necessary to test
> Geode
>  in the source release [1] - I think that would include benchmarks as
> >>> well.
> 
>  I don't really see any compelling reason *not* to include the
> benchmarks,
>  let's go ahead and get them into our source release!
> 
>  [1]
> 
> >>>
> http://www.apache.org/legal/release-policy.html#what-must-every-release-contain
> 
>  On Wed, Jan 15, 2020 at 10:26 PM Owen Nichols 
> >>> wrote:
> 
> > +1 for no changes
> >
> > On Wed, Jan 15, 2020 at 5:57 PM Jacob Barrett 
> >>> wrote:
> >
> >> We can live in areas of gray that don’t require any changes. Nobody
> is
> >> asking for benchmarks so let’s not do work to add them. Nobody is
> >> 

Re: RFC - Logging to Standard Out

[Robert Houghton]

+1

On Wed, Jan 8, 2020, 12:39 Jacob Barrett  wrote:

> Please see RFC for Logging to Standard Out.
>
> https://cwiki.apache.org/confluence/display/GEODE/Logging+to+Standard+Out
>  >
>
> Please comment by 1/21/2020.
>
> Thanks,
> Jake
>
>

Re: [DISCUSS] Proposal to require linear commit history on develop

[Robert Houghton]

@onichols
Correction: The 'git-flow' workflow that Geode follows is to merge a
release commit to master and to develop.

On Thu, Dec 19, 2019 at 4:51 PM Owen Nichols  wrote:

> Dan, to address some of your concerns:
>
> This proposal is to block merge commit on develop only.  The release
> process makes a merge commit to master only.
>
> This proposal explicitly allows reasonably separate and well written
> commits in a single PR.  For this case simply choose Rebase & Merge, and
> all of the commits in the PR will be brought to develop individually (no
> merge commit wrapping them).
>
> If you still feel strongly that merge commits should be allowed on
> develop, would you consider approving 1) to prevent accidental GitHub
> accidents, but veto only 2) to allow merge commits to be explicitly created
> manually when explicitly desired?
>
> > On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> >
> > -1 to (1) and (2).
> >
> > I think merge commits are appropriate in certain circumstances, so I
> don't
> > think we should make a blanket restriction. In fact I think our release
> > process involves some merges.
> >
> > I think setting standards on what is reasonable to be an individual
> commit
> > will do a lot more to clean up our history than blocking merges. We'd
> > rather not see commits like "Spotless Apply" in the history, but if
> > reasonably separate and well written commits come in as part of a merge,
> I
> > think that's fine.
> >
> > -Dan
> >
> > On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao  wrote:
> >
> >> +1
> >>
> >> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols  wrote:
> >>
> >>> I’d like to advance this topic from an informal request/discussion to a
> >>> discussion of a concrete proposal.
> >>>
> >>> To recap, it sounds like there is general agreement that commit history
> >> on
> >>> develop should be linear (no merge commits), and that the biggest
> >> obstacle
> >>> to this is that the PR merge button in GitHub creates a merge commit by
> >>> default.
> >>>
> >>> I propose the following changes:
> >>> 1) Change GitHub settings to remove the ability to create a merge
> commit.
> >>> This will make Squash & Merge the default.
> >>>
> >>> 2) Change GitHub settings to require linear history on develop.  This
> >>> prevents merge commits via command-line (not recommended, but wiki
> still
> >>> has instructions for merging PRs this way).
> >>>
> >>> 3) Update the PR template to change the text "Is your initial
> >> contribution
> >>> a single, squashed commit” to “Is your initial contribution squashed
> for
> >>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
> >>> ‘fix’ commit)"
> >>>
> >>> For clarity, I am proposing no-change in the following areas:
> >>> i) Leave Rebase & Merge as an option for PRs that have been structured
> to
> >>> benefit from it (this can make it easier in a bisect to see whether the
> >>> refactoring or the “fix” broke something).
> >>> ii) Leave existing wording in the wiki as-is [stating that squashing is
> >>> preferred].
> >>>
> >>>
> >>> Please comment via this email thread.
> >>> -Owen
> >>>
> >>>
> >>>
>  On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
> 
>  I think it's already resolved Udo ;)
> 
>  Here's the problem, if I fixup a dunit test by removing all uses of
> >>> "this."
>  and I rename the dunit test, then git doesn't remember that the file
> >> is a
>  rename -- it forever afterwards interprets it as a new file that I
> >>> created
>  if I touch more than 50% of the lines (which "this." can easily do).
> If
> >>> we
>  squash two commits: the rename and the cleanup of that dunit test --
> >> then
>  we effectively lose the history of that file and it shows that I
> >> created
> >>> a
>  new file.
> 
>  Also for the record, I've been working on Geode since the beginning
> >> and I
>  was never made aware of this change in our process. I never voted on
> >> it.
>  I'm not a big fan of changing various details in our process every
> >> single
>  week. It's very easy to miss these discussions unless someone points
> it
> >>> out
>  to me.
> 
>  On Mon, Dec 16, 2019 at 10:34 AM Udo Kohlmeyer  >
>  wrote:
> 
> > I'm not sure what this discussion is about... WE, as a community,
> have
> > agreed in common practices, in two place no less...
> >
> > 1) Quoting our PR template
> >
> >
> > For all changes:
> >
> > *
> >
> >   Is there a JIRA ticket associated with this PR? Is it referenced in
> >   the commit message?
> >
> > *
> >
> >   Has your PR been rebased against the latest commit within the
> >> target
> >   branch (typically|develop|)?
> >
> > *
> >
> >   ***Is your initial contribution a single, squashed commit?*
> >
> > *
> >
> >   Does|gradlew build|run cleanly?
> >
> > *
> >
> >   Have you written or updated unit tests to 

Re: [DISCUSS] abandon branch protection rules

[Robert Houghton]

@Jacob Barrett  I have some ideas on this. Want to
look at in on Thursday with me?

On Sat, Dec 28, 2019 at 5:28 AM Jacob Barrett  wrote:

> Let’s find a way to get the ci, docs, and other directories not effected
> by tests out of this testing hold.
>
> > On Dec 27, 2019, at 3:23 PM, Nabarun Nag  wrote:
> >
> > Please maintain the branch protection rules.
> > Waiting for reviews and Unit tests to pass does not stifle productivity,
> > but prevents us from making mistakes that are detrimental to the entire
> > community. If I am not mistaken, we still have pushed code which broke
> > builds and regressions. I would suggest not removing but improving / add
> > ons to the checks to prevent such issues from happening again.
> >
> > Also, personally I feel that CI code can separated out of geode code base
> > as they have no tests to run and they can circumvent the unit test pass
> > criteria.
> >
> > I would just like to say that fixing tests is all of our responsibility,
> > not a particular group of developers.
> >
> > Regards
> > Naba
> >
> >
> >
> >
> >
> >
> >
> >> On Fri, Dec 27, 2019 at 3:05 PM Jason Huynh  wrote:
> >>
> >> Just to add more flavor to my previous response... I currently have a PR
> >> open that modified a method signature that touched a few WAN tests.  It
> was
> >> a simple change, removing an unused parameter.  StressNewTest failed
> and I
> >> had to spend another day figuring out 10 or so different failures.  A
> waste
> >> of time?  Maybe..  At first, I wasn't going to continue, but after
> trying a
> >> few things, it looks like the tests installed a listener that was
> hampering
> >> other tests.  At the end (soon once it gets reviewed/merged), we end up
> >> with a Green PR and hopefully have unblocked others on these specific
> tests
> >> in the future.
> >>
> >>> On Fri, Dec 27, 2019 at 2:58 PM Jason Huynh  wrote:
> >>>
> >>> I feel the frustration at times, but I do also think the ci/pipelines
> are
> >>> improving, breaking less often.  I'm ok with the way things are for the
> >>> moment
> >>>
> >>> On Fri, Dec 27, 2019 at 1:47 PM Owen Nichols 
> >> wrote:
> >>>
>  In October we agreed to require at least 1 reviewer and 4 passing PR
>  checks before a PR can be merged.  Now that we’re tried it for a few
>  months, do we like it?
> 
>  I saw some strong opinions on the dev list recently:
> 
> > Changes to the infrastructure to flat out prevent things that should
> >> be
>  self policing is annoying. This PR review lock we have had already
> cost
> >> us
>  valuable time waiting for PR pipelines to pass that have no relevance
> to
>  the commit, like CI work. I hate to see process enforced that keeps us
> >> from
>  getting work done when necessary.
> 
> 
>  and
> 
> > I think we're getting more and more bureaucratic in our process and
>  that it stifles productivity.  I was recently forced to spend three
> days
>  fixing tests in which I had changed an import statement before they
> >> would
>  pass stress testing.  I'm glad the tests now pass reliably but I was
> >> very
>  frustrated by the process.
> 
> 
>  Just wondering if others feel the same way.  Is it time to make some
>  changes?
> 
>  -Owen
> >>>
> >>>
> >>
>

Re: PR Checks issues?

[Robert Houghton]

We could have the content of the PR jobs check that actual sources changed,
before doing any real 'work' or 'validation' of the change. If the patch is
only CI or readme (not docs, those get compiled) then we can exit with a
clean state. Saves time, and money!

If the issue is the lack of Concourse seeing the PR, there are several
issues and potential resolutions that I am happy to go into offline.

On Fri, Dec 13, 2019 at 11:31 AM Michael Oleske  wrote:

> Hi Geode Dev!
>
> This PR https://github.com/apache/geode/pull/4406 is a change to the
> readme.  However it took 3 empty commits to get it to go green enough to be
> allowed to be merged.  This seems odd, especially with just a readme
> change.  Is there something going with how CI works for PRs?  This was an
> incredibly frustrating experience.
>
> Thanks!
> -michael
>

Re: [VOTE] Release candidate for Apache Geode version 1.11.0.RC3.

[Robert Houghton]

@udo, if one needs to use a strong word like 'offender', then the offender
is the one using an internal API. Geode is under no obligation to maintain
or "fix" these for any project.

Is there a Jira, github issue, or pull-request to promote the internal
class to the public space? Is there a feature request to uncouple this
class you want to use in Spring?

Is Spring the tail wagging the dog for Geode releases?


On Wed, Dec 4, 2019, 17:59 Udo Kohlmeyer  wrote:

> @Dan,
>
> I will add my -1 to this.
>
> I understand your argument of "let's solve the problem by removing  the
> offender".
>
> But in reality who is the offender? Is it the one class that is using an
> "internal" api OR is it the implementation itself that is to tightly
> coupled that extending it is impossible?
>
> STDG right now is trying to create a test, to test functionality that
> requires it to inject/mock a Pool. The smallest piece of the code, not
> the WHOLE PoolManager. But the system does not allow for the injection
> of a `Pool` (which btw all the methods on the `PoolManagerImpl` has as
> arguments). It casts every generic `Pool` to its implementation. This is
> a very limiting factor.
>
> I understand that delaying a release is not optimal, but how much effort
> to we believe it to be to clean up the code that so tightly couples
> implementations together.
>
> I think we also must not forget that  John, like many of us, is a
> committer and PMC member on Geode. He also happens to be the main
> committer on SDG, SBDG, SSDG, STDG. But if he feels that a release
> should not go out without fixing a limiting feature to the Geode
> project, then he may do so.
>
> I also feel that this is a limiting factor.
>
> The only difference is that I am not the main committer on the Spring
> data projects for Geode. John is merely trying to get the best outcome
> for both projects.
>
> --Udo
>
> On 12/4/19 4:39 PM, Dan Smith wrote:
> >> Quite frankly the reasons STDG (or dependent projects downstream like
> SDG,
> >> SBDG, SSDG) are doing what it is (they are) doing is irrelevant to point
> >> articulated in the description of GEODE-753.
> >>
> > What bothers me here is not your suggestions in GEODE-1753, but the fact
> > that you are vetoing a Geode release because STDG is using an internal
> > Geode method and had a problem.
> >
> > How hard is it to remove the use of PoolManagerImpl from STDG? If we can
> > fix the issue there, that seems better than putting bandaids into the
> > release branch so STDG can continue to use internal APIs.
> >
> > -Dan
> >
>

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

[Robert Houghton]

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

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

On Fri, Nov 22, 2019, 14:19 Jason Huynh  wrote:

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

[VOTE] Fix bad-merge of GEODE-7488

[Robert Houghton]

I was overzealous in a merge to Geode, and got us into a chicken-and-egg
issue for PRs and reverts. Calling a vote to override the GitHub merge
button restriction via commiter privileges, to merge the fix in
https://github.com/apache/geode/pull/4360

Re: PR jobs failing to start

[Robert Houghton]

This was me. I merged a CI change that was missing an SSH option flag. We
need to override the PR merge check to fix. I have paused PR jobs until
then.

On Fri, Nov 22, 2019 at 11:52 AM Kirk Lund  wrote:

> FYI: Looks like all PR test and build jobs are failing with:
>
> Warning: Permanently added '10.0.0.61' (ECDSA) to the list of known hosts.
> ServerAliveCountMax=5: No such file or directory
> capture-call-stacks.sh
>