[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130782011
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
 ---
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.admin.remote.DistributionLocatorId;
+import org.apache.geode.protocol.operations.OperationHandler;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import org.apache.geode.protocol.protobuf.Success;
+import org.apache.geode.serialization.SerializationService;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+public class GetAvailableServersOperationHandler implements
--- End diff --

@Experimental or a doc comment explaining why this won't be around forever 
would be awesome.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130781962
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
 ---
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.admin.remote.DistributionLocatorId;
+import org.apache.geode.protocol.operations.OperationHandler;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import org.apache.geode.protocol.protobuf.Success;
+import org.apache.geode.serialization.SerializationService;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+public class GetAvailableServersOperationHandler implements
+OperationHandler {
+
+  @Override
+  public Result process(
+  SerializationService serializationService, 
ServerAPI.GetAvailableServersRequest request,
+  Cache cache) {
+HashSet locators = 
getLocatorIdsFromDistributedProperties(cache);
+
+TcpClient tcpClient = getTcpClient();
+Optional result = 
locators.stream()
+.map((locatorId -> getGetAvailableServersFromLocator(tcpClient, 
locatorId)))
+.filter((obj) -> obj != null).findFirst();
+
+if (result.isPresent()) {
+  return result.get();
+} else {
+  return Failure
+  .of(BasicTypes.ErrorResponse.newBuilder().setMessage("Unable to 
find a locator").build());
+}
+  }
+
+  private HashSet 
getLocatorIdsFromDistributedProperties(Cache cache) {
--- End diff --

Will this work if locators are configured via cluster config? I'm guessing 
it won't work if the original locators are no longer in the cluster. Since this 
is a prototype and will be redone better, I think it's fine for now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #676: GEODE-3321: Adding ErrorCode values to protobuf protocol

[galen-pivotal]

Github user galen-pivotal commented on the issue:

https://github.com/apache/geode/pull/676
  
It looks like this is failing in Travis due to `:rat`, but you've got a 
license in your only new file. I'm a bit mystified. Closing the pull request 
and reopening it can be a good way to trigger a new Travis build.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #676: GEODE-3321: Adding ErrorCode values to protobuf pro...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/676#discussion_r130781436
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandlerJUnitTest.java
 ---
@@ -95,8 +97,8 @@ public void 
processReturnsUnsucessfulResponseForInvalidRegion()
 operationHandler.process(serializationServiceStub, removeRequest, 
cacheStub);
 
 assertTrue(result instanceof Failure);
-org.junit.Assert.assertThat(result.getErrorMessage().getMessage(),
-CoreMatchers.containsString("Region"));
+assertEquals(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
--- End diff --

YES. I like these kinds of tests much better.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #676: GEODE-3321: Adding ErrorCode values to protobuf pro...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/676#discussion_r130781170
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtocolErrorCode.java
 ---
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License), Version 2.0
+ * (the "License")), you may not use this file except in compliance with 
the License. You may obtain
+ * a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE2.0
+ *
+ * Unless required by applicable law or agreed to in writing), software 
distributed under the
+ * License is distributed on an "AS IS" BASIS), WITHOUT WARRANTIES OR 
CONDITIONS OF ANY KIND),
+ * either express or implied. See the License for the specific language 
governing permissions and
+ * limitations under the License.
+ */
+package org.apache.geode.protocol.protobuf;
+
+public enum ProtocolErrorCode {
+  GENERIC_FAILURE(1000),
--- End diff --

Why are we starting at 1000?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #661: GEODE-3319 - refactor to use protobuf encoding for ...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/661#discussion_r130494367
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufPrimitiveTypes.java
 ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.utilities;
+
+import 
org.apache.geode.protocol.protobuf.utilities.exception.UnknownProtobufPrimitiveType;
+
+public enum ProtobufPrimitiveTypes {
--- End diff --

I think it's better to use the singular `ProtobufPrimitiveType` here, since 
each represents a protobuf primitive type.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #661: GEODE-3319 - refactor to use protobuf encoding for ...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/661#discussion_r130495450
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufPrimitiveTypes.java
 ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.utilities;
+
+import 
org.apache.geode.protocol.protobuf.utilities.exception.UnknownProtobufPrimitiveType;
+
+public enum ProtobufPrimitiveTypes {
+
+  STRING(String.class),
+  INT(Integer.class),
+  LONG(Long.class),
+  SHORT(Short.class),
+  BYTE(Byte.class),
+  BOOLEAN(Boolean.class),
+  DOUBLE(Double.class),
+  FLOAT(Float.class),
+  BINARY(byte[].class);
+
+  private Class clazzType;
+
+  ProtobufPrimitiveTypes(Class clazz) {
+this.clazzType = clazz;
+  }
+
+  public static ProtobufPrimitiveTypes valueOf(Class unencodedValueClass)
+  throws UnknownProtobufPrimitiveType {
+for (ProtobufPrimitiveTypes protobufPrimitiveTypes : values()) {
--- End diff --

I think the variable here should be singular as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #661: GEODE-3319 - refactor to use protobuf encoding for ...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/661#discussion_r130494791
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufPrimitiveTypes.java
 ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.utilities;
+
+import 
org.apache.geode.protocol.protobuf.utilities.exception.UnknownProtobufPrimitiveType;
+
+public enum ProtobufPrimitiveTypes {
+
+  STRING(String.class),
+  INT(Integer.class),
+  LONG(Long.class),
+  SHORT(Short.class),
+  BYTE(Byte.class),
+  BOOLEAN(Boolean.class),
+  DOUBLE(Double.class),
+  FLOAT(Float.class),
+  BINARY(byte[].class);
+
+  private Class clazzType;
--- End diff --

I think `clazz` would be clearer than `clazzType`. What is a type of class?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #661: GEODE-3319 - refactor to use protobuf encoding for ...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/661#discussion_r130779627
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandler.java
 ---
@@ -50,10 +50,7 @@
 return Success.of(RegionAPI.PutResponse.newBuilder().build());
   } catch (ClassCastException ex) {
 return Failure.of(BasicTypes.ErrorResponse.newBuilder()
-.setMessage("invalid key or value type for region " + 
regionName + ",passed key: "
--- End diff --

I'm curious why you chose to shrink the message here; what am I missing?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #659: GEODE-3308: Lucene rolling upgrade and backwards co...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/659#discussion_r130765375
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneSearchWithRollingUpgradeDUnit.java
 ---
@@ -0,0 +1,1044 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.lucene;
+
+import static org.apache.geode.test.dunit.Assert.fail;
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.GemFireCache;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.cache30.CacheSerializableRunnable;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.NetworkUtils;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.standalone.DUnitLauncher;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)

+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class LuceneSearchWithRollingUpgradeDUnit extends 
JUnit4DistributedTestCase {
+
+
+  @Parameterized.Parameters
+  public static Collection data() {
+List result = 
VersionManager.getInstance().getVersionsWithoutCurrent();
+// Lucene Compatibility checks start with Apache Geode v1.2.0
+// Removing the versions older than v1.2.0
+result.removeIf(s -> Integer.parseInt(s) < 120);
+if (result.size() < 1) {
+  throw new RuntimeException("No older versions of Geode were found to 
test against");
+} else {
+  System.out.println("running against these versions: " + result);
+}
+return result;
+  }
+
+  private File[] testingDirs = new File[3];
+
+  private static String INDEX_NAME = "index";
+
+  private static String diskDir = "LuceneSearchWithRollingUpgradeDUnit";
+
+  // Each vm will have a cache object
+  private static Object cache;
+
+  // the old version of Geode we're testing against
+  private String oldVersion;
+
+  private void deleteVMFiles() throws Exception {
+System.out.println("deleting files in vm" + VM.getCurrentVMNum());
+File pwd = new 

[GitHub] geode pull request #659: GEODE-3308: Lucene rolling upgrade and backwards co...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/659#discussion_r130765402
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneSearchWithRollingUpgradeDUnit.java
 ---
@@ -0,0 +1,1044 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.lucene;
+
+import static org.apache.geode.test.dunit.Assert.fail;
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.GemFireCache;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.cache30.CacheSerializableRunnable;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.NetworkUtils;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.standalone.DUnitLauncher;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)

+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class LuceneSearchWithRollingUpgradeDUnit extends 
JUnit4DistributedTestCase {
+
+
+  @Parameterized.Parameters
+  public static Collection data() {
+List result = 
VersionManager.getInstance().getVersionsWithoutCurrent();
+// Lucene Compatibility checks start with Apache Geode v1.2.0
+// Removing the versions older than v1.2.0
+result.removeIf(s -> Integer.parseInt(s) < 120);
+if (result.size() < 1) {
+  throw new RuntimeException("No older versions of Geode were found to 
test against");
+} else {
+  System.out.println("running against these versions: " + result);
+}
+return result;
+  }
+
+  private File[] testingDirs = new File[3];
+
+  private static String INDEX_NAME = "index";
+
+  private static String diskDir = "LuceneSearchWithRollingUpgradeDUnit";
+
+  // Each vm will have a cache object
+  private static Object cache;
+
+  // the old version of Geode we're testing against
+  private String oldVersion;
+
+  private void deleteVMFiles() throws Exception {
+System.out.println("deleting files in vm" + VM.getCurrentVMNum());
+File pwd = new 

[GitHub] geode pull request #658: GEODE-3315: Replaced PreferBytes... with VMCachedDe...

[upthewaterspout]

Github user upthewaterspout closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #658: GEODE-3315: Replaced PreferBytes... with VMCachedDeseriali...

[upthewaterspout]

Github user upthewaterspout commented on the issue:

https://github.com/apache/geode/pull/658
  
Merged to develop in 56ea940d3c826e98b16d6b508fc834f7bd50220c.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #676: GEODE-3321: Adding ErrorCode values to protobuf pro...

[WireBaron]

GitHub user WireBaron opened a pull request:

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

GEODE-3321: Adding ErrorCode values to protobuf protocol

@pivotal-amurmann @kohlmu-pivotal @hiteshk25 @galen-pivotal @bschuchardt 

Signed-off-by: Bruce Schuchardt 

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:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [x] 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, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/WireBaron/geode feature/GEODE-3321

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/676.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #676


commit 5a73db96aadd8542e90dccf9f164aa309e1ca3d2
Author: Brian Rowe 
Date:   2017-08-01T23:56:24Z

GEODE-3321: Adding ErrorCode values to protobuf protocol

Signed-off-by: Bruce Schuchardt 
Signed-off-by: Brian Rowe 




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #663: GEODE-3314: Fix DLockService token leak.

[galen-pivotal]

Github user galen-pivotal commented on the issue:

https://github.com/apache/geode/pull/663
  
@pivotal-amurmann I'd like to write a unit test. @hiteshk25 and I took a 
stab at it this afternoon but after an hour or two, got lost in mocks and the 
tangle of 
DistributedSystem/DistributedMember/DistributionManager/Elder/RequestProcessor 
. I would love to do some refactoring to make this logic more testable, but I'd 
also like to see this fix get into master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 61281: GEODE-3379 Geode transaction may commit on a secondary bucket after bucket rebalance

[anilkumar gingade]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61281/#review181924
---




geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 1159 (patched)


longer?



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 1164 (patched)


Since only one test using this; can we move this to the test itself.



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 1203 (patched)


How about using CatchExceptions ( CatchException.catchException) that gives 
a nice way to manage expected exceptions.


- anilkumar gingade


On Aug. 1, 2017, 9:41 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61281/
> ---
> 
> (Updated Aug. 1, 2017, 9:41 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Nick Reich.
> 
> 
> Bugs: GEODE-3379
> https://issues.apache.org/jira/browse/GEODE-3379
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Current geode commit method only hold primary bucket lock when the txRegion 
> is a primary bucket. However, in between the tx operation and commit, a 
> rebalance could cause primary bucket to move. In this case, commit should 
> detect the primary bucket has been moved and throw 
> TransactionDataRebalancedException.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java 
> 2c8c28b 
>   geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java 
> 77bf740 
>   
> geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
>  9ab5145 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
>  6578baa 
> 
> 
> Diff: https://reviews.apache.org/r/61281/diff/1/
> 
> 
> Testing
> ---
> 
> Ongoing precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #634 was SUCCESSFUL (with 2024 tests). Change made by John Blum.

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #634 was successful.
---
Scheduled with changes by John Blum.
2026 tests in total.

https://build.spring.io/browse/SGF-NAG-634/




--
Code Changes
--
John Blum (a23afdb7e3bc0ed73045bb261836e39bf89ac2ec):

>DATAGEODE-33 - Add EnableCachingDefinedRegions annotation to configure Geode 
>Regions based on Spring Caching annotations.



--
This message is automatically generated by Atlassian Bamboo

Re: Review Request 61166: GEODE-3313: Test utility supports building jar files with multiple classes

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61166/#review181921
---


Ship it!




Ship It!

- Jinmei Liao


On Aug. 1, 2017, 6:31 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61166/
> ---
> 
> (Updated Aug. 1, 2017, 6:31 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3313: Test utility supports building jar files with multiple classes
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractor.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/CompiledSourceCode.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilder.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilderTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompiler.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompilerTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCode.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCodeTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractExtendsFunctionAdapter.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractFunction.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ConcreteExtendsAbstractExtendsFunctionAdapter.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsAbstractFunction.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsFunctionAdapter.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ImplementsFunction.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61166/diff/6/
> 
> 
> Testing
> ---
> 
> Precheckin running
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

[GitHub] geode pull request #655: GEODE-3030: Set possibleDuplicate=true for all buck...

[upthewaterspout]

Github user upthewaterspout closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Review Request 61281: GEODE-3379 Geode transaction may commit on a secondary bucket after bucket rebalance

[Eric Shu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61281/
---

Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
and Nick Reich.


Bugs: GEODE-3379
https://issues.apache.org/jira/browse/GEODE-3379


Repository: geode


Description
---

Current geode commit method only hold primary bucket lock when the txRegion is 
a primary bucket. However, in between the tx operation and commit, a rebalance 
could cause primary bucket to move. In this case, commit should detect the 
primary bucket has been moved and throw TransactionDataRebalancedException.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java 2c8c28b 
  geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java 
77bf740 
  
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
 9ab5145 
  
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
 6578baa 


Diff: https://reviews.apache.org/r/61281/diff/1/


Testing
---

Ongoing precheckin.


Thanks,

Eric Shu

[GitHub] geode issue #570: GEODE-3055: waitUntilFlush should use data region's bucket...

[gesterzhou]

Github user gesterzhou commented on the issue:

https://github.com/apache/geode/pull/570
  
The forceRemovePrimary was useless and it can be removed because it always 
use "false".

But when I added the logic to remove the leader region bucket (when the 
shadow bucket failed to initialize), I have to call the removeBucket(xxx, 
forceRemovePrimary=true) by myself.

When removing leader bucket in the error handling, I have to skip a few 
"return false" exit points, because at this time the leader bucket is not 
logically ready and not qualified to be removed unless I force to remove it.

So I make use of the forceRemovePrimary parameter. Maybe I should change it 
to better name, such as forceToRemove, though. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #570: GEODE-3055: waitUntilFlush should use data region's...

[gesterzhou]

Github user gesterzhou commented on a diff in the pull request:

https://github.com/apache/geode/pull/570#discussion_r130729615
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
 ---
@@ -1472,6 +1472,19 @@ public boolean removeBucket(int bucketId, boolean 
forceRemovePrimary) {
   }
 
   BucketAdvisor bucketAdvisor = bucketRegion.getBucketAdvisor();
+  InternalDistributedMember primary = bucketAdvisor.getPrimary();
+  InternalDistributedMember myId =
+  
this.partitionedRegion.getDistributionManager().getDistributionManagerId();
+  if (primary == null || myId.equals(primary)) {
--- End diff --

The forceRemovePrimary WAS useless and it can be removed because it always 
use "false".

But when I added the logic to remove the leader region bucket (when the 
shadow bucket failed to initialize), then I have to call the removeBucket(xxx, 
true) by myself. 

The shadow bucket was removed first by the exception handling. But I added 
the logic to remove leader bucket, I have to skip a few "return false" exit 
points, because at this time the leader bucket is not logically ready and not 
qualified to be removed unless I force to remove it. 

So I make use of the forceRemovePrimary parameter. Maybe I should change it 
to better name, such as forceToRemove.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #674: GEODE-3380: There're 2 problems here

[gesterzhou]

Github user gesterzhou commented on a diff in the pull request:

https://github.com/apache/geode/pull/674#discussion_r130727639
  
--- Diff: 
geode-cq/src/test/java/org/apache/geode/internal/cache/PutAllCSDUnitTest.java 
---
@@ -500,6 +502,81 @@ public void run2() throws CacheException {
   }
 
   /**
+   * Create PR without redundancy on 2 servers with lucene index. Feed 
some key s. From a client, do
+   * removeAll on keys in server1. During the removeAll, restart server1 
and trigger the removeAll
+   * to retry. The retried removeAll should return the version tag of 
tombstones. Do removeAll again
+   * on the same key, it should get the version tag again.
+   */
+  @Test
+  public void test51871() throws CacheException, InterruptedException {
--- End diff --

OK, I can do that


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Build failed in Jenkins: Geode-nightly-flaky #82

[Apache Jenkins Server]

See 


Changes:

[jiliao] GEODE-2971: Introduce ExitCode to resolve inconsistency of shell exit

[jstewart] GEODE-3317: Fix UniversalMembershipListenerAdapterDUnitTest

[kmiller] GEODE-3324 Document finer-grained security permissions

[lhughesgodfrey] GEODE-2226: SessionReplicationIntegrationTests do not run on 
Windows

--
[...truncated 107.03 KB...]
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spring-web/2.6.1/springfox-spring-web-2.6.1.pom
Download 
https://repo1.maven.org/maven2/com/fasterxml/classmate/1.3.1/classmate-1.3.1.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-core/1.2.0.RELEASE/spring-plugin-core-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin/1.2.0.RELEASE/spring-plugin-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-metadata/1.2.0.RELEASE/spring-plugin-metadata-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct/1.0.0.Final/mapstruct-1.0.0.Final.pom
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct-parent/1.0.0.Final/mapstruct-parent-1.0.0.Final.pom
Download 
https://repo1.maven.org/maven2/org/springframework/spring-expression/4.3.5.RELEASE/spring-expression-4.3.5.RELEASE.pom
Download 
https://repo1.maven.org/maven2/com/thoughtworks/paranamer/paranamer/2.8/paranamer-2.8.pom
Download 
https://repo1.maven.org/maven2/com/thoughtworks/paranamer/paranamer-parent/2.8/paranamer-parent-2.8.pom
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-core/2.6.1/springfox-core-2.6.1.pom
Download 
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-scala_2.10/2.8.6/jackson-module-scala_2.10-2.8.6.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger2/2.6.1/springfox-swagger2-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger-ui/2.6.1/springfox-swagger-ui-2.6.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/hateoas/spring-hateoas/0.23.0.RELEASE/spring-hateoas-0.23.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/scala-lang/scala-library/2.10.6/scala-library-2.10.6.jar
Download 
https://repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.10.6/scala-reflect-2.10.6.jar
Download 
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-paranamer/2.8.6/jackson-module-paranamer-2.8.6.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-annotations/1.5.10/swagger-annotations-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-models/1.5.10/swagger-models-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spi/2.6.1/springfox-spi-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-schema/2.6.1/springfox-schema-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger-common/2.6.1/springfox-swagger-common-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spring-web/2.6.1/springfox-spring-web-2.6.1.jar
Download 
https://repo1.maven.org/maven2/com/fasterxml/classmate/1.3.1/classmate-1.3.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-core/1.2.0.RELEASE/spring-plugin-core-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-metadata/1.2.0.RELEASE/spring-plugin-metadata-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct/1.0.0.Final/mapstruct-1.0.0.Final.jar
Download 
https://repo1.maven.org/maven2/com/thoughtworks/paranamer/paranamer/2.8/paranamer-2.8.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-core/2.6.1/springfox-core-2.6.1.jar
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:geode-web-api:processResources
:geode-web-api:classes
:geode-assembly:docs
:geode-assembly:gfshDepsJar
:geode-common:javadocJar
:geode-common:sourcesJar
:geode-common:signArchives SKIPPED
:geode-core:javadocJar
:geode-core:raJar
:geode-core:jcaJar
:geode-core:sourcesJar
:geode-core:signArchives SKIPPED
:geode-core:webJar
:geode-cq:jar
:geode-cq:javadoc
:geode-cq:javadocJar
:geode-cq:sourcesJar
:geode-cq:signArchives SKIPPED
:geode-json:javadocJar
:geode-json:sourcesJar
:geode-json:signArchives SKIPPED
:geode-lucene:jar
:geode-lucene:javadoc
:geode-lucene:javadocJar
:geode-lucene:sourcesJar
:geode-lucene:signArchives SKIPPED
:geode-old-client-support:jar
:geode-old-client-support:javadoc
:geode-old-client-support:javadocJar
:geode-old-client-support:sourcesJar
:geode-old-client-support:signArchives SKIPPED
:geode-protobuf:jar
:geode-protobuf:javadoc
:geode-protobuf:javadocJar
:geode-protobuf:sourcesJar
:geode-protobuf:signArchives SKIPPED
:geode-pulse:javadoc

Re: Review Request 61166: GEODE-3313: Test utility supports building jar files with multiple classes

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61166/
---

(Updated Aug. 1, 2017, 6:31 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

Fix broken test


Repository: geode


Description
---

GEODE-3313: Test utility supports building jar files with multiple classes


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractor.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractorTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/CompiledSourceCode.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilder.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilderTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompiler.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompilerTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCode.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCodeTest.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractFunction.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ConcreteExtendsAbstractExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsAbstractFunction.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ImplementsFunction.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/61166/diff/6/

Changes: https://reviews.apache.org/r/61166/diff/5-6/


Testing
---

Precheckin running


Thanks,

Jared Stewart

[GitHub] geode pull request #651: GEODE-3230: Cleaning up unused (Cli)Strings

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130677475
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/Launcher.java 
---
@@ -99,7 +99,6 @@ protected Launcher() {
 this.allowedCommandLineCommands.add(CliStrings.START_JCONSOLE);
 this.allowedCommandLineCommands.add(CliStrings.START_JVISUALVM);
 this.allowedCommandLineCommands.add(CliStrings.START_LOCATOR);
-this.allowedCommandLineCommands.add(CliStrings.START_MANAGER);
--- End diff --

Can you explain the reason this change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #651: GEODE-3230: Cleaning up unused (Cli)Strings

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130678413
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 ---
@@ -633,8 +631,8 @@ public DataCommandResult put(String key, String value, 
boolean putIfAbsent, Stri
   try {
 keyObject = getClassObject(key, keyClass);
   } catch (ClassNotFoundException e) {
-return DataCommandResult.createPutResult(key, null, null,
-"ClassNotFoundException " + keyClass, false);
+return DataCommandResult.createPutResult(key, null, 
e.getException(),
--- End diff --

I think we can pass the Exception `e` itself rather than `e.getException()`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #651: GEODE-3230: Cleaning up unused (Cli)Strings

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130677047
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/MBeanProcessController.java
 ---
@@ -260,6 +265,7 @@ private JMXServiceURL getJMXServiceURL() throws 
AttachNotSupportedException, IOE
 // need to load the management-agent and get the address
 
 final String javaHome = 
vm.getSystemProperties().getProperty("java.home");
+assertState(StringUtils.isNotBlank(javaHome), 
CliStrings.JAVA_HOME_NOT_FOUND_ERROR_MESSAGE);
--- End diff --

I worry about changing the exception the exception thrown here (before I 
think it would have thrown an `IOException`).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #675: GEODE-3339: Refactoring ClusterConfigurationService...

[YehEmily]

GitHub user YehEmily opened a pull request:

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

GEODE-3339: Refactoring ClusterConfigurationServiceEndToEndDUnitTest

[View the JIRA ticket 
here.](https://issues.apache.org/jira/browse/GEODE-3339)

ClusterConfigurationServiceEndToEndDUnitTest has been refactored to use 
test rules instead of CliCommandsTestBase, and a version of the test that uses 
HTTP connection was also added.

- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [x] Have you written or updated unit tests to verify your changes?

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/YehEmily/geode GEODE-3339

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/675.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #675


commit 1f9ad5523bd04706cc96a48db990b4452548a7da
Author: YehEmily 
Date:   2017-08-01T17:30:09Z

GEODE-3339: Refactoring ClusterConfigurationServiceEndToEndDUnitTest




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #672: GEODE-3256: Refactoring DataCommands

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/672#discussion_r130673499
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommandsUtils.java
 ---
@@ -0,0 +1,311 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.StringTokenizer;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.StringUtils;
+
+import org.apache.geode.LogWriter;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.execute.FunctionService;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.DistributedRegionMXBean;
+import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.MBeanJMXAdapter;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.LogWrapper;
+import org.apache.geode.management.internal.cli.domain.DataCommandRequest;
+import org.apache.geode.management.internal.cli.domain.DataCommandResult;
+import 
org.apache.geode.management.internal.cli.functions.DataCommandFunction;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.CompositeResultData;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.TabularResultData;
+
+public class DataCommandsUtils {
--- End diff --

I tend to try to avoid `---Utils` classes, as they often end up as a bag of 
unrelated methods rather than a true object-oriented class with a single 
responsibility.  I'm not sure where all of these methods belong in this case 
though, and I think this is certainly a step in the right direction from the 
old `DataCommands` monolith.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Build failed in Jenkins: Geode-nightly #913

[Apache Jenkins Server]

See 


Changes:

[hiteshk25] GEODE-3286: Failing to cleanup connections from ConnectionTable 
receiver

[browe] GEODE-3286: incorporating review feedback

[browe] GEODE-3286: Improving closed connection check

[jiliao] GEODE-2971: Introduce ExitCode to resolve inconsistency of shell exit

[jstewart] GEODE-3317: Fix UniversalMembershipListenerAdapterDUnitTest

[kmiller] GEODE-3324 Document finer-grained security permissions

[lhughesgodfrey] GEODE-2226: SessionReplicationIntegrationTests do not run on 
Windows

--
[...truncated 706.53 KB...]
:geode-cq:testClasses
:geode-cq:checkMissedTests
:geode-cq:spotlessJavaCheck
:geode-cq:spotlessCheck
:geode-cq:test
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources
:geode-json:testClasses
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJavaNote: 

 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:integrationTest
:geode-old-versions:javadoc UP-TO-DATE
:geode-old-versions:javadocJar
:geode-old-versions:sourcesJar
:geode-old-versions:signArchives SKIPPED
:geode-old-versions:assemble
:geode-old-versions:compileTestJava UP-TO-DATE
:geode-old-versions:processTestResources UP-TO-DATE
:geode-old-versions:testClasses UP-TO-DATE
:geode-old-versions:checkMissedTests UP-TO-DATE
:geode-old-versions:spotlessJavaCheck
:geode-old-versions:spotlessCheck
:geode-old-versions:test UP-TO-DATE
:geode-old-versions:check
:geode-old-versions:build
:geode-old-versions:distributedTest UP-TO-DATE
:geode-old-versions:integrationTest UP-TO-DATE
:geode-protobuf:assemble
:geode-protobuf:extractIncludeTestProto
:geode-protobuf:extractTestProto UP-TO-DATE
:geode-protobuf:generateTestProto UP-TO-DATE
:geode-protobuf:compileTestJavaNote: Some input files use unchecked or unsafe 
operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-protobuf:processTestResources
:geode-protobuf:testClasses
:geode-protobuf:checkMissedTests
:geode-protobuf:spotlessJavaCheck
:geode-protobuf:spotlessCheck
:geode-protobuf:test
:geode-protobuf:check
:geode-protobuf:build
:geode-protobuf:distributedTest
:geode-protobuf:integrationTest
:geode-pulse:assemble
:geode-pulse:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: 

 uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-pulse:processTestResources
:geode-pulse:testClasses
:geode-pulse:checkMissedTests
:geode-pulse:spotlessJavaCheck
:geode-pulse:spotlessCheck
:geode-pulse:test
:geode-pulse:check
:geode-pulse:build
:geode-pulse:distributedTest
:geode-pulse:integrationTest
:geode-rebalancer:assemble
:geode-rebalancer:compileTestJava
:geode-rebalancer:processTestResources UP-TO-DATE
:geode-rebalancer:testClasses
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck

[GitHub] geode pull request #671: GEODE-3255: Refactor CreateAlterDestroyRegionComman...

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/671#discussion_r130668398
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
 ---
@@ -1143,4 +721,26 @@ private boolean 
isAttributePersistent(RegionAttributes attributes) {
 return attributes != null && attributes.getDataPolicy() != null
 && attributes.getDataPolicy().toString().contains("PERSISTENT");
   }
+
+  private static boolean regionExists(InternalCache cache, String 
regionPath) {
--- End diff --

I don't see any tests that validate the behavior of this method.  I think 
we can simplify it to: 
```
private static boolean regionExists(InternalCache cache, String regionPath) 
{
if (regionPath == null || Region.SEPARATOR.equals(regionPath)) {
  return false;
}

ManagementService managementService = 
ManagementService.getExistingManagementService(cache);
DistributedSystemMXBean dsMBean = 
managementService.getDistributedSystemMXBean();

String[] allRegionPaths = dsMBean.listAllRegionPaths();
return Arrays.stream(allRegionPaths).anyMatch(regionPath::equals);
  }
```

But it would be nice to have a test that would fail with this 
implementation:
```
private static boolean regionExists(InternalCache cache, String regionPath) 
{
return true;
}
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #665: GEODE-3254: Refactoring ConfigCommands and ConfigCo...

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/665#discussion_r130658877
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/Interceptor.java
 ---
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Map;
+
+import org.apache.geode.management.cli.Result;
+import 
org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
+import org.apache.geode.management.internal.cli.GfshParseResult;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.shell.Gfsh;
+
+/**
+ * Interceptor used by gfsh to intercept execution of export config 
command at "shell".
+ */
+public class Interceptor extends AbstractCliAroundInterceptor {
--- End diff --

This class appears to be unused.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #665: GEODE-3254: Refactoring ConfigCommands and ConfigCo...

[jaredjstewart]

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/665#discussion_r130661310
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommandDUnitTest.java
 ---
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.apache.commons.io.FileUtils.deleteDirectory;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.Properties;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.FlakyTest;
+
+@Category(DistributedTest.class)
+public class ExportConfigCommandDUnitTest extends CliCommandTestBase {
+  private File managerConfigFile;
+  private File managerPropsFile;
+  private File vm1ConfigFile;
+  private File vm1PropsFile;
+  private File vm2ConfigFile;
+  private File vm2PropsFile;
+  private File shellConfigFile;
+  private File shellPropsFile;
+  private File subDir;
+  private File subManagerConfigFile;
+
+  @Override
+  protected final void postSetUpCliCommandTestBase() throws Exception {
+this.managerConfigFile = 
this.temporaryFolder.newFile("Manager-cache.xml");
+this.managerPropsFile = 
this.temporaryFolder.newFile("Manager-gf.properties");
+this.vm1ConfigFile = this.temporaryFolder.newFile("VM1-cache.xml");
+this.vm1PropsFile = this.temporaryFolder.newFile("VM1-gf.properties");
+this.vm2ConfigFile = this.temporaryFolder.newFile("VM2-cache.xml");
+this.vm2PropsFile = this.temporaryFolder.newFile("VM2-gf.properties");
+this.shellConfigFile = this.temporaryFolder.newFile("Shell-cache.xml");
+this.shellPropsFile = 
this.temporaryFolder.newFile("Shell-gf.properties");
+this.subDir = this.temporaryFolder.newFolder(getName());
+this.subManagerConfigFile = new File(this.subDir, 
this.managerConfigFile.getName());
+  }
+
+  @Category(FlakyTest.class) // GEODE-1449
+  @Test
+  public void testExportConfig() throws Exception {
+Properties localProps = new Properties();
+localProps.setProperty(NAME, "Manager");
+localProps.setProperty(GROUPS, "Group1");
+setUpJmxManagerOnVm0ThenConnect(localProps);
+
+// Create a cache in another VM (VM1)
+Host.getHost(0).getVM(1).invoke(new SerializableRunnable() {
+  public void run() {
+Properties localProps = new Properties();
+localProps.setProperty(NAME, "VM1");
+localProps.setProperty(GROUPS, "Group2");
+getSystem(localProps);
+getCache();
+  }
+});
+
+// Create a cache in a 3rd VM (VM2)
+Host.getHost(0).getVM(2).invoke(new SerializableRunnable() {
+  public void run() {
+Properties localProps = new Properties();
+localProps.setProperty(NAME, "VM2");
+localProps.setProperty(GROUPS, "Group2");
+getSystem(localProps);
+getCache();
+  }
+});
+
+// Create a cache in the local VM
+localProps = new Properties();

Re: 2 unit tests fail in geode-core

[Kirk Lund]

I actually have a change that will be ready to merge to develop soon in
which I've rewritten LocatorLauncherTest and ServerLauncherTest. The
changes I have will fix these two failures that you're seeing.

I'm not aware of any recent changes that would've broken these two tests --
the two bind address tests that use yahoo.com already had bugs filed from a
similar intermittent failure during Geode Nightly Builds.

On Tue, Aug 1, 2017 at 8:59 AM, Anton Mironenko 
wrote:

> Hi Nabarun,
> My connectivity to the Internet is done via HTTP proxy. Maybe this can be
> the reason. But it is not an excuse for a test not to pass.
> Two weeks ago the tests passed, and now they don't. Seems like something
> was broken in the code.
>
> BR,
> Anton
>
> -Original Message-
> From: Nabarun Nag [mailto:n...@apache.org]
> Sent: Tuesday, August 01, 2017 18:06
> To: dev@geode.apache.org
> Subject: Re: 2 unit tests fail in geode-core
>
> Hi Anton,
>
> I was able to reproduce the issue if I shut down my wifi and remove my
> ethernet cable from my Mac [no network connections active / on]. Once wifi
> is switched on or ethernet is connected to the machine the tests pass.
>
> Regards
> Nabarun Nag
>
> > On Aug 1, 2017, at 7:48 AM, Anton Mironenko 
> wrote:
> >
> > gradlew geode-core:test
>
> This message and the information contained herein is proprietary and
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer <
> https://www.amdocs.com/about/email-disclaimer>
>
>

[GitHub] geode pull request #674: GEODE-3380: There're 2 problems here

[nabarunnag]

Github user nabarunnag commented on a diff in the pull request:

https://github.com/apache/geode/pull/674#discussion_r130654524
  
--- Diff: 
geode-cq/src/test/java/org/apache/geode/internal/cache/PutAllCSDUnitTest.java 
---
@@ -500,6 +502,81 @@ public void run2() throws CacheException {
   }
 
   /**
+   * Create PR without redundancy on 2 servers with lucene index. Feed 
some key s. From a client, do
+   * removeAll on keys in server1. During the removeAll, restart server1 
and trigger the removeAll
+   * to retry. The retried removeAll should return the version tag of 
tombstones. Do removeAll again
+   * on the same key, it should get the version tag again.
+   */
+  @Test
+  public void test51871() throws CacheException, InterruptedException {
--- End diff --

Will it be possible to change the test name to follow the when() naming convention


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

RE: 2 unit tests fail in geode-core

[Anton Mironenko]

Hi Nabarun,
My connectivity to the Internet is done via HTTP proxy. Maybe this can be the 
reason. But it is not an excuse for a test not to pass.
Two weeks ago the tests passed, and now they don't. Seems like something was 
broken in the code.

BR, 
Anton

-Original Message-
From: Nabarun Nag [mailto:n...@apache.org] 
Sent: Tuesday, August 01, 2017 18:06
To: dev@geode.apache.org
Subject: Re: 2 unit tests fail in geode-core

Hi Anton,

I was able to reproduce the issue if I shut down my wifi and remove my ethernet 
cable from my Mac [no network connections active / on]. Once wifi is switched 
on or ethernet is connected to the machine the tests pass.

Regards
Nabarun Nag

> On Aug 1, 2017, at 7:48 AM, Anton Mironenko  wrote:
> 
> gradlew geode-core:test

This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer 

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130647800
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandlerJUnitTest.java
 ---
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import 
org.apache.geode.protocol.protobuf.ServerAPI.GetAvailableServersResponse;
+import org.apache.geode.protocol.protobuf.Success;
+import 
org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@Category(UnitTest.class)
+public class GetAvailableServersOperationHandlerJUnitTest extends 
OperationHandlerJUnitTest {
+
+  private TcpClient mockTCPClient;
+
+  @Before
+  public void setUp() throws Exception {
+super.setUp();
+
+operationHandler = mock(GetAvailableServersOperationHandler.class);
+cacheStub = mock(GemFireCacheImpl.class);
+when(operationHandler.process(any(), any(), 
any())).thenCallRealMethod();
+InternalDistributedSystem mockDistributedSystem = 
mock(InternalDistributedSystem.class);
--- End diff --

Can we have some blank lines as separators between segments that pertain to 
a single mock? Right now this is a fairly impenetrable wall of text.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130646564
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/GetAvailableServersDUnitTest.java
 ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol;
+
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.protocol.exception.InvalidProtocolMessageException;
+import org.apache.geode.protocol.protobuf.ClientProtocol;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import 
org.apache.geode.protocol.protobuf.serializer.ProtobufProtocolSerializer;
+import 
org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities;
+import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import 
org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.IOException;
+import java.net.Socket;
+
+import static org.junit.Assert.assertEquals;
+
+@Category(DistributedTest.class)
+public class GetAvailableServersDUnitTest extends JUnit4CacheTestCase {
+
+  @Rule
+  public DistributedRestoreSystemProperties 
distributedRestoreSystemProperties =
+  new DistributedRestoreSystemProperties();
+
+  @Before
+  public void setup() {
+
+  }
+
+  @Test
+  public void testGetAllAvailableServersRequest()
+  throws IOException, InvalidProtocolMessageException {
+Host host = Host.getHost(0);
+VM vm0 = host.getVM(0);
+VM vm1 = host.getVM(1);
+VM vm2 = host.getVM(2);
+
+int locatorPort = DistributedTestUtils.getDUnitLocatorPort();
+
+// int cacheServer1Port = vm0.invoke("Start Cache1", () -> 
startCacheWithCacheServer());
--- End diff --

let's get rid off this


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130646340
  
--- Diff: geode-protobuf/src/main/proto/clientProtocol.proto ---
@@ -43,13 +43,12 @@ message Request {
 GetAllRequest getAllRequest = 5;
 RemoveRequest removeRequest = 6;
 RemoveAllRequest removeAllRequest = 7;
-ListKeysRequest listKeysRequest = 8;
 
 CreateRegionRequest createRegionRequest = 21;
 DestroyRegionRequest destroyRegionRequest = 22;
 
-PingRequest pingRequest = 41;
-GetServersRequest getServersRequest = 42;
+//PingRequest pingRequest = 41;
--- End diff --

what are you saving it for?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130646425
  
--- Diff: geode-protobuf/src/main/proto/clientProtocol.proto ---
@@ -64,15 +63,14 @@ message Response {
 GetAllResponse getAllResponse = 5;
 RemoveResponse removeResponse = 6;
 RemoveAllResponse removeAllResponse = 7;
-ListKeysResponse listKeysResponse = 8;
 
 ErrorResponse errorResponse = 13;
 
 CreateRegionResponse createRegionResponse = 20;
 DestroyRegionResponse destroyRegionResponse = 21;
 
-PingResponse pingResponse = 41;
-GetServersResponse getServersResponse = 42;
+//PingResponse pingResponse = 41;
--- End diff --

same as above


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130646177
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufUtilities.java
 ---
@@ -140,7 +141,7 @@
 
   /**
* This creates a MessageHeader
-   *
+   * 
--- End diff --

what's up with adding all this white trailing white space? Is this from 
spotless?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130645759
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
 ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.admin.remote.DistributionLocatorId;
+import org.apache.geode.protocol.operations.OperationHandler;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import org.apache.geode.protocol.protobuf.Success;
+import org.apache.geode.serialization.SerializationService;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+public class GetAvailableServersOperationHandler implements
+OperationHandler {
+
+  @Override
+  public Result process(
+  SerializationService serializationService, 
ServerAPI.GetAvailableServersRequest request,
+  Cache cache) {
+
+InternalDistributedSystem distributedSystem =
+(InternalDistributedSystem) cache.getDistributedSystem();
+Properties properties = distributedSystem.getProperties();
+String locatorsString = 
properties.getProperty(ConfigurationProperties.LOCATORS);
+
+HashSet locators = new HashSet();
+StringTokenizer stringTokenizer = new StringTokenizer(locatorsString, 
",");
+while (stringTokenizer.hasMoreTokens()) {
+  String locator = stringTokenizer.nextToken();
+  if (StringUtils.isNotEmpty(locator)) {
+locators.add(new DistributionLocatorId(locator));
+  }
+}
+
+TcpClient tcpClient = getTcpClient();
+for (DistributionLocatorId locator : locators) {
+  try {
+return getGetAvailableServersFromLocator(tcpClient, 
locator.getHost());
+  } catch (IOException | ClassNotFoundException e) {
+// try the next locator
+  }
+}
+return Failure
+.of(BasicTypes.ErrorResponse.newBuilder().setMessage("Unable to 
find a locator").build());
+  }
+
+  private Result 
getGetAvailableServersFromLocator(
+  TcpClient tcpClient, InetSocketAddress address) throws IOException, 
ClassNotFoundException {
+GetAllServersResponse getAllServersResponse = (GetAllServersResponse) 
tcpClient
+.requestToServer(address, new GetAllServersRequest(), 1000, true);
+Collection servers =
+(Collection) 
getAllServersResponse.getServers().stream()
+.map(serverLocation -> 
getServerProtobufMessage((ServerLocation) serverLocation))
+.collect(Collectors.toList());
+ServerAPI.GetAvailableServersResponse.Builder builder =
+
ServerAPI.GetAvailableServersResponse.newBuilder().addAllServers(servers);
--- End diff --

could we incrementally build this up as part of the stream processing?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not 

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130644766
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
 ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.admin.remote.DistributionLocatorId;
+import org.apache.geode.protocol.operations.OperationHandler;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import org.apache.geode.protocol.protobuf.Success;
+import org.apache.geode.serialization.SerializationService;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+public class GetAvailableServersOperationHandler implements
+OperationHandler {
+
+  @Override
+  public Result process(
+  SerializationService serializationService, 
ServerAPI.GetAvailableServersRequest request,
+  Cache cache) {
+
+InternalDistributedSystem distributedSystem =
+(InternalDistributedSystem) cache.getDistributedSystem();
+Properties properties = distributedSystem.getProperties();
+String locatorsString = 
properties.getProperty(ConfigurationProperties.LOCATORS);
+
+HashSet locators = new HashSet();
+StringTokenizer stringTokenizer = new StringTokenizer(locatorsString, 
",");
+while (stringTokenizer.hasMoreTokens()) {
+  String locator = stringTokenizer.nextToken();
+  if (StringUtils.isNotEmpty(locator)) {
+locators.add(new DistributionLocatorId(locator));
+  }
+}
+
+TcpClient tcpClient = getTcpClient();
+for (DistributionLocatorId locator : locators) {
--- End diff --

this is not super pretty. What do you think about using Stream#findFirst 
for this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/673#discussion_r130642543
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
 ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.protocol.protobuf.operations;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import 
org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import org.apache.geode.internal.admin.remote.DistributionLocatorId;
+import org.apache.geode.protocol.operations.OperationHandler;
+import org.apache.geode.protocol.protobuf.BasicTypes;
+import org.apache.geode.protocol.protobuf.Failure;
+import org.apache.geode.protocol.protobuf.Result;
+import org.apache.geode.protocol.protobuf.ServerAPI;
+import org.apache.geode.protocol.protobuf.Success;
+import org.apache.geode.serialization.SerializationService;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+public class GetAvailableServersOperationHandler implements
+OperationHandler {
+
+  @Override
+  public Result process(
+  SerializationService serializationService, 
ServerAPI.GetAvailableServersRequest request,
+  Cache cache) {
+
+InternalDistributedSystem distributedSystem =
+(InternalDistributedSystem) cache.getDistributedSystem();
+Properties properties = distributedSystem.getProperties();
+String locatorsString = 
properties.getProperty(ConfigurationProperties.LOCATORS);
--- End diff --

Could we pull the entire calculation of locators out either into a private 
method or a service object? I think that would make it much easier to 
understand what this method is doing. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: 2 unit tests fail in geode-core

[Nabarun Nag]

Hi Anton,

I was able to reproduce the issue if I shut down my wifi and remove my ethernet 
cable from my Mac [no network connections active / on]. Once wifi is switched 
on or ethernet is connected to the machine the tests pass.

Regards
Nabarun Nag

> On Aug 1, 2017, at 7:48 AM, Anton Mironenko  wrote:
> 
> gradlew geode-core:test

2 unit tests fail in geode-core

[Anton Mironenko]

Hello,
2 unit tests fail in geode-core on the latest commit 
9d59402b71beea84199c79399aa0260955a19d2c (August 1).
Whereas all unit tests passed on aa4878ef19d27a78454dc10b451199f088a2f37d (July 
18).

What is an easiest way to fix it?

gradlew geode-core:test
...
org.apache.geode.distributed.LocatorLauncherTest > 
testSetBindAddressToNonLocalHost FAILED
java.lang.Exception: Unexpected exception, 
expected but 
was

Caused by:
org.junit.ComparisonFailure: expected:<[yahoo.com is not an address for 
this machine].> but was:<[The hostname/IP address to which the Locator will be 
bound is unknown].>
at org.junit.Assert.assertEquals(Assert.java:115)
at org.junit.Assert.assertEquals(Assert.java:144)
at 
org.apache.geode.distributed.LocatorLauncherTest.testSetBindAddressToNonLocalHost(LocatorLauncherTest.java:167)

org.apache.geode.distributed.ServerLauncherTest > 
testSetServerBindAddressToNonLocalHost FAILED
   java.lang.Exception: Unexpected exception, 
expected but 
was

Caused by:
org.junit.ComparisonFailure: expected:<[yahoo.com is not an address for 
this machine].> but was:<[The hostname/IP address to which the Server will be 
bound is unknown].>

Anton Mironenko
Software Architect
Amdocs ASP team

This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer 

[GitHub] geode issue #664: Fix for GEODE-3292

[jujoramos]

Github user jujoramos commented on the issue:

https://github.com/apache/geode/pull/664
  
Hey @jaredjstewart,

Thanks for looking at this.
I'll write the tests, no worries at all.
Cheers.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---