[GitHub] [geode] kohlmu-pivotal merged pull request #5169: 1.12 - GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal merged pull request #5169: URL: https://github.com/apache/geode/pull/5169 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode-native] moleske commented on a change in pull request #604: GEODE-8183: Update dockerfile versions
moleske commented on a change in pull request #604:
URL: https://github.com/apache/geode-native/pull/604#discussion_r431572960
##
File path: docker/Dockerfile
##
@@ -41,15 +42,15 @@ RUN apt-get update && \
update-alternatives --install /usr/bin/clang-tidyclang-tidy
/usr/bin/clang-tidy-${CLANG_VERSION} 999 && \
update-alternatives --install /usr/bin/clang-format clang-format
/usr/bin/clang-format-${CLANG_VERSION} 999
-ENV GEODE_VERSION 1.9.0
+ENV GEODE_VERSION 1.12.0
RUN wget
"https://www.apache.org/dyn/closer.cgi?action=download=geode/${GEODE_VERSION}/apache-geode-${GEODE_VERSION}.tgz;
--quiet -O - | \
tar xzf -
ENV RAT_VERSION 0.13
RUN wget
"https://www.apache.org/dyn/closer.cgi?action=download=creadur/apache-rat-${RAT_VERSION}/apache-rat-${RAT_VERSION}-bin.tar.gz;
--quiet -O - | \
tar xzf -
-ENV CMAKE_VERSION 3.12.2
+ENV CMAKE_VERSION 3.12.4
Review comment:
I stuck with 3.12 since that is min version we specify in the
CMakeLists.txt. It seemed worth keeping that in sync but maybe it doesn't
matter?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #604: GEODE-8183: Update dockerfile versions
pivotal-jbarrett commented on a change in pull request #604:
URL: https://github.com/apache/geode-native/pull/604#discussion_r431562733
##
File path: docker/Dockerfile
##
@@ -41,15 +42,15 @@ RUN apt-get update && \
update-alternatives --install /usr/bin/clang-tidyclang-tidy
/usr/bin/clang-tidy-${CLANG_VERSION} 999 && \
update-alternatives --install /usr/bin/clang-format clang-format
/usr/bin/clang-format-${CLANG_VERSION} 999
-ENV GEODE_VERSION 1.9.0
+ENV GEODE_VERSION 1.12.0
RUN wget
"https://www.apache.org/dyn/closer.cgi?action=download=geode/${GEODE_VERSION}/apache-geode-${GEODE_VERSION}.tgz;
--quiet -O - | \
tar xzf -
ENV RAT_VERSION 0.13
RUN wget
"https://www.apache.org/dyn/closer.cgi?action=download=creadur/apache-rat-${RAT_VERSION}/apache-rat-${RAT_VERSION}-bin.tar.gz;
--quiet -O - | \
tar xzf -
-ENV CMAKE_VERSION 3.12.2
+ENV CMAKE_VERSION 3.12.4
Review comment:
Might as well take it to 3.17 or whatever the latest is today.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett opened a new pull request #607: Wip/no ace stats
pivotal-jbarrett opened a new pull request #607: URL: https://github.com/apache/geode-native/pull/607 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode-native] pivotal-jbarrett opened a new pull request #606: GEODE-8181: Fixes default value for statistic-sampling-enabled.
pivotal-jbarrett opened a new pull request #606: URL: https://github.com/apache/geode-native/pull/606 GEODE-8181: Fixes default value for statistic-sampling-enabled. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal merged pull request #5173: GEODE-8083: Add API Checking to Geode. (#5066) (#5103)
onichols-pivotal merged pull request #5173: URL: https://github.com/apache/geode/pull/5173 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] bschuchardt merged pull request #5172: GEODE-8144 another attempt to fix a failing test
bschuchardt merged pull request #5172: URL: https://github.com/apache/geode/pull/5172 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #4871: GEODE-7864: Override the default implementation of write(byte[],int,int)
lgtm-com[bot] commented on pull request #4871: URL: https://github.com/apache/geode/pull/4871#issuecomment-634981031 This pull request **fixes 5 alerts** when merging 54c972f1bcd078f62f2cfe78fea4add1ce9016a2 into a185267dca016954f857f6848863bc4f74e6de81 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-0ece1e209e4d49fdb7c53bba44e33a261092048d) **fixed alerts:** * 5 for Inefficient output stream This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #4866: GEODE-7864: Overriding hashCode when equals is overridden.
lgtm-com[bot] commented on pull request #4866: URL: https://github.com/apache/geode/pull/4866#issuecomment-634978509 This pull request **fixes 7 alerts** when merging 598d886b9c3beae173cc223167f2a0e9cdb70ca4 into a185267dca016954f857f6848863bc4f74e6de81 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-acd601de49877780575f3994a545681dbd1254ae) **fixed alerts:** * 7 for Inconsistent equals and hashCode This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] rhoughton-pivot opened a new pull request #5173: GEODE-8083: Add API Checking to Geode. (#5066) (#5103)
rhoughton-pivot opened a new pull request #5173: URL: https://github.com/apache/geode/pull/5173 * Add API check gradle task. * Add CI job that calls API check gradle task. * Fix spotless rules so it doesn't mistakenly mess up our gradle. * Allow changes if a class or superclass is marked @Experimental Co-authored-by: Sean Goller Co-authored-by: Robert Houghton (cherry picked from commit 9f0d4c9f91fb89821227e7d94d02d75d0ccbf0a9) Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] yozaner1324 closed pull request #5171: GEODE-8194 - Refactor Result type into geode-common for usage in othe…
yozaner1324 closed pull request #5171: URL: https://github.com/apache/geode/pull/5171 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] bschuchardt opened a new pull request #5172: GEODE-8144 another attempt to fix a failing test
bschuchardt opened a new pull request #5172: URL: https://github.com/apache/geode/pull/5172 The JMX tests in this test class use "localhost" to connect to a Manager. This is being transformed somehow into an IP numeric address and is failing endpoint verification. The test passes on my Windows machine and also passes on Mac and Ubuntu when I run it there. I'm adding the "localhost" IP address to the certificate to get past this. Another fix would be to change the Rule that's using "localhost" to use the real host name but that would affect a lot of other tests. Before I started messing with this test it was adding InetAddress.getLocalHost() as an IP address to the certificate so I don't think this is a big change to the original test. The test now uses LocalHostUtils.getLocalHost() to get an IP to add to the certificate, which is the correct "localhost" to use with Geode. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] gesterzhou merged pull request #5147: GEODE-8171: javadoc for putAll need to have accurate exception
gesterzhou merged pull request #5147: URL: https://github.com/apache/geode/pull/5147 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] gesterzhou commented on a change in pull request #5147: GEODE-8171: javadoc for putAll need to have accurate exception
gesterzhou commented on a change in pull request #5147: URL: https://github.com/apache/geode/pull/5147#discussion_r431450182 ## File path: geode-core/src/main/java/org/apache/geode/cache/Region.java ## @@ -1362,49 +1362,77 @@ Object selectValue(String queryPredicate) throws FunctionDomainException, TypeMi * Scope.LOCAL. * * If an exception is thrown due to this call, it can imply that there may have been a partial - * update performed on the region. Use putAll from within a transaction to to obtain an atomic - * update. + * update performed on the region. * * * @see java.util.Map#putAll(Map map) - * @param map the key/value pairs to put in this region. + * @param map the key/value pairs to put in this region + * @throws ServerOperationException if called from a client, and the server throws an exception + * such as CacheWriterException, PartitionedRegionStorageException or LowMemoryException. + * These server exceptions become the ServerOperationException cause + * @throws ServerConnectivityException if called from a client, and the server throws + * CancelException. CancelException will be the ServerConnectivityException cause * @throws NullPointerException if key is null or if value is null (use invalidate instead), or if * the key or value do not meet serializability requirements Review comment: fixed. ## File path: geode-core/src/main/java/org/apache/geode/cache/Region.java ## @@ -1362,49 +1362,77 @@ Object selectValue(String queryPredicate) throws FunctionDomainException, TypeMi * Scope.LOCAL. * * If an exception is thrown due to this call, it can imply that there may have been a partial - * update performed on the region. Use putAll from within a transaction to to obtain an atomic - * update. + * update performed on the region. * * * @see java.util.Map#putAll(Map map) - * @param map the key/value pairs to put in this region. + * @param map the key/value pairs to put in this region + * @throws ServerOperationException if called from a client, and the server throws an exception + * such as CacheWriterException, PartitionedRegionStorageException or LowMemoryException. + * These server exceptions become the ServerOperationException cause + * @throws ServerConnectivityException if called from a client, and the server throws + * CancelException. CancelException will be the ServerConnectivityException cause * @throws NullPointerException if key is null or if value is null (use invalidate instead), or if * the key or value do not meet serializability requirements * @throws ClassCastException if key does not satisfy the keyConstraint - * @throws org.apache.geode.distributed.LeaseExpiredException if lease expired on distributed lock - * for Scope.GLOBAL - * @throws TimeoutException if timed out getting distributed lock for Scope.GLOBAL + * @throws org.apache.geode.distributed.LeaseExpiredException if lease expired on distributed + * lock for regions with Scope.GLOBAL Review comment: fixed This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] yozaner1324 opened a new pull request #5171: GEODE-8194 - Refactor Result type into geode-common for usage in othe…
yozaner1324 opened a new pull request #5171: URL: https://github.com/apache/geode/pull/5171 …r modules. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] karensmolermiller commented on a change in pull request #5147: GEODE-8171: javadoc for putAll need to have accurate exception
karensmolermiller commented on a change in pull request #5147: URL: https://github.com/apache/geode/pull/5147#discussion_r431440863 ## File path: geode-core/src/main/java/org/apache/geode/cache/Region.java ## @@ -1362,49 +1362,77 @@ Object selectValue(String queryPredicate) throws FunctionDomainException, TypeMi * Scope.LOCAL. * * If an exception is thrown due to this call, it can imply that there may have been a partial - * update performed on the region. Use putAll from within a transaction to to obtain an atomic - * update. + * update performed on the region. * * * @see java.util.Map#putAll(Map map) - * @param map the key/value pairs to put in this region. + * @param map the key/value pairs to put in this region + * @throws ServerOperationException if called from a client, and the server throws an exception + * such as CacheWriterException, PartitionedRegionStorageException or LowMemoryException. + * These server exceptions become the ServerOperationException cause + * @throws ServerConnectivityException if called from a client, and the server throws + * CancelException. CancelException will be the ServerConnectivityException cause * @throws NullPointerException if key is null or if value is null (use invalidate instead), or if * the key or value do not meet serializability requirements Review comment: Please slightly reword NullPointerException (in BOTH putAll signatures): if the key is null, if the value is null (use invalidate instead), or if the key or value do not meet serialization requirements ## File path: geode-core/src/main/java/org/apache/geode/cache/Region.java ## @@ -1362,49 +1362,77 @@ Object selectValue(String queryPredicate) throws FunctionDomainException, TypeMi * Scope.LOCAL. * * If an exception is thrown due to this call, it can imply that there may have been a partial - * update performed on the region. Use putAll from within a transaction to to obtain an atomic - * update. + * update performed on the region. * * * @see java.util.Map#putAll(Map map) - * @param map the key/value pairs to put in this region. + * @param map the key/value pairs to put in this region + * @throws ServerOperationException if called from a client, and the server throws an exception + * such as CacheWriterException, PartitionedRegionStorageException or LowMemoryException. + * These server exceptions become the ServerOperationException cause + * @throws ServerConnectivityException if called from a client, and the server throws + * CancelException. CancelException will be the ServerConnectivityException cause * @throws NullPointerException if key is null or if value is null (use invalidate instead), or if * the key or value do not meet serializability requirements * @throws ClassCastException if key does not satisfy the keyConstraint - * @throws org.apache.geode.distributed.LeaseExpiredException if lease expired on distributed lock - * for Scope.GLOBAL - * @throws TimeoutException if timed out getting distributed lock for Scope.GLOBAL + * @throws org.apache.geode.distributed.LeaseExpiredException if lease expired on distributed + * lock for regions with Scope.GLOBAL Review comment: Please slightly reword LeaseExpiredException (in BOTH putall signatures): if the lease expired on a distributed lock, for regions with Scope.GLOBAL This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] kohlmu-pivotal merged pull request #5170: 1.13 GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal merged pull request #5170: URL: https://github.com/apache/geode/pull/5170 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] rhoughton-pivot commented on a change in pull request #5115: Feature/GEODE-7414_2_redo
rhoughton-pivot commented on a change in pull request #5115:
URL: https://github.com/apache/geode/pull/5115#discussion_r431420569
##
File path:
geode-core/src/main/java/org/apache/geode/net/SSLParameterExtension.java
##
@@ -29,9 +29,9 @@
/**
* Initialize the SSLParameterExtension.
*
- * @param config the DistributionConfig
+ * @param properties the Properties
*/
- default void init(DistributionConfig config) {}
+ default void init(Properties properties) {}
Review comment:
@pivotal-jbarrett , you are proposing that `Properties` be replaced with
a concrete class, wrapping a map? You did not request a change to the PR, so
I'm otherwise ready to merge this as-is, and get it onto the 1.13 branch before
it goes out, too.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] kohlmu-pivotal opened a new pull request #5170: 1.13 GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal opened a new pull request #5170: URL: https://github.com/apache/geode/pull/5170 (cherry picked from commit 93ef47c8c4d8712f60c2961fce1b28457fe447cc) Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] kohlmu-pivotal closed pull request #5168: 1.13 - GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal closed pull request #5168: URL: https://github.com/apache/geode/pull/5168 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] bschuchardt merged pull request #5164: Feature/geode 8144
bschuchardt merged pull request #5164: URL: https://github.com/apache/geode/pull/5164 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] agingade closed pull request #4987: GEODE-7678: Support for cache-listener and client-notification for Partitioned Region Clear operation.
agingade closed pull request #4987: URL: https://github.com/apache/geode/pull/4987 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] bschuchardt commented on a change in pull request #5164: Feature/geode 8144
bschuchardt commented on a change in pull request #5164:
URL: https://github.com/apache/geode/pull/5164#discussion_r431292388
##
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/GfshHostNameVerificationDistributedTest.java
##
@@ -60,14 +62,19 @@ public void setupCluster() throws Exception {
.isCA()
.generate();
-CertificateMaterial locatorCertificate = new CertificateBuilder()
+String hostname = LocalHostUtil.getCanonicalLocalHostName();
+final CertificateBuilder builder = new CertificateBuilder()
.commonName("locator")
.issuedBy(ca)
.sanDnsName(InetAddress.getLoopbackAddress().getHostName())
-.sanDnsName(InetAddress.getLocalHost().getHostName())
-.sanIpAddress(InetAddress.getLocalHost())
-.sanIpAddress(InetAddress.getByName("0.0.0.0")) // to pass on windows
-.generate();
+.sanDnsName(hostname)
Review comment:
loopback is supposed to be there. The hostnamechecker in the jdk will
ignore the sanDnsName(hostname) entry if it's an IP literal and instead look
for a sanIpAddress entry , so it doesn't hurt to have the sanDnsName there.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] bschuchardt commented on a change in pull request #5164: Feature/geode 8144
bschuchardt commented on a change in pull request #5164:
URL: https://github.com/apache/geode/pull/5164#discussion_r431292388
##
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/GfshHostNameVerificationDistributedTest.java
##
@@ -60,14 +62,19 @@ public void setupCluster() throws Exception {
.isCA()
.generate();
-CertificateMaterial locatorCertificate = new CertificateBuilder()
+String hostname = LocalHostUtil.getCanonicalLocalHostName();
+final CertificateBuilder builder = new CertificateBuilder()
.commonName("locator")
.issuedBy(ca)
.sanDnsName(InetAddress.getLoopbackAddress().getHostName())
-.sanDnsName(InetAddress.getLocalHost().getHostName())
-.sanIpAddress(InetAddress.getLocalHost())
-.sanIpAddress(InetAddress.getByName("0.0.0.0")) // to pass on windows
-.generate();
+.sanDnsName(hostname)
Review comment:
loopback is supposed to be there. The hostnamechecker in the jdk will
ignore the sanDnsName(hostname) entry if it's an IP literal, so it doesn't hurt
to have it there.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] gesterzhou commented on a change in pull request #5164: Feature/geode 8144
gesterzhou commented on a change in pull request #5164:
URL: https://github.com/apache/geode/pull/5164#discussion_r431288262
##
File path:
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/GfshHostNameVerificationDistributedTest.java
##
@@ -60,14 +62,19 @@ public void setupCluster() throws Exception {
.isCA()
.generate();
-CertificateMaterial locatorCertificate = new CertificateBuilder()
+String hostname = LocalHostUtil.getCanonicalLocalHostName();
+final CertificateBuilder builder = new CertificateBuilder()
.commonName("locator")
.issuedBy(ca)
.sanDnsName(InetAddress.getLoopbackAddress().getHostName())
-.sanDnsName(InetAddress.getLocalHost().getHostName())
-.sanIpAddress(InetAddress.getLocalHost())
-.sanIpAddress(InetAddress.getByName("0.0.0.0")) // to pass on windows
-.generate();
+.sanDnsName(hostname)
Review comment:
these 2 lines should be put in "else" or some better place?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] kohlmu-pivotal opened a new pull request #5169: GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal opened a new pull request #5169: URL: https://github.com/apache/geode/pull/5169 …ction. (cherry picked from commit 93ef47c8c4d8712f60c2961fce1b28457fe447cc) Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] bschuchardt closed pull request #5163: Revert "GEODE-8144: endpoint identification in servers is not working…
bschuchardt closed pull request #5163: URL: https://github.com/apache/geode/pull/5163 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] kohlmu-pivotal opened a new pull request #5168: GEODE-8174: Fix ConcurrentModificationException when using JTA transaction.
kohlmu-pivotal opened a new pull request #5168: URL: https://github.com/apache/geode/pull/5168 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### 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 verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] dschneider-pivotal merged pull request #5165: GEODE-8182: change String commands to use one region
dschneider-pivotal merged pull request #5165: URL: https://github.com/apache/geode/pull/5165 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5165: GEODE-8182: change String commands to use one region
dschneider-pivotal commented on a change in pull request #5165:
URL: https://github.com/apache/geode/pull/5165#discussion_r431238457
##
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetNXExecutor.java
##
@@ -36,27 +36,23 @@
public void executeCommand(Command command, ExecutionHandlerContext context)
{
List commandElems = command.getProcessedCommand();
-Region region =
-context.getRegionProvider().getStringsRegion();
-
if (commandElems.size() < 3) {
command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(),
ArityDef.SETNX));
return;
}
ByteArrayWrapper key = command.getKey();
-checkAndSetDataType(key, context);
-byte[] value = commandElems.get(VALUE_INDEX);
+ByteArrayWrapper value = new
ByteArrayWrapper(commandElems.get(VALUE_INDEX));
-Object oldValue =
-region.putIfAbsent(key, (RedisData) new RedisString(new
ByteArrayWrapper(value)));
+RedisStringCommands stringCommands = getRedisStringCommands(context);
+SetOptions setOptions = new SetOptions(NX, 0L, null, false);
Review comment:
Thanks for pointing this out. At this time, all that is used on the
SetOptions when we call RedisStringCommands.set is the first parameter (NX in
this case) so I just went with defaults on all the others. Some of these apply
to expiration so they will be reviewed when the string expiration work is done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jdeppe-pivotal commented on a change in pull request #5165: GEODE-8182: change String commands to use one region
jdeppe-pivotal commented on a change in pull request #5165:
URL: https://github.com/apache/geode/pull/5165#discussion_r431140254
##
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetNXExecutor.java
##
@@ -36,27 +36,23 @@
public void executeCommand(Command command, ExecutionHandlerContext context)
{
List commandElems = command.getProcessedCommand();
-Region region =
-context.getRegionProvider().getStringsRegion();
-
if (commandElems.size() < 3) {
command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(),
ArityDef.SETNX));
return;
}
ByteArrayWrapper key = command.getKey();
-checkAndSetDataType(key, context);
-byte[] value = commandElems.get(VALUE_INDEX);
+ByteArrayWrapper value = new
ByteArrayWrapper(commandElems.get(VALUE_INDEX));
-Object oldValue =
-region.putIfAbsent(key, (RedisData) new RedisString(new
ByteArrayWrapper(value)));
+RedisStringCommands stringCommands = getRedisStringCommands(context);
+SetOptions setOptions = new SetOptions(NX, 0L, null, false);
Review comment:
This should be `SetOptions(NX, 0L, ExpireUnit.NONE, false);`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jujoramos opened a new pull request #5167: [DO NOT REVIEW]: Debug 4 Regression Introduced Through GEODE-8119
jujoramos opened a new pull request #5167: URL: https://github.com/apache/geode/pull/5167 Hello @mkevo, This is just a draft `PR` to reproduce the regression introduced through [GEODE-8119](https://issues.apache.org/jira/browse/GEODE-8119). The first commit contains only your original changes, the second commit is the distributed test showing the regression failure. We can use this `PR` or the ticket for communication purposes, whatever you prefer. Thanks in advance!. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] alb3rtobr opened a new pull request #5166: GEODE-8193: Broken link in statistics list
alb3rtobr opened a new pull request #5166: URL: https://github.com/apache/geode/pull/5166 The link of region entry heap-based eviction is wrong and shows the html anchor id in the statistics lists: * Region Entry Eviction - Heap-based eviction (HeapLRUStatistics)#section_3B74F6FA08A374FBD92AA23047929B4F) https://geode.apache.org/docs/guide/112/reference/statistics_list.html This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #5165: GEODE-8182: change String commands to use one region
lgtm-com[bot] commented on pull request #5165: URL: https://github.com/apache/geode/pull/5165#issuecomment-634494424 This pull request **fixes 1 alert** when merging 3b4c1cd9aef9ff1a13b5172c58ae56c93934af5e into bef07b34131abddb8c0f04e0ab6a48f1daac991d - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-7d19dc7dd451817295f349613650ae387317eb4c) **fixed alerts:** * 1 for Useless null check This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mkevo commented on a change in pull request #5159: GEODE-7956: correct legal region names
mkevo commented on a change in pull request #5159: URL: https://github.com/apache/geode/pull/5159#discussion_r430875852 ## File path: geode-docs/basic_config/data_regions/region_naming.html.md.erb ## @@ -24,7 +24,7 @@ follow these region naming guidelines. - Permitted characters within region names are alphanumeric characters (`ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789`), -the underscore character (`_`), and the hyphen character (`-`). +the dot character (`.`), the underscore character (`_`), the square brackets characters (`[, ]`) and the hyphen character (`-`). Review comment: Done! Thanks @davebarnes97! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
