[GitHub] geode pull request #709: GEODE-3269: Refactoring ShellCommands
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/709 GEODE-3269: Refactoring ShellCommands [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3269) - [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? - [ ] Have you written or updated unit tests to verify your changes? (See [GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359)) **Testing progress: Precheckin in progress! See eyeh-geode-3269 because I probably won't be able to** You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3269 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/709.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 #709 commit 8a06b768b6e928ab3df6ec1ec4c6460141992bec Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-11T23:17:42Z GEODE-3269: Refactoring ShellCommands --- 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 #672: GEODE-3256: Refactoring DataCommands
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/672 Already merged to develop. --- 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
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/672 --- 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 #701: GEODE-3337: Refactoring LauncherLifecycleCommandsDU...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/701 GEODE-3337: Refactoring LauncherLifecycleCommandsDUnitTest [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3337) `LauncherLifecycleCommandsDUnitTest` was a huge test class that tested multiple commands, in addition to using the deprecated `CliCommandsTestBase`. These tests have been refactored to use more up-to-date test rules and also separated into test classes based on the command that they test. Some tests in the original `LauncherLifecycleCommandsDUnitTest` seemed to be unnecessary or redundant (for example, `StatusLocatorRealGfshTest` already tested some of the tests in `LauncherLifecycleCommandsDUnitTest`) so these were deleted. *Testing Status: Precheckin in progress* - [ ] JIRA ticket - [ ] Rebased - [ ] Single commit - [ ] `gradlew build` runs cleanly - [ ] Tests updated You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3337 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/701.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 #701 commit c4cdc531c41502d6a162715e324380b66d95794e Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-09T17:45:43Z GEODE-3337: Refactoring LauncherLifecycleCommandsDUnitTest --- 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
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
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
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<DistributedMember, PersistentID> 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
H
[GitHub] geode pull request #696: GEODE-3265: Refactoring MiscellaneousCommands
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 <emilyyeh1...@gmail.com> 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 #695: GEODE-3267: Refactoring QueueCommands
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 <emilyyeh1...@gmail.com> 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
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 <emilyyeh1...@gmail.com> 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. ---
[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands
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 <emilyyeh1...@gmail.com> 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 pull request #692: GEODE-3264: Refactoring MemberCommands
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 <emilyyeh1...@gmail.com> 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. ---
[GitHub] geode pull request #691: GEODE-3260: Refactoring FunctionCommands
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 <emilyyeh1...@gmail.com> 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
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 <emilyyeh1...@gmail.com> 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. ---
[GitHub] geode pull request #689: GEODE-3259: Refactoring DurableClientCommands
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
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
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.
---
[GitHub] geode pull request #687: GEODE-3258: Refactoring DiskStoreCommands
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/687 --- 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
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/687 GEODE-3258: Refactoring DiskStoreCommands [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3258) `DiskStoreCommands` was a huge class that contained a large number of commands. These commands have been split into their own individual command classes. Two methods shared by several commands were moved into `DiskStoreCommandsUtils` (it has been mentioned before that --Utils classes should be avoided, but because a better home for these methods could not be found, this issue may be resolved later in the future). Some tests were also updated - `DiskStoreCommandsJUnitTest` and `ListAndDescribeDiskStoreCommandsDUnitTest`. It really looks like the two tests share many of the same functionalities... I think that `ListAndDescribeDiskStoreCommandsDUnitTest` could be removed, since `DiskStoreCommandsJUnitTest` already seems to test `ListDiskStoreCommand` and `DescribeDiskStoreCommand`, and `DiskStoreCommandsJUnitTest` should be updated to test more commands, since it only seems to be testing `list disk-store` and `describe disk-store`. **TESTING STATUS: Precheckin in progress** - [ ] JIRA ticket referened - [ ] PR rebased - [ ] Initial commit is single and squashed - [ ] `gradlew build` runs cleanly - [ ] Unit tests updated You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3258 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/687.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 #687 commit 55bd2b1613ea02a895b8f38b9760d14c5ffa531e Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-03T16:00:08Z GEODE-3258: Refactoring DiskStoreCommands --- 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 #684: GEODE-3266: Refactoring PDXCommands
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/684 GEODE-3266: Refactoring PDXCommands [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3266) - [x] JIRA ticket referenced - [x] PR rebased - [x] Single, squashed commit - [x] `gradlew build` runs cleanly - [ ] Associated tests to 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-3266 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/684.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 #684 commit de75047552a794332f10773c4d9bfdb4f3015575 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-04T17:47:48Z GEODE-3266: Refactoring PDXCommands --- 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 #680: GEODE-3257: Refactoring DeployCommands
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/680 GEODE-3257: Refactoring DeployCommands [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3257) **Testing Status: Precheckin to be run on morning of 8/3** - [x] JIRA ticket - [x] PR rebased - [x] Commit single & squashed - [x] `gradlew build` runs cleanly - [ ] Tests to 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-3257 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/680.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 #680 commit db50d331c54ec4c8ed7f583685c78002c91c7bde Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-03T00:28:10Z GEODE-3257: Refactoring DeployCommands --- 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 #679: GEODE-3340: Refactor ConfigCommandsDUnitTest to use...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/679#discussion_r131003469
--- Diff:
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java
---
@@ -0,0 +1,242 @@
+/*
+ * 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.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
+import static
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static
org.apache.geode.distributed.ConfigurationProperties.STATISTIC_SAMPLING_ENABLED;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.logging.LogWriterImpl;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+
+@Category(DistributedTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class ConfigCommandDUnitTest {
--- End diff --
I think this move made it possible to run this test over HTTP without
needing to run it through `CommandOverHttpDUnitTest`, since we are trying to
get rid of `CommandOverHttpDUnitTest` anyway.
---
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 #679: GEODE-3340: Refactor ConfigCommandsDUnitTest to use...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/679#discussion_r131003075
--- Diff:
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java
---
@@ -0,0 +1,242 @@
+/*
+ * 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.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS;
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
+import static
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static
org.apache.geode.distributed.ConfigurationProperties.STATISTIC_SAMPLING_ENABLED;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.logging.LogWriterImpl;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+
+@Category(DistributedTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class ConfigCommandDUnitTest {
+ @Rule
+ public LocatorServerStartupRule startupRule = new
LocatorServerStartupRule();
+
+ @Rule
+ public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ @Test
+ @Parameters({"true", "false"})
+ public void testDescribeConfig(final boolean connectOverHttp) throws
Exception {
+Properties localProps = new Properties();
+localProps.setProperty(STATISTIC_SAMPLING_ENABLED, "true");
+localProps.setProperty(ENABLE_TIME_STATISTICS, "true");
+localProps.setProperty(GROUPS, "G1");
+MemberVM server0 = startupRule.startServerAsJmxManager(0, localProps);
+
+if (connectOverHttp) {
+ gfsh.connectAndVerify(server0.getHttpPort(),
GfshShellConnectionRule.PortType.http);
+} else {
+ gfsh.connectAndVerify(server0.getJmxPort(),
GfshShellConnectionRule.PortType.jmxManger);
+}
+
+server0.invoke(() -> {
+ InternalCache cache =
LocatorServerStartupRule.serverStarter.getCache();
+ InternalDistributedSystem system =
cache.getInternalDistributedSystem();
+ DistributionConfig config = system.getConfig();
+ config.setArchiveFileSizeLimit(1000);
+});
+
+gfsh.executeAndVerifyCommand("describe config --member=" +
server0.getName());
+String result = gfsh.getGfshOutput();
+
+assertThat(result).contains("enable-time-statistics
: true");
--- End diff --
Fixed! Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
[GitHub] geode pull request #671: GEODE-3255: Refactor CreateAlterDestroyRegionComman...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/671#discussion_r131002611
--- 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 --
Fixed! Thanks for your feedback! Do you mean it would be nice to have a
test that would fail if `regionExists` always returns `true`, regardless of
whether the region does or doesn't exist? I wrote a very simple test that does
this and put it in `CreateRegionCommandTest`.
---
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...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/665#discussion_r130965197
--- 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 --
Oops - it seems that it's actually being used, but only in
`ExportConfigCommand`, so I moved it there. Thanks for pointing this out!
---
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...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/665#discussion_r130960231
--- 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 --
Ahh, not sure how I didn't notice that! I've deleted it - thanks for your
feedback!
---
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
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/651#discussion_r130959623
--- 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 --
Fixed - thanks for your feedback!
---
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
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/651#discussion_r130959557
--- 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 --
Sure! Almost none of the `--- manager` CliStrings were used, so I deleted
them. Only `start manager` was ever used, and only in this place, so I figured
it would be safe to get rid of it. Do you think this might break any backwards
compatibility / should I roll back the 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
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/651#discussion_r130958981
--- 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 --
You're right... I was trying to find a place for the
`JAVA_HOME_NOT_FOUND_ERROR_MESSAGE` variable, but it looks like it would change
some behavior if inserted here. I'll just roll back the changes on this file
and delete `JAVA_HOME_NOT_FOUND_ERROR_MESSAGE`. Thanks for your feedback!
---
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...
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 <emilyyeh1...@gmail.com> 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
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/672 GEODE-3256: Refactoring DataCommands [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3256) `DataCommands.java` was a large class that contained multiple commands. Each command was refactored into a separate class, and the methods shared by the commands were refactored into a new and appropriately named class of their own (`DataCommandsUtils`). **Testing Done: Precheckin to be run on morning of 8/1** - [x] JIRA ticket - [x] PR rebased - [x] Single, squashed commit - [x] Does `gradlew build` runs cleanly - [ ] No unit tests updated with this change (yet) - some related tests could stand to be refactored (especially `GemfireDataCommandsDUnitTest`), but I think I will leave these for [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-3256 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/672.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 #672 commit 017e39eb9cfe27c010c4c31f248c3fcdeab391f9 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-31T23:45:19Z GEODE-3256: Refactoring DataCommands --- 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 #671: GEODE-3255: Refactor CreateAlterDestroyRegionComman...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/671 GEODE-3255: Refactor CreateAlterDestroyRegionCommands and tests [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3255) `CreateAlterDestroyRegionCommands` was a large class that contained multiple commands. Each command was refactored into a separate class, and the methods shared by the commands were refactored into a new and appropriately named class of their own. Tests related to `CreateAlterDestroyRegionCommands` were also refactored. **Testing status: Precheckin completed successfully** - [x] JIRA ticket referenced - [x] PR rebased - [x] Single, squashed commit - [x] `gradlew build` runs cleanly - [x] Tests updated You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3255 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/671.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 #671 commit 8a0e27fd3bf44cb63871b8b3b226800a845e0e4c Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-28T21:23:25Z GEODE-3255: Refactor CreateAlterDestroyRegionCommands and tests --- 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 #670: GEODE-3336: Refactor IndexCommandsDUnitTest to use ...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/670 GEODE-3336: Refactor IndexCommandsDUnitTest to use test rules [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3336) 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? ### 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/YehEmily/geode GEODE-1359 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/670.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 #670 commit 514018b97c5e5ffab8efbc1e7c119d8bcca64dc1 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-26T23:53:27Z GEODE-1359: Refactor IndexCommandsDUnitTest to use test rules and allow for removal of CliCommandTestBase --- 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...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/665 GEODE-3254: Refactoring ConfigCommands and ConfigCommandsDUnitTest [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3254) `ConfigCommands.java` was a large class that contained multiple commands. Each command was refactored into a separate class. `ConfigCommandsDUnitTest.java` was also split into separate test classes. - [x] JIRA ticket referenced - [x] PR rebased - [x] Single squashed commit - [x] Builds cleanly - [x] Tests updated You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3254 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/665.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 #665 commit 11c88b725eee219e5ec90e546d6e6fa1709a1b16 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-26T18:07:09Z GEODE-3254: Refactoring ConfigCommands and ConfigCommandsDUnitTest --- 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r129695770
--- Diff:
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
---
@@ -173,36 +157,54 @@ public void testUnsupportedOrderByForPR() throws
Exception {
@Test
public void testSupportedOrderByForRR() throws Exception {
-
String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status,
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status,
p.ID"};
Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
Position.resetCounter();
-// Create Regions
+// Create Regions
Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
for (int i = 0; i < 50; i++) {
r1.put(new Portfolio(i), new Portfolio(i));
}
for (int i = 0; i < unsupportedQueries.length; i++) {
- Query q = null;
-
+ Query q;
CacheUtils.getLogger().info("Executing query: " +
unsupportedQueries[i]);
q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
try {
r[i][0] = q.execute();
-
} catch (QueryInvalidException qe) {
qe.printStackTrace();
fail(qe.toString());
}
}
}
+ // The following tests cover edge cases in OrderByComparator.
+ @Test
+ public void testCompareTwoNulls() throws Exception {
+assert (createComparator().compare(null, null) == 0);
--- End diff --
Thanks for catching this! I've fixed it and updated 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r129399451
--- Diff:
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
---
@@ -173,36 +157,58 @@ public void testUnsupportedOrderByForPR() throws
Exception {
@Test
public void testSupportedOrderByForRR() throws Exception {
-
String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status,
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status,
p.ID"};
Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
Position.resetCounter();
-// Create Regions
+// Create Regions
Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
for (int i = 0; i < 50; i++) {
r1.put(new Portfolio(i), new Portfolio(i));
}
for (int i = 0; i < unsupportedQueries.length; i++) {
- Query q = null;
-
+ Query q;
CacheUtils.getLogger().info("Executing query: " +
unsupportedQueries[i]);
q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
try {
r[i][0] = q.execute();
-
} catch (QueryInvalidException qe) {
qe.printStackTrace();
fail(qe.toString());
}
}
}
+ /**
+ * Tests three cases that were not originally covered by the original
tests in this class. (To
+ * confirm that these tests cover all cases in OrderByComparator, run
them with coverage.)
+ */
+ @Test
+ public void testCompareMethodEdgeCases() throws Exception {
--- End diff --
Thanks for the feedback! I've updated 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r129399374
--- Diff:
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
---
@@ -173,36 +157,58 @@ public void testUnsupportedOrderByForPR() throws
Exception {
@Test
public void testSupportedOrderByForRR() throws Exception {
-
String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status,
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status,
p.ID"};
Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
Position.resetCounter();
-// Create Regions
+// Create Regions
Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
for (int i = 0; i < 50; i++) {
r1.put(new Portfolio(i), new Portfolio(i));
}
for (int i = 0; i < unsupportedQueries.length; i++) {
- Query q = null;
-
+ Query q;
CacheUtils.getLogger().info("Executing query: " +
unsupportedQueries[i]);
q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
try {
r[i][0] = q.execute();
-
} catch (QueryInvalidException qe) {
qe.printStackTrace();
fail(qe.toString());
}
}
}
+ /**
+ * Tests three cases that were not originally covered by the original
tests in this class. (To
--- End diff --
All good points - I've fixed this and updated 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 #651: GEODE-3230: Cleaning up unused (Cli)Strings
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/651 GEODE-3230: Cleaning up unused (Cli)Strings [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3230) `CliStrings.java` contained a lot of Strings that were not used anywhere in the code. In this PR, each of these seemingly unused Strings were examined to find potential uses of them in the code. Strings that did seem useful were inserted into the code as needed. Strings that didn't seem to be useful were deleted. Strings that don't seem to be used, but are used in the closed side, were preserved and tagged as such. (Note: An attempt was made to use all Strings that weren't currently being used in parts of the code that seemed appropriate, but most of these were removed because they broke tests.) **Testing Status**: **Precheckin running** ### For all changes: - [x] JIRA ticket - [x] PR rebased - [x] Initial commit is squashed and single - [x] `gradlew build` runs cleanly - [x] Tests updated to verify changes You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3230 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/651.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 #651 commit 06dcd340a33c83cab7b4700a3c56e8fe780d0964 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-21T21:29:55Z GEODE-3230: Cleaning up unused (Cli)Strings --- 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/580 @jhuynh1 Thanks for the suggestion! I updated the tests in `OrderByComparatorJUnitTest.java` to cover all the cases in `OrderByComparator.java` and updated 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 #650: GEODE-3253: Refactoring ClientCommands and related ...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/650 GEODE-3253: Refactoring ClientCommands and related tests [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3253) `ClientCommands.java` was a large class that contained multiple commands. In this PR, each command was refactored into a separate class. `ClientCommandsDUnitTest.java` was also refactored and split into `DescribeClientCommandDUnitTest.java` and `ListClientCommandDUnitTest.java`. Methods shared by these two tests were moved into a new class, `ClientCommandsTestUtils.java`. - [x] JIRA Ticket referenced - [x] PR rebased against develop - [x] Initial contribution is a single squashed commit - [x] `gradlew build` runs cleanly - [x] Tests updated to verify changes **Testing status**: **Precheckin currently running** You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3253 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/650.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 #650 commit bf623e78ed44621f3efd6e5884f3d97d0cac31ee Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-20T23:54:22Z GEODE-3253: Refactoring ClientCommands and related tests --- 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 #647: GEODE-3271: Refactor WanCommands
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/647 GEODE-3271: Refactor WanCommands [See the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3271) `WanCommands.java` was a large class that contained multiple commands. Each command has been refactored into a separate class, and the methods shared by these commands have been refactored into a new and appropriately named class of their own. Tests associated with `WanCommands` have been renamed and split up into tests with names that relate to the newly separated commands. - [x] There is a JIRA ticket associated with this PR that is referenced in the commit message - [x] This PR has been rebased against the latest commit within the target branch - [x] The initial contribution is a single, squashed commit - [x] `gradlew build` runs cleanly - [x] Unit tests have been updated to verify changes You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3271 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/647.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 #647 commit 6dd031d13473d500181342ba35f203caf1b8edbc Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-19T20:16:03Z GEODE-3271: Separating WanCommands into multiple command classes commit f9a7242f3a4250ad157cfba5a20ff5e9b8b4a688 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-19T20:16:03Z GEODE-3271: Separating WanCommands into multiple command classes commit 0728ccebb4538b7934bf73d7c9ab4aca832ede6d Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-19T22:22:02Z Merge branch 'GEODE-3271' of github.com:YehEmily/geode into GEODE-3271 commit 4c9873e46150264c2fb92b64926c603130460c47 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-19T22:41:22Z Adding tests --- 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 #598: GEODE-2818: Completing implementation/testing of al...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/598#discussion_r128363171 --- Diff: geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/OptionAliasesIntegrationTest.java --- @@ -0,0 +1,259 @@ +/* + * 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; --- End diff -- I think you're right - this test does seem to do just the same things OptionAliasParserTest does. I'll get rid of this test and update the PR - thanks! --- 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 #598: GEODE-2818: Completing implementation/testing of al...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/598#discussion_r128362946 --- Diff: geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java --- @@ -26,6 +26,8 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import javax.mail.Folder; --- End diff -- Nope! I'll delete it and update the PR - thanks again! --- 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 #598: GEODE-2818: Completing implementation/testing of al...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/598#discussion_r128362904 --- Diff: geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java --- @@ -14,6 +14,8 @@ */ package org.apache.geode.management.internal.cli.commands; +import java.util.concurrent.TimeUnit; --- End diff -- Nope! I'll delete it and update the PR - thanks! --- 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 #642: GEODE-3031: Extracting startLocator and startServer...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/642 GEODE-3031: Extracting startLocator and startServer from LauncherLifecycleCommands [See the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3031?jql=project%20%3D%20GEODE%20AND%20text%20~%20%22launcherlifecyclecommands%22) `LauncherLifecycleCommands.java` was a huge class that contained commands for starting a server (`startServer`) and for starting a locator (`startLocator`). As a result, these gfsh commands were extracted. Then, the other methods associated with these two gfsh commands were refactored and moved to other classes. **Testing Progress:** Precheckin running - [x] JIRA ticket associated with this PR is referenced in the commit message - [x] PR has been rebased against the latest commit within the target branch - [x] Initial contribution is a single, squashed commit - [x] `gradlew build` runs cleanly - [x] Wrote/updated unit tests to verify changes You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-3031 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/642.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 #642 commit e37930172e46d36433ba2c479d04ed69a7a16902 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-17T21:35:50Z GEODE-3031: Extracting startLocator and startServer from LauncherLifecycleCommands, removing the entire LauncherLifecycleCommands class, and moving tests from LauncherLifecycleCommandsTest to GfshCommandTest, StartServerCommands, and StartLocatorCommands --- 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 #575: GEODE-3048: Editing tests requiring Gradle test run...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/575 --- 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 #549: GEODE-2203: gfsh status locator/server - Command no...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/549 --- 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 #606: GEODE-2601: Fixing startup configurations logging t...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/606 GEODE-2601: Fixing startup configurations logging twice [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-2601) The original fix (pull request #582) fixed the security banner logging twice, but not the startup configurations. This PR finishes the fix. - [ ] JIRA ticket referenced in the commit message - [ ] PR rebased against `develop` - [ ] Initial contribution is a single, squashed commit - [ ] `gradlew build` runs cleanly You can merge this pull request into a Git repository by running: $ git pull https://github.com/YehEmily/geode GEODE-2601-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/606.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 #606 commit aa22d3c1c9a51bda1a529121453b34045a5802c7 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-27T19:56:40Z GEODE-2601: Fixing banner and startup configurations logging twice --- 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 #600: GEODE-1958: Rolling back changes to decrypt method
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/600 GEODE-1958: Rolling back changes to decrypt method Earlier, I made changes to the `decrypt` method in `PasswordUtil` to allow it to decrypt any password, when it should only be decrypting passwords that are prefaced by `encrypt(`. This PR rolls back that change and fixes any issues that might have resulted. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-1958-fix-decrypt Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/600.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 #600 --- 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 #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/591#discussion_r123854670
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
---
@@ -925,79 +917,89 @@ private SelectResults
applyProjectionOnCollection(SelectResults resultSet,
private SelectResults prepareEmptyResultSet(ExecutionContext context,
boolean ignoreOrderBy)
throws TypeMismatchException, AmbiguousNameException {
-// if no projection attributes or '*'as projection attribute
-// & more than one/RunTimeIterator then create a StrcutSet.
-// If attribute is null or '*' & only one RuntimeIterator then create a
-// ResultSet.
-// If single attribute is present without alias name , then create
-// ResultSet
-// Else if more than on attribute or single attribute with alias is
-// present then return a StrcutSet
-// create StructSet which will contain root objects of all iterators in
-// from clause
+// If no projection attributes or '*' as projection attribute & more
than one/RunTimeIterator
+// then create a StructSet.
+// If attribute is null or '*' & only one RuntimeIterator then create
a ResultSet.
+// If single attribute is present without alias name, then create
ResultSet.
+// Else if more than on attribute or single attribute with alias is
present then return a
+// StructSet.
+// Create StructSet which will contain root objects of all iterators
in from clause.
ObjectType elementType = this.cachedElementTypeForOrderBy != null
? this.cachedElementTypeForOrderBy : prepareResultType(context);
SelectResults results;
-if (this.distinct || !this.count) {
- if (this.orderByAttrs != null) {
-boolean nullValuesAtStart = !((CompiledSortCriterion)
orderByAttrs.get(0)).getCriterion();
-if (elementType.isStructType()) {
- if (ignoreOrderBy) {
-results = this.distinct ? new LinkedStructSet((StructTypeImpl)
elementType)
-: new SortedResultsBag(elementType, nullValuesAtStart);
- } else {
-OrderByComparator comparator = this.hasUnmappedOrderByCols
-? new OrderByComparatorUnmapped(this.orderByAttrs,
(StructTypeImpl) elementType,
-context)
-: new OrderByComparator(this.orderByAttrs,
(StructTypeImpl) elementType, context);
-results = this.distinct ? new SortedStructSet(comparator,
(StructTypeImpl) elementType)
-: new SortedStructBag(comparator, (StructType)
elementType, nullValuesAtStart);
+if (!this.distinct && this.count) {
+ // Shobhit: If it's a 'COUNT' query and no End processing required
Like for 'DISTINCT'
+ // we can directly keep count in ResultSet and ResultBag is good
enough for that.
+ countStartQueryResult = 0;
+ return new ResultsBag(new ObjectTypeImpl(Integer.class), 1,
context.getCachePerfStats());
+}
- }
-} else {
- if (ignoreOrderBy) {
-results =
-this.distinct ? new LinkedResultSet() : new
SortedResultsBag(nullValuesAtStart);
+// Potential edge-case: Could this be non-null but empty?
+boolean nullValuesAtStart = orderByAttrs != null &&
!orderByAttrs.get(0).getCriterion();
+OrderByComparator comparator;
+switch (convertToStringCase(elementType, ignoreOrderBy)) {
+ case "UNORDERED DISTINCT STRUCT":
+return new StructSet((StructType) elementType);
+ case "UNORDERED DISTINCT RESULTS":
+return new ResultsSet(elementType);
+ case "UNORDERED INDISTINCT STRUCT":
+return new StructBag((StructType) elementType,
context.getCachePerfStats());
+ case "UNORDERED INDISTINCT RESULTS":
+return new ResultsBag(elementType, context.getCachePerfStats());
+
+ case "ORDERED DISTINCT STRUCT IGNORED":
+return new LinkedStructSet((StructTypeImpl) elementType);
+ case "ORDERED INDISTINCT STRUCT IGNORED":
+return new SortedResultsBag(elementType, nullValuesAtStart);
+ case "ORDERED DISTINCT STRUCT UNIGNORED":
--- End diff --
Good suggestion - thanks! @PurelyApplied and I worked together to get rid
of the Strings, and we're now using enums. You should be able to see these
changes starting at around line 920 of `CompiledSelect` of the latest commit.
We're using a naming convention that I think follows your suggestion (instead
of `UNORDERED_DISTINCT_STRUCT_IGNORED`, for example, we just use
`UNORDERED_DISTINCT_STRUCT`).
[GitHub] geode pull request #598: GEODE-2818: Completing implementation/testing of al...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/598 GEODE-2818: Completing implementation/testing of aliases [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-2818) Changes were previously implemented to allow users to use either `--group` or `--groups` and `--member` or `--members` and `--jar` or `--jars` when specifying parameters for certain commands. @sbawaska found that these changes weren't fully implemented for some commands, though, so this PR aims to fix this. Two tests were also implemented to cover all related changes: - `OptionAliasesIntegrationTest.java` uses `GfshRule` to show that these changes work in the environment (gfsh). Note that this test is not completed yet, as some commands were impractical to implement (since they required setting up certain permissions, etc.). The test could also be improved by not only running the commands, but also checking the contents of the logs written up by the `GfshRule`, although these logs aren't accessible yet (changes pending via @jaredjstewart). I'll probably file a JIRA ticket to keep track of these improvements to this test. - `OptionAliasesParsingTest.java` shows that all the commands that accept either `--group` or `--groups` and `--member` or `--members` and `--jar` or `--jars` are parsed correctly. Some imports were also reordered in the process of implementing changes and writing tests. **_Precheckin status: pending_** Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2818 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/598.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 #598 commit 5ccd1ac9ea3208540c52241ccae9e7f1d1fd09fe Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-20T19:43:07Z GEODE-2818: Making "groups" a valid option when starting locators or servers and adding a comprehensive test to cover changes. --- 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 #592: GEODE-2707: Removing TXLockToken
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/592 GEODE-2707: Removing TXLockToken [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-2707) It seems that there are no other classes that use the `TXLockToken` class, so it was safely removed. It's also an internal class, so it doesn't seem likely that the removal of this class will break any backwards compatibility. **_Precheckin status: In progress_** Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2707 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/592.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 #592 commit e51f2047136ea4cdaa09c22a3cc06290e7dd0a01 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-19T17:36:53Z GEODE-2707: Removing TXLockToken (it doesn't appear to be used anywhere, although I still need to check for backwards compatibility issues) --- 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 #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/591 GEODE-3073: Renamed OrderByComparatorUnmapped to OrderByComparatorMapped [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3073) `OrderByComparatorUnmapped.java` actually uses mapping, which is why it should really be called `OrderByComparatorMapped.java`. Some refactoring was also done (similar to the refactoring done in [pull request #580](https://github.com/apache/geode/pull/580), although to a lesser extent). Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-3073-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/591.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 #591 commit b915804d857fd3efd7fefe6af45a6a8c44745bed Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-16T20:41:21Z GEODE-3073: Renamed OrderByComparatorUnmapped to OrderByComparatorMapped and refactored the code a bit. --- 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 #590: GEODE-3090: Fixing gfsh help message (and a lot of ...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/590 GEODE-3090: Fixing gfsh help message (and a lot of other typos) [View the original JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-3090) The original ticket called for fixing a typo in `CliStrings.java`, and in the process, other typos were fixed. **_Precheckin status: in progress_** Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-3090 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/590.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 #590 commit b40874852c47c38b4b5c790f6eef6c1df53a04d1 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-16T22:49:53Z GEODE-3090: Fixing gfsh help message (and a lot of other typos) --- 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 #582: GEODE-2601: Fix banner being logged twice
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/582#discussion_r122476764
--- Diff:
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
---
@@ -2167,7 +2191,7 @@ private static void
notifyReconnectListeners(InternalDistributedSystem oldsys,
private void notifyResourceEventListeners(ResourceEvent event, Object
resource) {
for (Iterator iter =
resourceListeners.iterator(); iter.hasNext();) {
--- End diff --
How nice! Thank you, I'll edit this and 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 #578: GEODE-1958: Removing/deprecating PasswordUtil
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/578#discussion_r122279107
--- Diff:
geode-core/src/test/java/org/apache/geode/cache/util/PasswordUtilJUnitTest.java
---
@@ -1,42 +0,0 @@
-/*
- * 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.util;
-
-import static org.junit.Assert.*;
-
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-
-import org.apache.geode.internal.util.PasswordUtil;
-import org.apache.geode.test.junit.categories.SecurityTest;
-import org.apache.geode.test.junit.categories.UnitTest;
-
-@Category({UnitTest.class, SecurityTest.class})
-public class PasswordUtilJUnitTest {
-
- @Test
- public void testPasswordUtil() {
-String x = "password";
-String z = null;
-
-// System.out.println(x);
-String y = PasswordUtil.encrypt(x);
--- End diff --
Fixed - 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r122067285
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
---
@@ -228,4 +139,55 @@ void addEvaluatedSortCriteria(Object row,
ExecutionContext context)
// No op
}
+ private int compareHelperMethod(Object obj1, Object obj2) {
+if (obj1 == null || obj2 == null) {
+ return compareIfOneOrMoreNull(obj1, obj2);
+} else if (obj1 == QueryService.UNDEFINED || obj2 ==
QueryService.UNDEFINED) {
+ return compareIfOneOrMoreQueryServiceUndefined(obj1, obj2);
+} else {
+ return compareTwoObjects(obj1, obj2);
+}
+ }
+
+ private int compareIfOneOrMoreNull(Object obj1, Object obj2) {
+if (obj1 == null) {
+ return obj2 == null ? 0 : -1;
+} else {
+ return 1;
+}
+ }
+
+ private int compareIfOneOrMoreQueryServiceUndefined(Object obj1, Object
obj2) {
+if (obj1 == QueryService.UNDEFINED) {
+ return obj2 == QueryService.UNDEFINED ? 0 : -1;
+} else {
+ return 1;
+}
+ }
+
+ private int compareTwoObjects(Object obj1, Object obj2) {
+if (obj1 instanceof Number && obj2 instanceof Number) {
+ return compareTwoNumbers(obj1, obj2);
+} else {
+ return compareTwoStrings(obj1, obj2);
+}
+ }
+
+ private int compareTwoNumbers(Object obj1, Object obj2) {
--- End diff --
Oh no, that's very true! I'll fix this and 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 #580: GEODE-2936: Refactoring OrderByComparator
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/580#discussion_r122055572
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
---
@@ -45,78 +44,50 @@ public OrderByComparator(List
orderByAttrs, ObjectType ob
}
/**
- * Yogesh : This methods evaluates sort criteria and returns a ArrayList
of Object[] arrays of
- * evaluated criteria
+ * This method evaluates sort criteria and returns an ArrayList of
Object[] arrays of the
+ * evaluated criteria.
*
- * @param value
- * @return Object[]
+ * @param value the criteria to be evaluated.
+ * @return an Object array of Object arrays of the evaluated criteria.
*/
protected Object[] evaluateSortCriteria(Object value) {
-
CompiledSortCriterion csc;
Object[] array = null;
if (orderByAttrs != null) {
array = new Object[orderByAttrs.size()];
- Iterator orderiter = orderByAttrs.iterator();
+ Iterator orderIter = orderByAttrs.iterator();
int i = 0;
- while (orderiter.hasNext()) {
-csc = (CompiledSortCriterion) orderiter.next();
-Object[] arr = new Object[2];
-arr[0] = csc.evaluate(value, context);
-arr[1] = Boolean.valueOf(csc.getCriterion());
+ while (orderIter.hasNext()) {
--- End diff --
Fixed, and updating the PR now - 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 #580: GEODE-2936: Refactoring OrderByComparator
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/580 GEODE-2936: Refactoring OrderByComparator [View the JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-2936) `OrderByComparator.java` contained a lot of redundant and confusing code, as well as somewhat misleading comments (for example, the `compare` method was supposed to throw a `ClassCastException` if the parameters given could not be compared using this comparator, but it didn't. This has been fixed). Code that is shared by both the `compare` and `evaluateSortCriteria` methods has been moved to helper methods to improve readability and reduce redundancy. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2936-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/580.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 #580 commit 3ff6d0a2b4f097947e337f5567b641aeadfb0e22 Author: Jinmei Liao <jil...@pivotal.io> Date: 2017-06-07T23:10:56Z GEODE-2936: Refactoring OrderByComparator to reduce redundant code and improve readability. --- 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 #578: GEODE-1958: Removing PasswordUtil
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/578 Precheckin is mostly green, with one (seemingly unrelated) test failure! I'm rerunning the test and looking into the failure 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 pull request #578: GEODE-1958: Removing PasswordUtil
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/578 GEODE-1958: Removing PasswordUtil [View the original JIRA ticket here.](https://issues.apache.org/jira/browse/GEODE-1958) `PasswordUtil.java` contained methods used to encrypt a password to be stored in `cache.xml`, which was an unsafe way to handle security. As a result, most methods in `PasswordUtil.java` were removed _except_ `decrypt(String password)`, since we want to maintain backwards compatibility. `decrypt(String password)` has been deprecated, and references to encrypting passwords have been removed. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-1958-v3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/578.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 #578 commit 2f08e08fa7fec127324ee47a338d331bf059bbf6 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-12T18:42:15Z GEODE-1958: Working on removing PasswordUtil and all related commands, classes, etc. Keeping decrypt() method to maintain backwards compatibility. --- 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 #575: GEODE-3048: Editing tests requiring Gradle test run...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/575#discussion_r121717861 --- Diff: geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java --- @@ -39,6 +39,8 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.ClassRule; --- End diff -- Okay, I'll get that done and update the PR! 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 issue #575: GEODE-3048: Editing tests requiring Gradle test runner to ...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/575 Precheckin in progress! --- 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 #575: GEODE-3048: Editing tests requiring Gradle test run...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/575 GEODE-3048: Editing tests requiring Gradle test runner to fail descriptively in IntelliJ Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-3048 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/575.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 #575 commit 9ba574244450766b2fa2e843719eb4a51a344b7d Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-08T19:09:14Z GEODE-3048: Editing tests that require the Gradle test runner to fail descriptively when run through IntelliJ --- 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 #571: GEODE-2601: Fixing banner being logged twice during...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/571 --- 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 #571: GEODE-2601: Fixing banner being logged twice during...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/571#discussion_r121499647 --- Diff: geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java --- @@ -87,19 +87,19 @@ public static InternalLogWriter createLogWriterLogger(final boolean isLoner, // LOG:CONFIG: logger.info(LogMarker.CONFIG, Banner.getString(null)); } + System.setProperty(InternalLocator.INHIBIT_DM_BANNER, "true"); // Ensure no more banners will --- End diff -- Yes, I had a suspicion that this would be an issue - do you have any suggestions concerning how to do this without changing a system property? I was considering just checking the log for whether it contains the banner, but this seems inefficient. --- 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 #571: GEODE-2601: Fixing banner being logged twice during...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/571#discussion_r121498978
--- Diff:
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
---
@@ -478,9 +478,8 @@ private InternalLocator(int port, File logF, File
stateF, InternalLogWriter logW
if (logWriter == null) {
logWriter = LogWriterFactory.createLogWriterLogger(false, false,
this.config,
!startDistributedSystem);
- if (logger.isDebugEnabled()) {
+ if (logger.isDebugEnabled())
--- End diff --
Oh, okay! I'll add the brackets back in!
---
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 #571: GEODE-2601: Fixing banner being logged twice during locato...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/571 Precheckin is in progress! Some failures so far that also exist on the develop branch - will take a look at those soon. --- 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 #571: GEODE-2601: Fixing banner being logged twice during...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/571 GEODE-2601: Fixing banner being logged twice during locator startup Now only logs once if both locator and distributed systems are started! There appear to be some minor changes in various places, possibly due to spotless checking; the main changes are in **lines 616-617 of `InternalDistributedSystem.java`** and **lines 483-484 of `InternalLocator.java`**. Patrick (@PurelyApplied) also noticed an unnecessary `if` statement in **lines 95 & 96 of `LogWriterFactory.java`**, which has been fixed in this PR. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2601-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/571.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 #571 commit d0c07e7bd0d938649ad9b49e2da1bc135145218f Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-08T21:40:39Z GEODE-2601: Fixing banner being logged twice during locator startup (now only logs once if both locator and distributed systems are started) --- 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 #561: GEODE-3033: Fixing NPE when jarFileNames is null in Cluste...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/561 Precheckin in progress! --- 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 #561: GEODE-3033: Fixing NPE when jarFileNames is null in...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/561 GEODE-3033: Fixing NPE when jarFileNames is null in ClusterConfigurationLoader Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-3033 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/561.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 #561 commit b0044cdc9e7fbfc8db421b1af0d511df77fdbc1a Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-05T21:21:31Z GEODE-3033: Fixing NPE when jarFileNames is null in ClusterConfigurationLoader --- 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 #560: Geode 2818: Adding aliases to any command options t...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/560 Geode 2818: Adding aliases to any command options that involve "group", "member", or "jar" ⦠"member", "jar" and replace CliString variables with GROUP, MEMBER, JAR, etc. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2818 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/560.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 #560 commit 24958612611c293497cec7300856c68f2bfb819a Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-06-05T20:03:39Z GEODE-2818: add alias to any command's options that involves "group", "member", "jar" and replace CliString variables with GROUP, MEMBER, JAR, etc. --- 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 #548: GEODE-2818: add alias to any command's options that...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/548 --- 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 #548: GEODE-2818: add alias to any command's options that...
Github user YehEmily commented on a diff in the pull request: https://github.com/apache/geode/pull/548#discussion_r120137950 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java --- @@ -412,7 +411,7 @@ 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.COMPACT_DISK_STORE__GROUP, + @CliOption(key = CliStrings.GROUP, --- End diff -- Sure thing! I'll update the PR once this is done! --- 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 #549: GEODE-2203: gfsh status locator/server - Command no...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/549#discussion_r119422035
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
---
@@ -771,11 +771,19 @@ public Result statusLocator(
CliStrings.STATUS_SERVICE__GFSH_NOT_CONNECTED_ERROR_MESSAGE,
LOCATOR_TERM_NAME));
}
} else {
-final LocatorLauncher locatorLauncher =
-new
LocatorLauncher.Builder().setCommand(LocatorLauncher.Command.STATUS)
-
.setBindAddress(locatorHost).setDebug(isDebugging()).setPid(pid)
-
.setPort(locatorPort).setWorkingDirectory(workingDirectory).build();
-
+final LocatorLauncher locatorLauncher;
+
+if ((locatorHost == null) && (locatorPort == null) &&
(workingDirectory == null)
+&& (pid == null)) {
+ return ResultBuilder.createShellClientErrorResult(
+
String.format(CliStrings.STATUS_LOCATOR__NO_LOCATOR_SPECIFIED_ERROR_MESSAGE,
+ getLocatorId(locatorHost, locatorPort),
+ StringUtils.defaultIfBlank(workingDirectory,
SystemUtils.CURRENT_DIRECTORY)));
--- End diff --
Oops! Sorry, I'll fix that right away - 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 #549: GEODE-2203: gfsh status locator/server - Command no...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/549#discussion_r119225797
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
---
@@ -1794,18 +1801,20 @@ public Result statusServer(
.format(CliStrings.STATUS_SERVICE__GFSH_NOT_CONNECTED_ERROR_MESSAGE, "Cache
Server"));
}
} else {
-final ServerLauncher serverLauncher = new ServerLauncher.Builder()
-
.setCommand(ServerLauncher.Command.STATUS).setDebug(isDebugging())
-// NOTE since we do not know whether the "CacheServer" was
enabled or not on the GemFire
-// server when it was started, set the disableDefaultServer
property in the
-// ServerLauncher.Builder to default status
-// to the MemberMBean
-// TODO fix this hack! (how, the 'start server' loop needs it)
-
.setDisableDefaultServer(true).setPid(pid).setWorkingDirectory(workingDirectory)
-.build();
-
+final ServerLauncher serverLauncher;
+if ((member == null)) {
--- End diff --
Oops, that's very true - thanks! 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 #549: GEODE-2203: gfsh status locator/server - Command no...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/549#discussion_r119225590
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
---
@@ -771,11 +771,18 @@ public Result statusLocator(
CliStrings.STATUS_SERVICE__GFSH_NOT_CONNECTED_ERROR_MESSAGE,
LOCATOR_TERM_NAME));
}
} else {
-final LocatorLauncher locatorLauncher =
-new
LocatorLauncher.Builder().setCommand(LocatorLauncher.Command.STATUS)
-
.setBindAddress(locatorHost).setDebug(isDebugging()).setPid(pid)
-
.setPort(locatorPort).setWorkingDirectory(workingDirectory).build();
+final LocatorLauncher locatorLauncher;
+if ((locatorHost == null) && (locatorPort == null) &&
(workingDirectory == null)) {
+ return ResultBuilder.createShellClientErrorResult(
+
String.format(CliStrings.STATUS_LOCATOR__NO_LOCATOR_SPECIFIED_ERROR_MESSAGE,
+ getLocatorId(locatorHost, locatorPort),
+ StringUtils.defaultIfBlank(workingDirectory,
SystemUtils.CURRENT_DIRECTORY)));
--- End diff --
Got it - thanks! 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 #549: GEODE-2203: gfsh status locator/server - Command no...
Github user YehEmily commented on a diff in the pull request:
https://github.com/apache/geode/pull/549#discussion_r119225532
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
---
@@ -771,11 +771,18 @@ public Result statusLocator(
CliStrings.STATUS_SERVICE__GFSH_NOT_CONNECTED_ERROR_MESSAGE,
LOCATOR_TERM_NAME));
}
} else {
-final LocatorLauncher locatorLauncher =
-new
LocatorLauncher.Builder().setCommand(LocatorLauncher.Command.STATUS)
-
.setBindAddress(locatorHost).setDebug(isDebugging()).setPid(pid)
-
.setPort(locatorPort).setWorkingDirectory(workingDirectory).build();
+final LocatorLauncher locatorLauncher;
+if ((locatorHost == null) && (locatorPort == null) &&
(workingDirectory == null)) {
--- End diff --
I think that if a value is given for locatorPort but not for locatorHost,
`locatorLauncher` will just have a null value for locatorPort, with no errors
(in the `else` statement). Do you mean there will be an error in a different
part of the code?
---
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 #549: GEODE-2203: gfsh status locator/server - Command now gives...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/549 Precheckin pending! --- 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 #549: GEODE-2203: gfsh status locator/server - Command no...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/549 GEODE-2203: gfsh status locator/server - Command now gives more descr⦠â¦iptive output on empty parameter Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2203 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/549.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 #549 commit 784ded407a0d16dc7fbf3c6693d0e1a8443fdd44 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-26T22:54:46Z GEODE-2203: gfsh status locator/server - Command now gives more descriptive output on empty parameter --- 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 #548: GEODE-2818: add alias to any command's options that...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/548 GEODE-2818: add alias to any command's options that involves "group",⦠⦠"member", "jar" and replace CliString variables with GROUP, MEMBER, JAR, etc. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode GEODE-2818 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/548.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 #548 commit 96df68ea155e63fa80be8f0a4be2f7b7e0cc6363 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-30T17:56:48Z GEODE-2818: add alias to any command's options that involves "group", "member", "jar" and replace CliString variables with GROUP, MEMBER, JAR, etc. --- 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 #548: GEODE-2818: add alias to any command's options that involv...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/548 Precheckin in progress! --- 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 #546: GEODE-2818: add alias to any command's options that...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/546 --- 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 #546: GEODE-2818: add alias to any command's options that...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/546 GEODE-2818: add alias to any command's options that involves "group", "member", "jar" Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode synonyms.rerun.rererun Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/546.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 #546 commit 80871fe2fc68247c8f53ac3ac78aaff16adc483a Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-25T19:18:45Z GEODE-2818: Edited command files and testing files to allow either 'group' or 'groups' and 'member' or 'members' and 'jar' or 'jars' commit 3fd97485d708cdd489968e12c08153f0a39fa243 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-26T18:50:35Z GEODE-2818: Edited command files and testing files to allow either 'group' or 'groups' and 'member' or 'members' and 'jar' or 'jars' commit bcf2e7d6940cddb241418463da41a44df5e883bb Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-29T22:21:40Z GEODE-2818: add alias to any command's options that involves "group", "member", "jar" --- 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 #539: GEODE-2818: add alias to any command's options that...
Github user YehEmily closed the pull request at: https://github.com/apache/geode/pull/539 --- 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 #539: GEODE-2818: add alias to any command's options that involv...
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/539 Precheckin pending! --- 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 #539: GEODE-2818: add alias to any command's options that...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/539 GEODE-2818: add alias to any command's options that involves "group", "member", "jar" Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode synonyms Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/539.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 #539 commit 01c5119654f27a254f8fb334b9fe8a2d5c4039ac Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-23T19:27:40Z GEODE-2977: make group/name option values consistent commit 2076f03d8cd3748f204518c47b3a6245f9f6a714 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-25T19:18:45Z Edited command files to allow either 'group' or 'groups' and 'member' or 'members --- 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 #536: GEODE-2977: make group/name option values consistent
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/536 Precheckin was successful! --- 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 #536: GEODE-2977: make group/name option values consisten...
GitHub user YehEmily opened a pull request: https://github.com/apache/geode/pull/536 GEODE-2977: make group/name option values consistent Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, 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/YehEmily/geode groupname Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/536.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 #536 commit 01c5119654f27a254f8fb334b9fe8a2d5c4039ac Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-05-23T19:27:40Z GEODE-2977: make group/name option values consistent --- 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. ---
