Re: [DISCUSS] proposal to pare down old-version testing

[Anilkumar Gingade]

+1 
Thanks for bringing this and taking care of this.

-Anil.


On 1/3/22, 10:41 AM, "Dan Smith"  wrote:

Looking at KnownVersion.java - we did make protocol changes in 1.12.1 and 
1.13.2. So, my suggestion would be to keep 1.12.0 and 1.13.1, but dop all the 
other patch versions that aren't the latest.

-Dan

From: Dan Smith 
Sent: Monday, January 3, 2022 10:37 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] proposal to pare down old-version testing

+1 - this seems reasonable to me. If we do make a protocol change in a 
patch, we could potentially keep around an older patch version just in that 
specific case, but otherwise I think this makes sense.

-Dan

From: Anthony Baker 
Sent: Thursday, December 23, 2021 8:53 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] proposal to pare down old-version testing

Interesting data point:  40% of maven central downloads last month were for 
version 1.4.0. Of course those numbers can be easily skewed by CI bots, but 
still!

@Owen, I think your suggestion nicely improves practicality while 
continuing to support strong compatibility. In many cases it’s quite a bit 
easier to upgrade the Geode server cluster compared to potentially many, many 
client applications.  Supporting older client versions gives users time to 
upgrade, quicker access to bug fixes, and helps avoids downtime.

+1

Anthony


> On Dec 22, 2021, at 7:13 PM, Owen Nichols  wrote:
>
> Since adopting our N-2 support policy, the list of released versions in 
/settings.gradle has ballooned to over 30 entries [1].
>
> CI tests use this list to confirm that we don’t break rolling upgrade 
ability or compatibility with older clients, but some of these tests don’t seem 
to scale well: PR#7203 to add the most recent 3 releases (bringing the total to 
33) is unable to pass CI after 8 tries.
>
> Possible solutions fall into two categories: keep the full list and throw 
developers and/or more hardware at the struggling tests, or concede that 
testing every version is not a scalable approach and find ways to shorten the 
list, e.g. randomly select a subset of old versions at runtime, or manually 
pare down the list.
>
> I propose to shorten the list [2] by keeping only the latest patch for 
each minor (unless the client or server protocol version has changed, so also 
keep the patch prior to 1.12.1 and prior to 1.13.2).  As long as a patch 
release doesn’t change the client or server protocol version, I see low value 
in testing upgrades from every patch version to every future version forever.  
The months between patch releases already provide plenty of upgrade coverage on 
that specific patch, then we can move on to the next…even if there could 
somehow be a corner-case where transitive property of upgradability doesn’t 
hold, most users probably take the latest-to-latest upgrade path anyway, which 
will always be tested.
>
> Let’s keep discussion open until 3PM PST Jan 5.  In case of no response, 
I will assume lazy consensus and update settings.gradle as proposed [2].
>
>
>
> [1] Current list from 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fsettings.gradle%23L72-L101&data=04%7C01%7Cagingade%40vmware.com%7C8b7e0a65ccc34f37b8a608d9cee8b189%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637768321075908672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Kla9OIPhvSC54LCeIQ%2FhrjOcpn7K39cVBHfAWJlbVh8%3D&reserved=0
 :
> 1.0.0-incubating
> 1.1.0
> 1.1.1
> 1.2.0
> 1.3.0
> 1.4.0
> 1.5.0
> 1.6.0
> 1.7.0
> 1.8.0
> 1.9.0
> 1.9.1
> 1.9.2
> 1.10.0
> 1.11.0
> 1.12.0
> 1.12.1
> 1.12.2
> 1.12.3
> 1.12.4
> 1.12.5
> 1.12.6
> 1.12.7*
> 1.13.0
> 1.13.1
> 1.13.2
> 1.13.3
> 1.13.4
> 1.13.5
> 1.13.6*
> 1.14.0
> 1.14.1
> 1.14.2*
> *=released, but not yet added to settings.gradle due to PR#7203 not able 
to pass CI due to size of version list
>
> [2] Proposed shortlist:
> 1.1.1
> 1.2.0
> 1.3.0
> 1.4.0
> 1.5.0
> 1.6.0
> 1.7.0
> 1.8.0
> 1.9.2
> 1.10.0
> 1.11.0
> 1.12.0
> 1.12.7
> 1.13.1
> 1.13.6
> 1.14.2
>

Re: PR to add unique ID to DUnit log output

[Dale Emery]

Hi Jens and all,

A possibility to consider: Instead of generating an arbitrary-but-unique ID, 
use an existing identifier from the test environment… such as the ID in the 
test worker directory name. That might make it easier to map a given log line 
to the other artifacts from the test. So if the test worker dir was 
test-worker-98, then use 98 (or 98) as the ID in the logs. 
ProcessManager could get the ID by parsing the current working directory. Or we 
could change the “test isolation” stuff to set an environment variable that 
ProcessManager could use.

Another possibility to look into: Teach Gradle to distinguish different 
instances of the same test class, so that it doesn’t merge the logs. I don’t 
know whether this is possible, but it may be worth exploring. (The challenge is 
that Gradle assumes it will run a test class only once, and includes code to 
ensure that. We bypass that code in order to run a test class multiple times, 
but we don’t change how Gradle decides what log to append a test process’s 
output to.)

Dale

From: Jens Deppe 
Date: Monday, January 3, 2022 at 1:03 PM
To: dev@geode.apache.org 
Subject: PR to add unique ID to DUnit log output
Hi All.

Just a heads up that I have a PR up 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7232&data=04%7C01%7Cdemery%40vmware.com%7C73f6134e922f4a69cbda08d9cefc883d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637768406280110715%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=t5iFtadRhKMy3jHmplgrYt7TWdtVeVPDSHCUhBpSj2g%3D&reserved=0)
 which, if merged, will slightly change the log output from DUnit runs. The PR 
simply adds a 4 character unique ID to the log line. As in:

[vm0-51ec] [info 2021/12/24 15:43:54.367 UTC  ; tid=0x1d] Reinitializing JarDeploymentService with 
new working directory: null

[vm0-47b7] [info 2021/12/24 15:43:54.416 UTC   tid=0x1d] Reinitializing JarDeploymentService with 
new working directory: null

[vm1-47b7] [info 2021/12/24 15:43:54.431 UTC   tid=0x1d] Received method: 
org.apache.geode.test.dunit.internal.IdentifiableCallable.call with 0 args on 
object: IdentifiableCallable(0:start locator in vm0)

The ID is intended to be unique per test run when tests are run repeatedly.

I did this to assist in debugging test output from repeated test runs 
(StressNewTest) where individual tests’ log lines are simply interleaved making 
debugging very difficult.

The change, unfortunately, does not affect log lines generated from the test VM 
but only from VMs launched by the ProcessManager. If anyone has ideas how to do 
that, I’d love to hear them.

Well, actually one approach I’ve used is related to this PR 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7231&data=04%7C01%7Cdemery%40vmware.com%7C73f6134e922f4a69cbda08d9cefc883d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637768406280110715%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=EeSqSSXhmSnkW0548njAwUPquR9DkRjuZ7m4jcS%2B2c0%3D&reserved=0)
 which lets one name ExecutorServiceRule threads. Using this, I can use the 
ProcessManager.ID to name the executor threads and thus that ID becomes visible 
when the thread name is logged. Kinda hacky, but it’s still effective.

Anyway, this is not a call to review (although you’re welcome to do so) but 
just a FYI.

--Jens

Re: PR to add unique ID to DUnit log output

[Mark Hanson]

I support Jens' change until something better is identified. I think just this 
minor change will have a big impact on our ability to decipher what is going on 
in dunit tests. Thanks for making the change Jens!

Mark

On 1/4/22, 10:44 AM, "Dale Emery"  wrote:

Hi Jens and all,

A possibility to consider: Instead of generating an arbitrary-but-unique 
ID, use an existing identifier from the test environment… such as the ID in the 
test worker directory name. That might make it easier to map a given log line 
to the other artifacts from the test. So if the test worker dir was 
test-worker-98, then use 98 (or 98) as the ID in the logs. 
ProcessManager could get the ID by parsing the current working directory. Or we 
could change the “test isolation” stuff to set an environment variable that 
ProcessManager could use.

Another possibility to look into: Teach Gradle to distinguish different 
instances of the same test class, so that it doesn’t merge the logs. I don’t 
know whether this is possible, but it may be worth exploring. (The challenge is 
that Gradle assumes it will run a test class only once, and includes code to 
ensure that. We bypass that code in order to run a test class multiple times, 
but we don’t change how Gradle decides what log to append a test process’s 
output to.)

Dale

From: Jens Deppe 
Date: Monday, January 3, 2022 at 1:03 PM
To: dev@geode.apache.org 
Subject: PR to add unique ID to DUnit log output
Hi All.

Just a heads up that I have a PR up 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7232&data=04%7C01%7Chansonm%40vmware.com%7C7dbe022839934e3446e308d9cfb24749%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769186901894631%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rE6JFmAGYeINPRtBXccHx%2Bqy5pp2Wz2Br5xR2iRncWY%3D&reserved=0)
 which, if merged, will slightly change the log output from DUnit runs. The PR 
simply adds a 4 character unique ID to the log line. As in:

[vm0-51ec] [info 2021/12/24 15:43:54.367 UTC  ; tid=0x1d] Reinitializing JarDeploymentService with 
new working directory: null

[vm0-47b7] [info 2021/12/24 15:43:54.416 UTC   tid=0x1d] Reinitializing JarDeploymentService with 
new working directory: null

[vm1-47b7] [info 2021/12/24 15:43:54.431 UTC   tid=0x1d] Received method: 
org.apache.geode.test.dunit.internal.IdentifiableCallable.call with 0 args on 
object: IdentifiableCallable(0:start locator in vm0)

The ID is intended to be unique per test run when tests are run repeatedly.

I did this to assist in debugging test output from repeated test runs 
(StressNewTest) where individual tests’ log lines are simply interleaved making 
debugging very difficult.

The change, unfortunately, does not affect log lines generated from the 
test VM but only from VMs launched by the ProcessManager. If anyone has ideas 
how to do that, I’d love to hear them.

Well, actually one approach I’ve used is related to this PR 
(https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7231&data=04%7C01%7Chansonm%40vmware.com%7C7dbe022839934e3446e308d9cfb24749%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769186901894631%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=L5hedKNBMIilghxkFqAgTs%2B4SXOPHapSEcnKy3HEcmM%3D&reserved=0)
 which lets one name ExecutorServiceRule threads. Using this, I can use the 
ProcessManager.ID to name the executor threads and thus that ID becomes visible 
when the thread name is logged. Kinda hacky, but it’s still effective.

Anyway, this is not a call to review (although you’re welcome to do so) but 
just a FYI.

--Jens

Re: [DISCUSS] proposal to pare down old-version testing

[Alexander Murmann]

+1 Great compromise! Let's not forget about this whenever we change the protocol


--
Please provide anonymous feedback for me 
here.

From: Anilkumar Gingade 
Sent: Tuesday, January 4, 2022 07:09
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] proposal to pare down old-version testing

+1
Thanks for bringing this and taking care of this.

-Anil.


On 1/3/22, 10:41 AM, "Dan Smith"  wrote:

Looking at KnownVersion.java - we did make protocol changes in 1.12.1 and 
1.13.2. So, my suggestion would be to keep 1.12.0 and 1.13.1, but dop all the 
other patch versions that aren't the latest.

-Dan

From: Dan Smith 
Sent: Monday, January 3, 2022 10:37 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] proposal to pare down old-version testing

+1 - this seems reasonable to me. If we do make a protocol change in a 
patch, we could potentially keep around an older patch version just in that 
specific case, but otherwise I think this makes sense.

-Dan

From: Anthony Baker 
Sent: Thursday, December 23, 2021 8:53 AM
To: dev@geode.apache.org 
Subject: Re: [DISCUSS] proposal to pare down old-version testing

Interesting data point:  40% of maven central downloads last month were for 
version 1.4.0. Of course those numbers can be easily skewed by CI bots, but 
still!

@Owen, I think your suggestion nicely improves practicality while 
continuing to support strong compatibility. In many cases it’s quite a bit 
easier to upgrade the Geode server cluster compared to potentially many, many 
client applications.  Supporting older client versions gives users time to 
upgrade, quicker access to bug fixes, and helps avoids downtime.

+1

Anthony


> On Dec 22, 2021, at 7:13 PM, Owen Nichols  wrote:
>
> Since adopting our N-2 support policy, the list of released versions in 
/settings.gradle has ballooned to over 30 entries [1].
>
> CI tests use this list to confirm that we don’t break rolling upgrade 
ability or compatibility with older clients, but some of these tests don’t seem 
to scale well: PR#7203 to add the most recent 3 releases (bringing the total to 
33) is unable to pass CI after 8 tries.
>
> Possible solutions fall into two categories: keep the full list and throw 
developers and/or more hardware at the struggling tests, or concede that 
testing every version is not a scalable approach and find ways to shorten the 
list, e.g. randomly select a subset of old versions at runtime, or manually 
pare down the list.
>
> I propose to shorten the list [2] by keeping only the latest patch for 
each minor (unless the client or server protocol version has changed, so also 
keep the patch prior to 1.12.1 and prior to 1.13.2).  As long as a patch 
release doesn’t change the client or server protocol version, I see low value 
in testing upgrades from every patch version to every future version forever.  
The months between patch releases already provide plenty of upgrade coverage on 
that specific patch, then we can move on to the next…even if there could 
somehow be a corner-case where transitive property of upgradability doesn’t 
hold, most users probably take the latest-to-latest upgrade path anyway, which 
will always be tested.
>
> Let’s keep discussion open until 3PM PST Jan 5.  In case of no response, 
I will assume lazy consensus and update settings.gradle as proposed [2].
>
>
>
> [1] Current list from 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fsettings.gradle%23L72-L101&data=04%7C01%7Camurmann%40vmware.com%7C579b50b0cacc49ed54df08d9cf9436a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769057757912491%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8%2F%2BuhyHnyI0ZPbY4Y%2F0vRG6O%2Bi3wuSf3%2BeISAvg0DR8%3D&reserved=0
 :
> 1.0.0-incubating
> 1.1.0
> 1.1.1
> 1.2.0
> 1.3.0
> 1.4.0
> 1.5.0
> 1.6.0
> 1.7.0
> 1.8.0
> 1.9.0
> 1.9.1
> 1.9.2
> 1.10.0
> 1.11.0
> 1.12.0
> 1.12.1
> 1.12.2
> 1.12.3
> 1.12.4
> 1.12.5
> 1.12.6
> 1.12.7*
> 1.13.0
> 1.13.1
> 1.13.2
> 1.13.3
> 1.13.4
> 1.13.5
> 1.13.6*
> 1.14.0
> 1.14.1
> 1.14.2*
> *=released, but not yet added to settings.gradle due to PR#7203 not able 
to pass CI due to size of version list
>
> [2] Proposed shortlist:
> 1.1.1
> 1.2.0
> 1.3.0
> 1.4.0
> 1.5.0
> 1.6.0
> 1.7.0
> 1.8.0
> 1.9.2
> 1.10.0
> 1.11.0
> 1.12.0
> 1.12.7
> 1.13.1
> 1.13.6
> 1.14.2
>

[VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

[Dan Smith]

Hello Geode Dev Community,

This is a release candidate for Apache Geode Kafka Connector version 1.1.0. 
This contains a bump to log4j 2.16.

Please do a review and give your feedback.

Voting deadline:
3PM PST Tuesday, Jan 11, 2022.

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

Source and Binary Distributions: 
https://dist.apache.org/repos/dist/dev/geode/kafka-connector-1.1.0/
Github: https://github.com/apache/geode-kafka-connector/tree/rel/v1.1.0

Command to build the connector:
mvn package

Thanks!
-Dan

Re: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

[Dan Smith]

This is the same release I started a VOTE on in December and canceled due to 
Christmas. Here was my earlier comment on where this release is coming from:

On Dec 16, 2021 11:33 AM, Dan Smith  wrote:
This is related to the earlier discussion about the kafka connector. This code 
was already linked from the confluent hub, but I want to make sure we're in 
compliance with the apache release policy.

To create this release, I took the source code and binaries Naba mentioned and 
compared them with the rel/1.1.0 source tag and the results of building that 
tag. They were identical except for the build date in the metadata.json file 
for the binary.

I added gpg and sha256sum signatures and uploaded the artifacts to the apache 
staging repo for you all to review.

I checked out the license, notice, and source code headers and everything 
looked ok to me from that perspective.

In the future, we may want to release this in sync with the rest of the geode 
artifacts, but for now I'm just trying to create an official release that 
follows the release policy - https://www.apache.org/legal/release-policy.html


From: Dan Smith 
Sent: Tuesday, January 4, 2022 2:18 PM
To: dev@geode.apache.org 
Subject: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

Hello Geode Dev Community,

This is a release candidate for Apache Geode Kafka Connector version 1.1.0.
This contains a bump to log4j 2.16.

Please do a review and give your feedback.

Voting deadline:
3PM PST Tuesday, Jan 11, 2022.

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

Source and Binary Distributions: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fgeode%2Fkafka-connector-1.1.0%2F&data=04%7C01%7Cdasmith%40vmware.com%7C4ea1c82a07a348a28efe08d9cfd03470%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769315404324461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=vdEwrWUbhikvadJb1AYHQt2wannGXODwa%2Ft2l2TXtnQ%3D&reserved=0
Github: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-kafka-connector%2Ftree%2Frel%2Fv1.1.0&data=04%7C01%7Cdasmith%40vmware.com%7C4ea1c82a07a348a28efe08d9cfd03470%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769315404324461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=6KCZtjdYZ%2FhCCMO6H6NGeSWL0DcQBYvkS3xRxWkh5B8%3D&reserved=0

Command to build the connector:
mvn package

Thanks!
-Dan

Re: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

[Nabarun Nag]

+1
Checked with building source and running tests.

Regards
Naba

From: Dan Smith 
Sent: Tuesday, January 4, 2022 2:37 PM
To: dev@geode.apache.org 
Subject: Re: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

This is the same release I started a VOTE on in December and canceled due to 
Christmas. Here was my earlier comment on where this release is coming from:

On Dec 16, 2021 11:33 AM, Dan Smith  wrote:
This is related to the earlier discussion about the kafka connector. This code 
was already linked from the confluent hub, but I want to make sure we're in 
compliance with the apache release policy.

To create this release, I took the source code and binaries Naba mentioned and 
compared them with the rel/1.1.0 source tag and the results of building that 
tag. They were identical except for the build date in the metadata.json file 
for the binary.

I added gpg and sha256sum signatures and uploaded the artifacts to the apache 
staging repo for you all to review.

I checked out the license, notice, and source code headers and everything 
looked ok to me from that perspective.

In the future, we may want to release this in sync with the rest of the geode 
artifacts, but for now I'm just trying to create an official release that 
follows the release policy - 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.apache.org%2Flegal%2Frelease-policy.html&data=04%7C01%7Cnnag%40vmware.com%7Ceb251e5e0a4f42bf46e108d9cfd2db04%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769326818200926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=h%2BC1DQJsrqYTbs1nDOvdKvnr73SSeEtGEpMU3IyPS9o%3D&reserved=0


From: Dan Smith 
Sent: Tuesday, January 4, 2022 2:18 PM
To: dev@geode.apache.org 
Subject: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

Hello Geode Dev Community,

This is a release candidate for Apache Geode Kafka Connector version 1.1.0.
This contains a bump to log4j 2.16.

Please do a review and give your feedback.

Voting deadline:
3PM PST Tuesday, Jan 11, 2022.

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

Source and Binary Distributions: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fgeode%2Fkafka-connector-1.1.0%2F&data=04%7C01%7Cnnag%40vmware.com%7Ceb251e5e0a4f42bf46e108d9cfd2db04%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769326818200926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9Ompca49Q%2FGwVVxp6JwYFW%2FR03aAjAxz5TdQGACAaRw%3D&reserved=0
Github: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-kafka-connector%2Ftree%2Frel%2Fv1.1.0&data=04%7C01%7Cnnag%40vmware.com%7Ceb251e5e0a4f42bf46e108d9cfd2db04%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769326818200926%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ulk3r0c7kttqzeb9i1GGY%2Bmy8CVNdKNQE0gDrLgX8fE%3D&reserved=0

Command to build the connector:
mvn package

Thanks!
-Dan

LGTM as a gating job on PRs not actually gating?

[Donal Evans]

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://github.com/apache/geode/pull/7219#issuecomment-1005181341
The "passing" LGTM job for the PR showing newly introduced alerts: 
https://github.com/apache/geode/pull/7219/checks?check_run_id=4707178926

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: 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-1005181341&data=04%7C01%7Crhoughton%40vmware.com%7Cd6e65375f2f14cb1ecf808d9cfd8d5dd%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769352482495865%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ijOnxfdNO7jLa85akx%2BXoiQKteueZOhcfMXbxFN3BKM%3D&reserved=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%3D4707178926&data=04%7C01%7Crhoughton%40vmware.com%7Cd6e65375f2f14cb1ecf808d9cfd8d5dd%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769352482495865%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=4nfV3tde3CI79tAm8%2BKN3WYNHo8eWWY%2BePNeAeL204M%3D&reserved=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: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

[Owen Nichols]

Quibbles:
- artifact naming does not follow standard naming convention of 
THING-VERSION.tgz and THING-VERSION-src.tgz (also Geode decided to stop 
distributing .zip files years ago)
- not based on the latest Geode 1.12 patch.  I would like to see Geode 1.12.8 
picked up once it's available later this month.
- the log4j version 2.16.0 advertised in this release fixes only 2 of the 4 
recent log4j vulnerabilities.  I would prefer to see log4j 2.17.1.
- vote email is missing a link to release notes and a link to the KEYS file 
used to sign the release.
- artifact paths and email subject are missing "RC1" qualifier

Concerns:
- NOTICE and LICENSE are found inside a "doc" folder instead of at the top 
level of the artifact
- Some dependencies are missing from LICENSE.  While most deps are Apache2 and 
don't require a mention, LatencyUtils is BSD-2 and should be mentioned, and 
likely a few others from Geode's LICENSE need to be there as well because they 
are incorporated in source form into geode-core. 

Please consider above suggestions for next time.

+0

On 1/4/22, 2:19 PM, "Dan Smith"  wrote:

Hello Geode Dev Community,

This is a release candidate for Apache Geode Kafka Connector version 1.1.0. 
This contains a bump to log4j 2.16.

Please do a review and give your feedback.

Voting deadline:
3PM PST Tuesday, Jan 11, 2022.

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

Source and Binary Distributions: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fgeode%2Fkafka-connector-1.1.0%2F&data=04%7C01%7Conichols%40vmware.com%7C371195448ed74cdb909308d9cfd033e3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769315400140534%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZbgkfljvRh0McQAUL0nFvClIjW2xLq5jl8804lB7Txs%3D&reserved=0
Github: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-kafka-connector%2Ftree%2Frel%2Fv1.1.0&data=04%7C01%7Conichols%40vmware.com%7C371195448ed74cdb909308d9cfd033e3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769315400140534%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=PgJv8d4yla2DjGWrppCxIFIkbwmrYT7fp2i5iBiOjNo%3D&reserved=0

Command to build the connector:
mvn package

Thanks!
-Dan

Re: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

[Nabarun Nag]

As it is primarily created for Confluent Marketplace we need to follow the 
steps required for hosting in the marketplace, which included how things are to 
be named, folder structure etc.

Looking at the list of things to do and conflicts with Geode / Confluent 
requirements. We can remove it from the Apache domain and move it to internal 
open source repo like gpdb or rabbitMQ while keeping the Apache License. 
Alternatives can be the VMware or VMware-labs opensource orgs in Github.

We can definitely add the missing licenses and wait for 1.12.8 release of 
Apache Geode to update those dependencies.


Regards
Naba


From: Owen Nichols 
Sent: Tuesday, January 4, 2022 4:45 PM
To: dev@geode.apache.org 
Subject: Re: [VOTE] - Apache Geode Kafka Connector 1.1.0 - Take 2

Quibbles:
- artifact naming does not follow standard naming convention of 
THING-VERSION.tgz and THING-VERSION-src.tgz (also Geode decided to stop 
distributing .zip files years ago)
- not based on the latest Geode 1.12 patch.  I would like to see Geode 1.12.8 
picked up once it's available later this month.
- the log4j version 2.16.0 advertised in this release fixes only 2 of the 4 
recent log4j vulnerabilities.  I would prefer to see log4j 2.17.1.
- vote email is missing a link to release notes and a link to the KEYS file 
used to sign the release.
- artifact paths and email subject are missing "RC1" qualifier

Concerns:
- NOTICE and LICENSE are found inside a "doc" folder instead of at the top 
level of the artifact
- Some dependencies are missing from LICENSE.  While most deps are Apache2 and 
don't require a mention, LatencyUtils is BSD-2 and should be mentioned, and 
likely a few others from Geode's LICENSE need to be there as well because they 
are incorporated in source form into geode-core.

Please consider above suggestions for next time.

+0

On 1/4/22, 2:19 PM, "Dan Smith"  wrote:

Hello Geode Dev Community,

This is a release candidate for Apache Geode Kafka Connector version 1.1.0.
This contains a bump to log4j 2.16.

Please do a review and give your feedback.

Voting deadline:
3PM PST Tuesday, Jan 11, 2022.

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

Source and Binary Distributions: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fgeode%2Fkafka-connector-1.1.0%2F&data=04%7C01%7Cnnag%40vmware.com%7C446cbaa9b22e405c8ab908d9cfe4af60%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769403373246616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rIRSVkdDIlv0c2yFZDSXZe9fDFrOX0usjDtyyEDa0%2Bg%3D&reserved=0
Github: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-kafka-connector%2Ftree%2Frel%2Fv1.1.0&data=04%7C01%7Cnnag%40vmware.com%7C446cbaa9b22e405c8ab908d9cfe4af60%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637769403373246616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=3yvWDc7u%2FNi%2FkF5zXQKDYkkEOImzOMxWnDAgFzm8P%2B4%3D&reserved=0

Command to build the connector:
mvn package

Thanks!
-Dan