Re: Gitbox enables the full GitHub workflow

2017-08-07 Thread Hong
Good to know!! The problem for Github issues is that the id is not
continuous(the id space is shared with Github's issues and pull requests)
which is not a good design IMO.

Hong

2017-08-08 10:01 GMT+08:00 Lei Chang :

> cool. this will simplify the commit workflow a lot.
>
> And if we can use github issues instead of apache JIRA, that will be more
> convenient.
>
> Cheers
> Lei
>
>
>
>
> On Tue, Aug 8, 2017 at 9:09 AM, Roman Shaposhnik 
> wrote:
>
> > Hi!
> >
> > it has just come to my attention that Gitbox at ASF
> > has been enabling full GitHub workflow (with being
> > able to click Merge this PR button, etc.) for quite
> > some time.
> >
> > This basically allows a project to have GH as a R/W
> > repo as opposed to R/O mirror of what we all currnently
> > have: https://gitbox.apache.org/repos/asf
> >
> > Personally I'm not sure I like GH workflow all that much,
> > but if there's interest -- you can opt-in into Gitbox. Once
> > you do -- your source of truth moves to GH. You can't
> > have it both ways with git-wip-us.apache.org and Gitbox.
> >
> > Thanks,
> > Roman.
> >
>


Re: Gitbox enables the full GitHub workflow

2017-08-07 Thread Lei Chang
cool. this will simplify the commit workflow a lot.

And if we can use github issues instead of apache JIRA, that will be more
convenient.

Cheers
Lei




On Tue, Aug 8, 2017 at 9:09 AM, Roman Shaposhnik 
wrote:

> Hi!
>
> it has just come to my attention that Gitbox at ASF
> has been enabling full GitHub workflow (with being
> able to click Merge this PR button, etc.) for quite
> some time.
>
> This basically allows a project to have GH as a R/W
> repo as opposed to R/O mirror of what we all currnently
> have: https://gitbox.apache.org/repos/asf
>
> Personally I'm not sure I like GH workflow all that much,
> but if there's interest -- you can opt-in into Gitbox. Once
> you do -- your source of truth moves to GH. You can't
> have it both ways with git-wip-us.apache.org and Gitbox.
>
> Thanks,
> Roman.
>


Re: Gitbox enables the full GitHub workflow

2017-08-07 Thread Jacob Barrett
+1 for Gitbox

This will greatly simplify the workflow committers go through with pull 
requests from the community. It will also allow us to close out pulls that go 
dark. On our local repos we would really have no need to ever include the 
apache repo so it reduces the number of remotes we need to track. 

-Jake

Sent from my iPhone

> On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik  wrote:
> 
> Hi!
> 
> it has just come to my attention that Gitbox at ASF
> has been enabling full GitHub workflow (with being
> able to click Merge this PR button, etc.) for quite
> some time.
> 
> This basically allows a project to have GH as a R/W
> repo as opposed to R/O mirror of what we all currnently
> have: https://gitbox.apache.org/repos/asf
> 
> Personally I'm not sure I like GH workflow all that much,
> but if there's interest -- you can opt-in into Gitbox. Once
> you do -- your source of truth moves to GH. You can't
> have it both ways with git-wip-us.apache.org and Gitbox.
> 
> Thanks,
> Roman.


Gitbox enables the full GitHub workflow

2017-08-07 Thread Roman Shaposhnik
Hi!

it has just come to my attention that Gitbox at ASF
has been enabling full GitHub workflow (with being
able to click Merge this PR button, etc.) for quite
some time.

This basically allows a project to have GH as a R/W
repo as opposed to R/O mirror of what we all currnently
have: https://gitbox.apache.org/repos/asf

Personally I'm not sure I like GH workflow all that much,
but if there's interest -- you can opt-in into Gitbox. Once
you do -- your source of truth moves to GH. You can't
have it both ways with git-wip-us.apache.org and Gitbox.

Thanks,
Roman.


Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-07 Thread Jared Stewart

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




geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
Line 292 (original), 292 (patched)


I might be missing something, but would callers of the no-arg variant 
`getClientQueueSizes()` potentially be prone to the same deadlock that 
originally affected `CacheServerBridge.getNumSubscriptions`?



geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
Lines 53 (patched)


I wonder if `startedExecuting` and `finishedExecuting` might make the 
intent of these variables clearer.



geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
Lines 91 (patched)


I really like this style of this test!



geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
Lines 97 (patched)


It might be nice to have a commment (or to wrap line this in a named 
method) to indicate that this line is expected to obtain the lock on 
`CacheFactory`.


- Jared Stewart


On Aug. 8, 2017, 12:19 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61487/
> ---
> 
> (Updated Aug. 8, 2017, 12:19 a.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3407
> https://issues.apache.org/jira/browse/GEODE-3407
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Change InternalClientMembership to not synchronize on CacheFactory
> by accepting Cache parameter from CacheServerBridge.
> 
> New regression test confirms bug and this fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
>  504081d65515adb294dd43ecffee450477f08339 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
>  728402c8a290d88026db753657e26e5f7440a990 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
>  6834998da13deec5e074a61e5373ec2f8dce2ad7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61487/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-07 Thread Kirk Lund

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

Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


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


Repository: geode


Description
---

Change InternalClientMembership to not synchronize on CacheFactory
by accepting Cache parameter from CacheServerBridge.

New regression test confirms bug and this fix.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
 504081d65515adb294dd43ecffee450477f08339 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
 728402c8a290d88026db753657e26e5f7440a990 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
 6834998da13deec5e074a61e5373ec2f8dce2ad7 
  
geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
 PRE-CREATION 


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


Testing
---

precheckin in progress


Thanks,

Kirk Lund



[GitHub] geode pull request #697: GEODE-3407: fix deadlock between JMX and Reconnect

2017-08-07 Thread kirklund
GitHub user kirklund opened a pull request:

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

GEODE-3407: fix deadlock between JMX and Reconnect

Change InternalClientMembership to not synchronize on CacheFactory
by accepting Cache parameter from CacheServerBridge.

New regression test confirms bug and this fix.

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?

- [x] 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/kirklund/geode feature/GEM-1256

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

https://github.com/apache/geode/pull/697.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 #697


commit ba7a323cbbd68f4de8b0968038fe1a604c861fcb
Author: Kirk Lund 
Date:   2017-08-07T23:39:04Z

GEODE-3407: fix deadlock between JMX and Membership

Change InternalClientMembership to not synchronize on CacheFactory
by accepting Cache parameter from CacheServerBridge.

New regression test confirms bug and this fix.




---
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 #690: GEODE-3262: Refactoring IndexCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/690#discussion_r131790263
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java
 ---
@@ -0,0 +1,197 @@
+/*
+ * 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.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.Cache;
--- End diff --

My eternal nitpicking: This import 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 issue #691: GEODE-3260: Refactoring FunctionCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on the issue:

https://github.com/apache/geode/pull/691
  
+1.

A far-from-critical nitpick: there are several places in 
`ExecuteFunctionCommand` and one in `DestroyFunctionCommand` that are of the 
form

```
result = ResultBuilder.[...];
return result;
```

In other instances, the result is returned directly

```
   return ResultBuilder.[...];
```

I'd change those few assign-and-immediately-return to just return the 
object directly.  You might then be able to delete the declaration of the 
variable 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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread YehEmily
Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131787338
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java
 ---
@@ -0,0 +1,107 @@
+/*
+ * 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.geode.management.internal.cli.commands.DiskStoreCommandsUtils.configureLogging;
--- End diff --

Fixed! I also found one more occurrence in `ValidateDiskStoreCommand` and 
removed that one, too. Thank you!


---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread pdxrunner
Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131787137
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java
 ---
@@ -0,0 +1,107 @@
+/*
+ * 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.geode.management.internal.cli.commands.DiskStoreCommandsUtils.configureLogging;
--- End diff --

Remove import. See my previous moment regarding utility method calls.


---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread pdxrunner
Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131787085
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java
 ---
@@ -0,0 +1,179 @@
+/*
+ * 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.geode.management.internal.cli.commands.DiskStoreCommandsUtils.configureLogging;
--- End diff --

Remove import. See my previous moment regarding utility method calls.


---
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 #693: GEODE-3268: Refactoring RegionCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/693#discussion_r131775864
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeRegionCommand.java
 ---
@@ -333,9 +247,8 @@ private void 
writeCommonAttributesToTable(TabularResultData table, String attrib
 }
   }
 
-  private boolean writeFixedPartitionAttributesToTable(TabularResultData 
table,
-  String attributeType, List fpaList, 
String member,
-  boolean isMemberNameAdded) {
+  private void writeFixedPartitionAttributesToTable(TabularResultData 
table,
--- End diff --

In this function, a couple of low-hanging cleanups could be:

(1) Change the big `if` to a guard clause
```
if (fpaList == null) {
return;
}
```

(2) Change the iterated `while` loop to a `for:` loop.

Also, both of these can be cleanly easily handled by your IDE (although you 
might need to move the `Iterator fpaIter = 
fpaList.iterator();` next to the `while` first).


---
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 #693: GEODE-3268: Refactoring RegionCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/693#discussion_r131776451
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeRegionCommand.java
 ---
@@ -359,7 +272,7 @@ private boolean 
writeFixedPartitionAttributesToTable(TabularResultData table,
 fpaBuilder.append(fpa.getNumBuckets());
 
 if (!isTypeAdded) {
-  type = attributeType;
+  type = "";
--- End diff --

I agree with the removal of a constant parameter, but in this case, we're 
not distinguishing between `type = "";` here vs `type = blank; // = ""` a few 
lines below. 
 These could be removed altogether, with `""` passed directly to the 
`writeAttributeToTable` call at the end of this method.


---
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 #693: GEODE-3268: Refactoring RegionCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/693#discussion_r131783322
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
 ---
@@ -0,0 +1,113 @@
+/*
+ * 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.LinkedHashSet;
+import java.util.Set;
+import java.util.TreeSet;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.execute.FunctionInvocationTargetException;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.domain.RegionInformation;
+import 
org.apache.geode.management.internal.cli.functions.GetRegionsFunction;
+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.result.TabularResultData;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class ListRegionCommand implements GfshCommand {
+  private static final GetRegionsFunction getRegionsFunction = new 
GetRegionsFunction();
+
+  @CliCommand(value = {CliStrings.LIST_REGION}, help = 
CliStrings.LIST_REGION__HELP)
+  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION)
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+  operation = ResourcePermission.Operation.READ)
+  public Result listRegion(
+  @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+  optionContext = ConverterHint.MEMBERGROUP,
+  help = CliStrings.LIST_REGION__GROUP__HELP) String[] group,
+  @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+  optionContext = ConverterHint.MEMBERIDNAME,
+  help = CliStrings.LIST_REGION__MEMBER__HELP) String[] 
memberNameOrId) {
+Result result = null;
+try {
+  Set regionInfoSet = new LinkedHashSet<>();
+  ResultCollector rc;
+  Set targetMembers = CliUtil.findMembers(group, 
memberNameOrId);
+
+  if (targetMembers.isEmpty()) {
+return 
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+  }
+
+  TabularResultData resultData = 
ResultBuilder.createTabularResultData();
+  rc = CliUtil.executeFunction(getRegionsFunction, null, 
targetMembers);
+  ArrayList resultList = (ArrayList) rc.getResult();
+
+  if (resultList != null) {
--- End diff --

It is arguably beyond the scope of this ticket to refactor this, but this 
sort of nesting makes my skin crawl.

Dropping line 72 because `instanceof` requires non-`null`.  
([Docs](http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.20.2))

I'm pretty confident this is equivalent, but it would certainly warrant a 
new precheckin.

```
  Set regionInfoSet;
  if (resultList != null) {
regionInfoSet = resultList.stream()
.filter(resultObj -> resultObj instanceof Object[])
.map(resultObj -> (Object[]) resultObj)
.flatMap(Arrays::stream)
.filter(regionInfo -> regionInfo instanceof RegionInformation)
.map(regionInfo -> (RegionInformation) regionInfo)
.collect(Collectors.toCollection(LinkedHashSet::new));
// ...
} // ends if (result != null)

Come to that, you could flatten a 

[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands

2017-08-07 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/693#discussion_r131775100
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java
 ---
@@ -26,11 +26,12 @@
 import org.springframework.web.context.request.WebRequest;
 
 /**
- * The RegionCommands class implements GemFire Management REST API web 
service endpoints for the
- * Gfsh Region Commands.
+ * The ListRegionCommand and DescribeRegionCommand classes implement 
GemFire Management REST API web
--- End diff --

My eternal nitpick: make sure you optimize / reorder imports in the files 
you touch, please.


---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread YehEmily
Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131783193
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java
 ---
@@ -0,0 +1,178 @@
+/*
+ * 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.geode.management.internal.cli.commands.DiskStoreCommandsUtils.configureLogging;
--- End diff --

Agreed - thank you for your feedback! I'll update the PR!


---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread YehEmily
Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131783008
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactDiskStoreCommand.java
 ---
@@ -0,0 +1,185 @@
+/*
+ * 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.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.persistence.PersistentID;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.DistributedSystemMXBean;
+import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.LogWrapper;
+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.messages.CompactRequest;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class CompactDiskStoreCommand implements GfshCommand {
+  @CliCommand(value = CliStrings.COMPACT_DISK_STORE, help = 
CliStrings.COMPACT_DISK_STORE__HELP)
+  @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE})
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+  operation = ResourcePermission.Operation.MANAGE, target = 
ResourcePermission.Target.DISK)
+  public Result compactDiskStore(
+  @CliOption(key = CliStrings.COMPACT_DISK_STORE__NAME, mandatory = 
true,
+  optionContext = ConverterHint.DISKSTORE,
+  help = CliStrings.COMPACT_DISK_STORE__NAME__HELP) String 
diskStoreName,
+  @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+  help = CliStrings.COMPACT_DISK_STORE__GROUP__HELP) String[] 
groups) {
+Result result;
+
+try {
+  // disk store exists validation
+  if (!diskStoreExists(diskStoreName)) {
+result = ResultBuilder.createUserErrorResult(
+
CliStrings.format(CliStrings.COMPACT_DISK_STORE__DISKSTORE_0_DOES_NOT_EXIST,
+new Object[] {diskStoreName}));
+  } else {
+InternalDistributedSystem ds = 
getCache().getInternalDistributedSystem();
+
+Map overallCompactInfo = new 
HashMap<>();
+
+Set otherMembers = 
ds.getDistributionManager().getOtherNormalDistributionManagerIds();
+Set allMembers = new HashSet<>();
+
+for (Object member : otherMembers) {
+  allMembers.add((InternalDistributedMember) member);
+}
+allMembers.add(ds.getDistributedMember());
+
+String groupInfo = "";
+// if groups are specified, find members in the specified group
+if (groups != null && groups.length > 0) {
+  groupInfo = 
CliStrings.format(CliStrings.COMPACT_DISK_STORE__MSG__FOR_GROUP,
+  new Object[] {Arrays.toString(groups) + "."});
+  final Set selectedMembers = new 
HashSet<>();
+  List targetedGroups = Arrays.asList(groups);
+  for 

[GitHub] geode pull request #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread pdxrunner
Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131782938
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java
 ---
@@ -0,0 +1,178 @@
+/*
+ * 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.geode.management.internal.cli.commands.DiskStoreCommandsUtils.configureLogging;
--- End diff --

I would delete this import. Both of the DiskStoreCommandsUtils methods 
should be treated the same. Either provide static imports for both 
configureLogging and validatedDirectories, or for neither of them. I would opt 
for the later so it's obvious from the calls that these are static utility 
methods.


---
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.
---


[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #640 was SUCCESSFUL (with 2024 tests)

2017-08-07 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #640 was successful.
---
Scheduled
2026 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

[GitHub] geode pull request #696: GEODE-3265: Refactoring MiscellaneousCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3265: Refactoring MiscellaneousCommands

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

`MiscellaneousCommands` was a huge class that contained a large number of 
commands. These commands have been split into their own individual command 
classes.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/696.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 #696


commit f130cf3999de22164f405664a29a74d1e8838bba
Author: YehEmily 
Date:   2017-08-07T22:37:23Z

GEODE-3265: Refactoring MiscellaneousCommands




---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread pdxrunner
Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/687#discussion_r131777554
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactDiskStoreCommand.java
 ---
@@ -0,0 +1,185 @@
+/*
+ * 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.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.persistence.PersistentID;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.DistributedSystemMXBean;
+import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.LogWrapper;
+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.messages.CompactRequest;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class CompactDiskStoreCommand implements GfshCommand {
+  @CliCommand(value = CliStrings.COMPACT_DISK_STORE, help = 
CliStrings.COMPACT_DISK_STORE__HELP)
+  @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE})
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+  operation = ResourcePermission.Operation.MANAGE, target = 
ResourcePermission.Target.DISK)
+  public Result compactDiskStore(
+  @CliOption(key = CliStrings.COMPACT_DISK_STORE__NAME, mandatory = 
true,
+  optionContext = ConverterHint.DISKSTORE,
+  help = CliStrings.COMPACT_DISK_STORE__NAME__HELP) String 
diskStoreName,
+  @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+  help = CliStrings.COMPACT_DISK_STORE__GROUP__HELP) String[] 
groups) {
+Result result;
+
+try {
+  // disk store exists validation
+  if (!diskStoreExists(diskStoreName)) {
+result = ResultBuilder.createUserErrorResult(
+
CliStrings.format(CliStrings.COMPACT_DISK_STORE__DISKSTORE_0_DOES_NOT_EXIST,
+new Object[] {diskStoreName}));
+  } else {
+InternalDistributedSystem ds = 
getCache().getInternalDistributedSystem();
+
+Map overallCompactInfo = new 
HashMap<>();
+
+Set otherMembers = 
ds.getDistributionManager().getOtherNormalDistributionManagerIds();
+Set allMembers = new HashSet<>();
+
+for (Object member : otherMembers) {
+  allMembers.add((InternalDistributedMember) member);
+}
+allMembers.add(ds.getDistributedMember());
+
+String groupInfo = "";
+// if groups are specified, find members in the specified group
+if (groups != null && groups.length > 0) {
+  groupInfo = 
CliStrings.format(CliStrings.COMPACT_DISK_STORE__MSG__FOR_GROUP,
+  new Object[] {Arrays.toString(groups) + "."});
+  final Set selectedMembers = new 
HashSet<>();
+  List targetedGroups = Arrays.asList(groups);
+  for 

[GitHub] geode pull request #695: GEODE-3267: Refactoring QueueCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3267: Refactoring QueueCommands

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

`QueueCommands` was a big class that contained two commands (each class 
should only contain one). These commands have been split into their own 
individual command classes.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/695.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 #695


commit 48f6616ea10cf2ae0bdea9f698b6531d35018ff4
Author: YehEmily 
Date:   2017-08-07T21:47:15Z

GEODE-3267: Refactoring QueueCommands




---
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 #694: GEODE-3270: Refactoring (renaming) StatusCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3270: Refactoring (renaming) StatusCommands

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

`StatusCommands` was a class that already contained a single command. A 
better-fitting name for the class is `StatusClusterConfigServiceCommand`.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

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

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

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

https://github.com/apache/geode/pull/694.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 #694


commit fcf96b0e24a5a7a3ff93a11e4a5cdb3aa6127e49
Author: YehEmily 
Date:   2017-08-07T21:32:43Z

GEODE-3270: Refactoring (renaming) StatusCommands




---
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.
---


[DRAFT] Geode Board Report August 2017

2017-08-07 Thread Dan Smith
Hi folks,

Please review the below draft for our report to the board for this past
quarter and offer corrections. As Mark mentioned, we need to submit the
draft this Wednesday.

Thanks!
-Dan

-

## Description:

- Apache Geode provides a database-like consistency model, reliable
transaction processing and a shared-nothing architecture to maintain very
low
latency performance with high concurrency processing.

## Issues:

- There are no issues requiring board attention at this time.

## Activity:

- Geode 1.2 was released with over 300 tickets resolved, including
  - Integration with Apache Lucene is no longer experimental
  - A built in partition resolver
  - Examples bundled with the distribution
- ApacheCon presentation - Building an IoT platform with Apache Apollo and
Apache Geode by Swapnil Bawaskar

## Health report:

- We’re continuing to work on attracting new contributors and making it
easier
to participate in the community.
- Mailing list activity is healthy.
- Work has started towards the next Geode release, 1.3.0 including
development of a new client/server protocol.

## PMC changes:

- Currently 36 PMC members (+1 since last report)
- Joey McAllister was added to the PMC on Mon Jul 24 2017

## Committer base changes:

- Currently 81 committers (+2 since last report)
- Joey McAllister was added as a committer on Tue Jul 25 2017
- Deepak Dixit was added as a committer on Thu Jul 13 2017

## Releases:

- 1.2.0 was released on Wed July 17th 2017

## Mailing list activity:

- Mailing lists remain active and we’re seeing continued growth in
subscriber
counts.  Mailing list traffic on dev went down because we redirected jira
traffic to iss...@geode.apache.org, but the combined activity increased.

- dev@geode.apache.org:
 - 174 subscribers (up 10 in the last 3 months)
 - 4264 emails sent in the past 3 months, 7166 in the previous cycle

- iss...@geode.apache.org:
 - 56 subscribers (up 1 in the last 3 months)
 - 3183 emails sent in the past 3 months, 0 in the previous cycle

- u...@geode.apache.org:
 - 232 subscribers (up 12 in the last 3 months)
 - 286 emails sent in the past 3 months, 202 in the previous cycle

## JIRA activity:

- 513 JIRA tickets created in the last 3 months
- 351 JIRA tickets closed/resolved in the last 3 months


[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3268: Refactoring RegionCommands

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

`RegionCommands` was a big class that contained too many commands (each 
class should only contain one command). These commands have been split into 
their own individual command classes.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/693.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 #693


commit a2f58ca7986b68345b637fd30cc6d89b845fad8e
Author: YehEmily 
Date:   2017-08-07T20:58:08Z

GEODE-3268: Refactoring RegionCommands




---
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-native pull request #114: GEODE-2729: Remove global variables

2017-08-07 Thread dgkimura
GitHub user dgkimura opened a pull request:

https://github.com/apache/geode-native/pull/114

GEODE-2729: Remove global variables

- Remove CacheImpl::getInstance calls
- Converted PdxTypeRegistry clear to non-static method
- Adding Type Registry to cpp code as a public object.
- Convert static PoolManager method calls into instance method calls
- Remove PoolFactory/PoolManager global connectionPool singleton
- Allows multiple PoolFactories per Cache
- Remove default pool logic from Cache into PoolManager
- Add mutli-cache integration test

All c++ and .NET integration tests passed.  All unittests passed.

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

$ git pull https://github.com/dgkimura/geode-native feature/GEODE-2740

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

https://github.com/apache/geode-native/pull/114.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 #114






---
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 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-07 Thread Jinmei Liao


> On Aug. 7, 2017, 8:21 p.m., Jared Stewart wrote:
> > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
> > Lines 140 (patched)
> > 
> >
> > Can you clarify something for me?  The change looks like it's addding a 
> > check for CLUSTER:MANAGE:QUERY.  But from reading GEODE-3330, I thought we 
> > already checked for that and instead needed to add a check for 
> > DATA:READ:REGION.  But it looks like that might have already been happening 
> > on line 126 (122 of the original code).

line 126 is the authorization for old AccessControl interface, it does nothing 
for the new integrated security. it was there in case user is still using the 
old security model.

I was orignally confused as well. Turns out we are checking 
DATA:READ:regionName already for both execute and executeWithInitialResult, we 
are just adding the CLUSTER:MANAGE.QUERY check.


- Jinmei


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


On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 61472: GEODE-3097: fix an accidental bug introduced when working on ssl over http

2017-08-07 Thread Jared Stewart

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


Ship it!




Ship It!

- Jared Stewart


On Aug. 7, 2017, 5:18 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61472/
> ---
> 
> (Updated Aug. 7, 2017, 5:18 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3097: fix an accidental bug introduced when working on ssl over http
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
>  be556a447d862144e453f69809a2de67ee00b654 
> 
> 
> Diff: https://reviews.apache.org/r/61472/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



[GitHub] geode pull request #692: GEODE-3264: Refactoring MemberCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3264: Refactoring MemberCommands

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

`MemberCommands` was a big class that contained too many commands (each 
class should only contain one command). These commands have been split into 
their own individual command classes.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/692.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 #692


commit ca90a20aafa0277e049ad07cc93601b00834b3dd
Author: YehEmily 
Date:   2017-08-07T20:09:42Z

GEODE-3264: Refactoring MemberCommands




---
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 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-07 Thread Jinmei Liao

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

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


Repository: geode


Description
---

* cq.execute() and cq.executeWithInitialResult() all would still require 
DATA:READ because it will send the result back to the client either initially 
or later.


Diffs
-

  geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
5d5c2148e2eba8054df305942e04f43ea69c0a79 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 a455aff315cd26abd398630ff63d8b54b0d1d12b 
  
geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
 6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
  
geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
 77a608c7e65a2030c0037e9f327cf8c17e9313db 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
89167258ddbc06315068c849255a8530faefe060 
  
geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java 
45f45abd17cf4f90f96163ebe4f01e67b3b53633 
  geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
PRE-CREATION 


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


Testing
---

precheckin running


Thanks,

Jinmei Liao



[GitHub] geode pull request #691: GEODE-3260: Refactoring FunctionCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3260: Refactoring FunctionCommands

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

`FunctionCommands` was a huge class that contained a large number of 
commands. These commands have been split into their own individual command 
classes.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/691.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 #691


commit 0265b29cc104e0b166dc5972bf739f042276cb5b
Author: YehEmily 
Date:   2017-08-07T19:56:17Z

GEODE-3260: Refactoring FunctionCommands




---
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 #690: GEODE-3262: Refactoring IndexCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3262: Refactoring IndexCommands

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

`IndexCommands` was a huge class that contained a large number of commands. 
These commands have been split into their own individual command classes.

`IndexCommandsDUnitTest` was also refactored to `ListIndexCommandDUnitTest` 
because it was only testing `ListIndexCommand`... In the future, more tests 
should be written to cover the other commands that were in `IndexCommands`.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/690.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 #690


commit 34a2cc1f0b18309078c26abfd3bbf12d6c26423f
Author: YehEmily 
Date:   2017-08-07T19:35:14Z

GEODE-3262: Refactoring IndexCommands




---
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: continuous query internal mechanism

2017-08-07 Thread Anilkumar Gingade
CQ Processing on server side is same for all clients (Java, C++)...

The subscription events are sent to client as ClientUpdateMessage, which
holds information about registered events and CQ events. The client process
this and updates/invokes the client side cache/listeners with respective
event. Look into ClientUpdateMessageImpl and CacheClientUpdater (for client
side processing).

-Anil.




On Mon, Aug 7, 2017 at 11:01 AM, Roi Apelker  wrote:

> Thanks,
>
> By the way, is there any difference in the behaviour of the server, if the
> client that registered the CQ is a native (C++) client?
>
> I have been going over the classes and code for some time and can't seem
> to find the actual location where a CQ update/notification is sent...
>
> It's like CqEventImpl class is never even generated in this scenario.
>
> If anyone can help here I would be most grateful :-)
>
> Thanks
>
> Roi
>
>
>
> -Original Message-
> From: Anilkumar Gingade [mailto:aging...@pivotal.io]
> Sent: Monday, August 07, 2017 8:23 PM
> To: dev@geode.apache.org
> Subject: Re: continuous query internal mechanism
>
> You can find those in CqServiceImpl.process*()...
>
> -Anil.
>
>
> On Mon, Aug 7, 2017 at 9:14 AM, Roi Apelker 
> wrote:
>
> > Hello,
> >
> > I am trying to look into the code of the continuous query mechanism -
> > where the GEODE server sends the notification back to the client.
> >
> > Can anyone point me to the central classes of continuous query,
> > especially to the one that is responsible for the calculation of the
> > new data and packing it as a message back to the client?
> >
> > Thanks,
> >
> > Roi
> >
> > 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>
> >
> 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 #689: GEODE-3259: Refactoring DurableClientCommands

2017-08-07 Thread YehEmily
GitHub user YehEmily opened a pull request:

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

GEODE-3259: Refactoring DurableClientCommands

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

`DurableClientCommands` was a huge class that contained a large number of 
commands. These commands have been split into their own individual command 
classes. The methods shared by these commands all appeared to be used for 
building results, so they were moved to a new class named 
`DurableClientCommandsResultBuilder`.

**TESTING STATUS: Precheckin in progress**

- [x] JIRA ticket referenced

- [x] PR rebased

- [x] Initial commit is single and squashed

- [x] `gradlew build` runs cleanly

- [ ] Unit tests will be updated with 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

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

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

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

https://github.com/apache/geode/pull/689.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 #689






---
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 #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread pdxrunner
Github user pdxrunner commented on the issue:

https://github.com/apache/geode/pull/687
  
I'll go ahead and review your command refactorings in this PR, keeping in 
mind that the test will be dealt with under the other JIRA. I agree with not 
changing those two DUnit tests at this time. 


---
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: continuous query internal mechanism

2017-08-07 Thread Roi Apelker
Thanks,

By the way, is there any difference in the behaviour of the server, if the 
client that registered the CQ is a native (C++) client?

I have been going over the classes and code for some time and can't seem to 
find the actual location where a CQ update/notification is sent...

It's like CqEventImpl class is never even generated in this scenario.

If anyone can help here I would be most grateful :-)

Thanks

Roi



-Original Message-
From: Anilkumar Gingade [mailto:aging...@pivotal.io] 
Sent: Monday, August 07, 2017 8:23 PM
To: dev@geode.apache.org
Subject: Re: continuous query internal mechanism

You can find those in CqServiceImpl.process*()...

-Anil.


On Mon, Aug 7, 2017 at 9:14 AM, Roi Apelker  wrote:

> Hello,
>
> I am trying to look into the code of the continuous query mechanism - 
> where the GEODE server sends the notification back to the client.
>
> Can anyone point me to the central classes of continuous query, 
> especially to the one that is responsible for the calculation of the 
> new data and packing it as a message back to the client?
>
> Thanks,
>
> Roi
>
> 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>
>
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 



Build failed in Jenkins: Geode-nightly #916

2017-08-07 Thread Apache Jenkins Server
See 


Changes:

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

[hkhamesra] GEODE-3307: CI failure: Uncaught exception in Membership View 
Creator

[ukohlmeyer] GEODE-3321: Adding ErrorCode values to protobuf protocol. This now

[nnag] GEODE-3387: Cleaned up and minor refactoring of Lucene module

[dbarnes] GEODE-3396 Provide pub-tools support for product name & version

[jiliao] GEODE-3328: adding ssl-truststore-type to the config

--
[...truncated 596.66 KB...]

:geode-cq:processTestResources
: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

Request to blacklist H2 on Apache Nightly Builds

2017-08-07 Thread Nabarun Nag
Hi

We need to blacklist H2 machine from running the Apache Nightly builds for
Apache Geode project.
Please can someone with the necessary permission do it.

Regards
Nabarun Nag


[GitHub] geode pull request #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...

2017-08-07 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: continuous query internal mechanism

2017-08-07 Thread Anilkumar Gingade
You can find those in CqServiceImpl.process*()...

-Anil.


On Mon, Aug 7, 2017 at 9:14 AM, Roi Apelker  wrote:

> Hello,
>
> I am trying to look into the code of the continuous query mechanism -
> where the GEODE server sends the notification back to the client.
>
> Can anyone point me to the central classes of continuous query, especially
> to the one that is responsible for the calculation of the new data and
> packing it as a message back to the client?
>
> Thanks,
>
> Roi
>
> 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>
>


Review Request 61472: GEODE-3097: fix an accidental bug introduced when working on ssl over http

2017-08-07 Thread Jinmei Liao

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

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


Repository: geode


Description
---

GEODE-3097: fix an accidental bug introduced when working on ssl over http


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 


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


Testing
---

precheckin running


Thanks,

Jinmei Liao



[GitHub] geode issue #687: GEODE-3258: Refactoring DiskStoreCommands

2017-08-07 Thread YehEmily
Github user YehEmily commented on the issue:

https://github.com/apache/geode/pull/687
  
@pdxrunner The tests that cover these commands are part of a separate JIRA 
ticket ([GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359)), so 
@jinmeiliao recommended just refactoring the commands and leaving the tests to 
be refactored in the future. I think I will try to add tests to cover the rest 
of the commands to `DiskStoreCommandsJUnitTest`, since there are very few 
commands covered by that test, but I think I'll leave 
`DiskStoreCommandsDUnitTest` and `ListAndDescribeDiskStoreCommandsDUnitTest ` 
alone for now. What do you think?


---
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 #685: GEODE-3261: Refactoring GfshHelpCommands

2017-08-07 Thread YehEmily
Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/685#discussion_r131705221
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
 ---
@@ -34,7 +36,7 @@
   public static void beforeClass() {
 helper = new Helper();
 // use GfshHelpCommand for testing
-Method[] methods = GfshHelpCommands.class.getMethods();
+Method[] methods = GfshHelpCommand.class.getMethods();
--- End diff --

Good idea - thank you for your feedback! I don't think the original test 
ever actually tested the hint command, so I wrote two completely new tests: 
`HelperIntegrationTest.TestHintWithInput` and 
`HelperIntegrationTest.TestHintWithoutInput`. PR has also been updated!


---
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: Need to know complete execution time for geode-core Junit tests

2017-08-07 Thread Darrel Schneider
For test004StartServerFailsFastOnMissingGemFirePropertiesFile I think this
test just needs to be improved for Windows.
The test has an assertion that the failure message contains a certain
string. On Windows the message contains "C:" and "\" which the test does
not expect. Instead of the test treating "cacheXmlPathName" as a constant I
think it should create an instance of File using cacheXmlPathName and then
make sure the message contains the result of calling getAbsolutePath on
that File.


On Sun, Aug 6, 2017 at 11:54 PM, Dinesh Akhand  wrote:

> Hi Team,
>
> When we are running test on windows : we are getting below error
> We ran : gradlew precheckin
>
>
> rg.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest > 
> test003StartServerFailsFastOnMissingCacheXmlFile
> FAILED
> java.lang.AssertionError:
> Warning: The Geode cache XML file C:\path\to\missing\cache.xml could
> not be found.
>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.assertTrue(Assert.java:41)
> at org.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest.test003StartServerFailsFastOnM
> issingCacheXmlFile(LauncherLifecycleCommandsDUnitTest.java:502)
>
> org.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest > 
> test004StartServerFailsFastOnMissingGemFirePropertiesFile
> FAILED
> java.lang.AssertionError:
> Warning: The Geode properties file C:\path\to\missing\gemfire.properties
> could not be found.
>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.assertTrue(Assert.java:41)
> at
>
> Is there any path we need to set in windows to make it run.
>
> Thanks,
> Dinesh Akhand
>
>
>
>
>
>
>
>
>
>
>
> 
> ---
> Found suspect string in log4j at line 546
>
> -Original Message-
> From: Dan Smith [mailto:dsm...@pivotal.io]
> Sent: Tuesday, July 25, 2017 9:59 PM
> To: dev@geode.apache.org
> Subject: Re: Need to know complete execution time for geode-core Junit
> tests
>
> What target are you running? It's a bit confusing because there are
> actually 3 different sets of tests. These times a really rough because I
> don't have a good run in front of me, but this should give you an idea.
>
> "True" unit tests, runs in 1-2 minutes like Jens Said:
> ./gradlew geode-core:test
>
> Single VM integration tests - runs in maybe 1.5 hours:
> ./gradlew geode-core:integrationTest
>
> Multiple VM integration tests - runs in maybe 5 hours?
> .gradlew geode-core:distributedTest
>
> Run all of the tests
> ./gradlew geode-core:precheckin
>
> -Dan
>
> On Tue, Jul 25, 2017 at 8:34 AM, Anthony Baker  wrote:
>
> > You might want to get a few thread dumps to see if there is a hung test.
> > Also, make sure you have sufficient RAM.
> >
> > Anthony
> >
> > > On Jul 25, 2017, at 1:13 AM, Dinesh Akhand 
> wrote:
> > >
> > > Hi Team,
> > >
> > > I trigger the geode-core Junit tests . it keeping running from last
> > > 3
> > hour.
> > > Can you please let me know how much time it take completion of Junit
> > test in Geode-core.
> > > I am using geode 1.1.1
> > >
> > >
> > > Thanks,
> > > Dinesh Akhand
> > > 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>
> >
> >
> 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>
>


continuous query internal mechanism

2017-08-07 Thread Roi Apelker
Hello,

I am trying to look into the code of the continuous query mechanism - where the 
GEODE server sends the notification back to the client.

Can anyone point me to the central classes of continuous query, especially to 
the one that is responsible for the calculation of the new data and packing it 
as a message back to the client?

Thanks,

Roi

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 



Re: Review Request 61417: GEODE-3328: adding ssl-truststore-type to the config

2017-08-07 Thread Dave Barnes
Hi Jinmei,
1. Yes, a parallel change would be good for keystore. In both cases, we
need to specify the default. My sample text said empty string ('') -- is
that correct, or is the default JKS?
2 & 3: Good - glad they're correct. I wasn't sure, just noticed the
patterns.
-Dave

On Sat, Aug 5, 2017 at 12:15 PM, Jinmei Liao  wrote:

>
>
> > On Aug. 3, 2017, 11:36 p.m., Dave Barnes wrote:
> > > 1. The help message for the property should state what it does and
> whether it has a default. JKS was our assumption in the past, but with the
> introduction of this feature other values (such as 'pkcs12') are possible.
> > > Current wording: "For Java truststore file format, this property has
> the value jks (or JKS)."
> > > Suggested wording: "Specifies the type of the ssl truststore. The
> default is '' (an empty string) For Java truststore format, specify 'jks'
> or 'JKS'."
> > >
> > > The following questions come from me reading for patterns, please
> forgive if they're stupid ones:
> > >
> > > 2. In configureLegacyClusterSSL (and ..ServerSSL and ..JMXSSL and
> ..ServiceSSL), I see two occurrences of "sslConfig.setTruststoreType(
> getDistributionConfig().getClusterSSLKeyStoreType());". Should the second
> one in each case be getClusterSSLTrustStoreType() ?
> > >
> > > 3. In DistributionConfigImpl.java, there's a long list of static
> imports for gateway, http, jmx, etc. KEYSTORE_TYPE is included for each
> group, but not TRUSTSTORE_TYPE. Is this correct?
>
> 1. I am simply copying from the ssl-keystore-type, should this suggested
> description also apply to the keystore as well?
> 2. the truststore type does not exist for legacy ssl specifications, so
> there is no getClusterSSLTrustStoreType() method. Here I am just assuming
> the keystore and truststore type is the same.
> 3. IDEA did that according to the latest import style.
>
>
> - Jinmei
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61417/#review182181
> ---
>
>
> On Aug. 3, 2017, 9:15 p.m., Jinmei Liao wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/61417/
> > ---
> >
> > (Updated Aug. 3, 2017, 9:15 p.m.)
> >
> >
> > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund,
> Patrick Rhomberg, and Udo Kohlmeyer.
> >
> >
> > Repository: geode
> >
> >
> > Description
> > ---
> >
> > GEODE-3328: adding ssl-truststore-type to the config
> >
> > this is what's requested in the JIRA ticket. This changeset just deals
> with adding the property into gemfire properties
> >
> >
> > Diffs
> > -
> >
> >   
> > geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
> 63f6505101f6edb62167b854d3d16d76d0a893cd
> >   geode-core/src/main/java/org/apache/geode/distributed/internal/
> AbstractDistributionConfig.java 795f6a5a4a4f42fe065c9ca6013fd5598f9311d8
> >   geode-core/src/main/java/org/apache/geode/distributed/
> internal/DistributionConfig.java c2a395de0bfe21ed8efeb6b25e331f7085b3bf4f
> >   
> > geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
> fbe894c96447b2e30594eb2ed0652dd08e1028ce
> >   
> > geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
> f86f07eb5e58b8509e909b7748795530efd65339
> >   
> > geode-core/src/main/java/org/apache/geode/management/GemFireProperties.java
> 08fa9b54ea066b4158478ae89d8295ed0b1a337b
> >   geode-core/src/main/java/org/apache/geode/management/
> internal/beans/BeanUtilFuncs.java 499ef010f354ebf88765190f1b5eb945da83accc
> >   geode-core/src/test/java/org/apache/geode/distributed/internal/
> DistributionConfigJUnitTest.java 525f988cd3cb4f19872168df9b7105645f5c0498
> >
> >
> > Diff: https://reviews.apache.org/r/61417/diff/1/
> >
> >
> > Testing
> > ---
> >
> > precheckin running
> >
> >
> > Thanks,
> >
> > Jinmei Liao
> >
> >
>
>


Re: Volunteer Request: Write Draft Board Report For August 2017

2017-08-07 Thread Dan Smith
Hi Mark,

I can take this on. I'll try to get a draft out today.

-Dan

On Fri, Aug 4, 2017 at 12:06 PM, Mark Bretl  wrote:

> Hi Everyone,
>
> We are scheduled to report to the Apache board for this months board
> meeting on August 16 and need to submit a report by Wednesday, August 9th.
> Goal would be to have a initial draft for the community by Tuesday.
>
> Template:
> https://cwiki.apache.org/confluence/display/GEODE/ASF+
> Board+Report+Template
>
> Previous Reports: https://whimsy.apache.org/board/minutes/Geode.html
>
> Would anyone like to volunteer to write a draft of the report?
>
> Best regards,
>
> --Mark
>


RE: Need to know complete execution time for geode-core Junit tests

2017-08-07 Thread Dinesh Akhand
Hi Team,

When we run :
./gradlew integrationTest
./gradlew distributedTest 

Many test are getting failed with head allocation error or configuration error.
Can you please suggest , what extra configuration we need to do .

I am able to run Junit correctly.
unit tests - run with ./gradlew test  // this works properly.

I have i5 , 8GB laptop RAM. 

Thanks,
Dinesh Akhand

-Original Message-
From: Dinesh Akhand 
Sent: Monday, August 7, 2017 12:24 PM
To: dev@geode.apache.org
Subject: RE: Need to know complete execution time for geode-core Junit tests

Hi Team,

When we are running test on windows : we are getting below error We ran : 
gradlew precheckin


rg.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest
 > test003StartServerFailsFastOnMissingCacheXmlFile FAILED
java.lang.AssertionError:
Warning: The Geode cache XML file C:\path\to\missing\cache.xml could not be 
found.

at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at 
org.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest.test003StartServerFailsFastOnMissingCacheXmlFile(LauncherLifecycleCommandsDUnitTest.java:502)

org.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest
 > test004StartServerFailsFastOnMissingGemFirePropertiesFile FAILED
java.lang.AssertionError:
Warning: The Geode properties file C:\path\to\missing\gemfire.properties 
could not be found.

at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at 

Is there any path we need to set in windows to make it run.

Thanks,
Dinesh Akhand











---
Found suspect string in log4j at line 546

-Original Message-
From: Dan Smith [mailto:dsm...@pivotal.io] 
Sent: Tuesday, July 25, 2017 9:59 PM
To: dev@geode.apache.org
Subject: Re: Need to know complete execution time for geode-core Junit tests

What target are you running? It's a bit confusing because there are actually 3 
different sets of tests. These times a really rough because I don't have a good 
run in front of me, but this should give you an idea.

"True" unit tests, runs in 1-2 minutes like Jens Said:
./gradlew geode-core:test

Single VM integration tests - runs in maybe 1.5 hours:
./gradlew geode-core:integrationTest

Multiple VM integration tests - runs in maybe 5 hours?
.gradlew geode-core:distributedTest

Run all of the tests
./gradlew geode-core:precheckin

-Dan

On Tue, Jul 25, 2017 at 8:34 AM, Anthony Baker  wrote:

> You might want to get a few thread dumps to see if there is a hung test.
> Also, make sure you have sufficient RAM.
>
> Anthony
>
> > On Jul 25, 2017, at 1:13 AM, Dinesh Akhand  wrote:
> >
> > Hi Team,
> >
> > I trigger the geode-core Junit tests . it keeping running from last 
> > 3
> hour.
> > Can you please let me know how much time it take completion of Junit
> test in Geode-core.
> > I am using geode 1.1.1
> >
> >
> > Thanks,
> > Dinesh Akhand
> > 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>
>
>
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 

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 



RE: Need to know complete execution time for geode-core Junit tests

2017-08-07 Thread Dinesh Akhand
Hi Team,

When we are running test on windows : we are getting below error 
We ran : gradlew precheckin


rg.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest
 > test003StartServerFailsFastOnMissingCacheXmlFile FAILED
java.lang.AssertionError:
Warning: The Geode cache XML file C:\path\to\missing\cache.xml could not be 
found.

at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at 
org.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest.test003StartServerFailsFastOnMissingCacheXmlFile(LauncherLifecycleCommandsDUnitTest.java:502)

org.apache.geode.management.internal.cli.commands.LauncherLifecycleCommandsDUnitTest
 > test004StartServerFailsFastOnMissingGemFirePropertiesFile FAILED
java.lang.AssertionError:
Warning: The Geode properties file C:\path\to\missing\gemfire.properties 
could not be found.

at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at 

Is there any path we need to set in windows to make it run.

Thanks,
Dinesh Akhand











---
Found suspect string in log4j at line 546

-Original Message-
From: Dan Smith [mailto:dsm...@pivotal.io] 
Sent: Tuesday, July 25, 2017 9:59 PM
To: dev@geode.apache.org
Subject: Re: Need to know complete execution time for geode-core Junit tests

What target are you running? It's a bit confusing because there are actually 3 
different sets of tests. These times a really rough because I don't have a good 
run in front of me, but this should give you an idea.

"True" unit tests, runs in 1-2 minutes like Jens Said:
./gradlew geode-core:test

Single VM integration tests - runs in maybe 1.5 hours:
./gradlew geode-core:integrationTest

Multiple VM integration tests - runs in maybe 5 hours?
.gradlew geode-core:distributedTest

Run all of the tests
./gradlew geode-core:precheckin

-Dan

On Tue, Jul 25, 2017 at 8:34 AM, Anthony Baker  wrote:

> You might want to get a few thread dumps to see if there is a hung test.
> Also, make sure you have sufficient RAM.
>
> Anthony
>
> > On Jul 25, 2017, at 1:13 AM, Dinesh Akhand  wrote:
> >
> > Hi Team,
> >
> > I trigger the geode-core Junit tests . it keeping running from last 
> > 3
> hour.
> > Can you please let me know how much time it take completion of Junit
> test in Geode-core.
> > I am using geode 1.1.1
> >
> >
> > Thanks,
> > Dinesh Akhand
> > 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>
>
>
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