You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by PurelyApplied <gi...@git.apache.org> on 2017/08/28 17:55:45 UTC
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
GitHub user PurelyApplied opened a pull request:
https://github.com/apache/geode/pull/745
GEODE-3436: Restore reverted GFSH command refactoring
Given that this would restore multiple tickets worth of commits, it was unclear if this PR should squash all commits or if the history would be clearer without one enormous commit.
Precheckin returned green excepting spotless and some other minor adjustments (import ordering correction, etc). Running another precheckin for due course.
-----
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`)?
- [ ] Is your initial contribution a single, squashed commit?
- [x] Does `gradlew build` run cleanly?
- [n/a] Have you written or updated unit tests to verify your changes? [Existing tests provide coverage for refactored code.]
- [n/a] 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/PurelyApplied/geode geode-3436
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/745.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 #745
----
commit caeb8375db68d68ec0ddbca6b81fd6ca6c91a73f
Author: YehEmily <em...@gmail.com>
Date: 2017-08-22T17:57:05Z
GEODE-3436: Restore refactoring of MemberCommands
* See initial commit GEODE-3264 (d27f8b956de7d9c5d95ebdc68dfc67ee8b2d7b51)
commit 44cc0e8bb7927e61bb3e7613612703518fc87db6
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T18:52:14Z
GEODE-3436: Restore refactoring of DurableClientCommands
* See initial commit GEODE-3259 (440c87f81fab96f9ce38a2d53ded75e5fe8390d7)
commit 1e9eea1f1b2d2c620679283aaf6e6c5ac12e1728
Author: YehEmily <em...@gmail.com>
Date: 2017-08-04T17:47:48Z
GEODE-3436: Restore refactoring of PDXCommands
* See initial commit GEODE-3266 (67185abcdd68b908dea6888cb94286b8aa9ea49f)
commit 691700c2e51aef5b18ba5feec711bddf5ae9a05e
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T20:58:08Z
GEODE-3436: Restore refactoring of RegionCommands
* See initial commits GEODE-3268 (64de3b69c2aecb4930bcfd0a1161569b1d5fda89), GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890)
commit 0acf8be27f5ab5f6d9d5009bb7ec6740c1f7ae1b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T21:47:15Z
GEODE-3436: Restore refactoring of QueueCommands
* See initial commit GEODE-3267 (fd47ed660168864a6f81b2a4cd7dbceebc99a282)
commit 7594ebdd5e593179f0b33a94c30c7d5949fabf96
Author: YehEmily <em...@gmail.com>
Date: 2017-08-24T22:42:12Z
GEODE-3436: Restore refactoring of GfshHelpCommands
* See initial commit GEODE-3261 (cf91426692349d0c81ce77394935576d9cc336e8)
commit 5daeb011cbeebfa2a71c225ed3dc3abc2047e3dd
Author: YehEmily <em...@gmail.com>
Date: 2017-08-24T23:11:13Z
GEODE-3436: Restore refactoring of CreateAlterDestroyRegionCommands
* See initial commit GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890)
commit a1823d323c26adebacf7536448e2cadf054d9528
Author: YehEmily <em...@gmail.com>
Date: 2017-07-26T18:07:09Z
GEODE-3436: Restore refactoring of ConfigCommands
* See initial commit GEODE-3254 (97c4e9a59f17c7bc914e39dd048b0a4cd96293c4)
commit 78ec3cb687313799a911ed3d1ca6e560cb358de8
Author: YehEmily <em...@gmail.com>
Date: 2017-08-03T16:00:08Z
GEODE-3436: Restore refactoring of DiskStoreCommands
* See initial commit GEODE-3258 (5d6cad7755ec3c4fe931e3d0f8e89fb181038543)
commit 1ecbd6fa08343a8de50bf30839c9ef47c02e73aa
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T19:35:14Z
GEODE-3436: Restore refactoring of IndexCommands
* See initial commit GEODE-3262 (ed293e817e547fb5ecd399bf4ba10d694af51e0a)
commit b562bdafc46045bd2679c233cf591549fb016d8b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T19:56:17Z
GEODE-3436: Restore refactoring of Refactoring FunctionCommands
* See initial commit GEODE-3260 (90f5440de8ec747f301a309a0a34101e8defcd29)
commit e1ed96e4130d7b8894c71a2a4d497a6a628d8f16
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T22:37:23Z
GEODE-3436: Restore refactoring of Refactoring MiscellaneousCommands
* See initial commit GEODE-3265 (63169699e933f6e0fdd90b95ed039e4e3c92c32c)
commit a9f76c2db17f9cf18ccb49a36b827328424cb23f
Author: YehEmily <em...@gmail.com>
Date: 2017-08-03T00:28:10Z
GEODE-3436: Restore refactoring of DeployCommands
* See initial commit GEODE-3257 (9d967446a44a78b612f605b6a8f8eedcfc625b3a)
commit dcb2f57adc2d3bb5d5d5b0483f815e585eab6f3b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-28T17:08:26Z
GEODE-3436: Restore refactoring of StatusCommands
* See initial commit GEODE-3270 (359e3fff6482ecfb375939d387f4dad3a636246b)
commit f5ea7c75a740a9ddd32649beae1d2415970cf51b
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-26T00:18:17Z
GEODE-3436: Corrected import order of touched files.
* Spotless and some minor improvements based on IDE inspections
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135635781
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommand.java ---
@@ -794,7 +84,7 @@ public Result showMetrics(
}
if (regionName != null && !regionName.isEmpty()) {
- if (!org.apache.geode.internal.lang.StringUtils.isBlank(cacheServerPortString)) {
+ if (!StringUtils.isBlank(cacheServerPortString)) {
--- End diff --
One step closer to purging that class altogether!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135632737
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java ---
@@ -247,7 +207,7 @@ public Result createRegion(
new Object[] {CliStrings.CREATE_REGION__USEATTRIBUTESFROM, useAttributesFrom}));
--- End diff --
If there is one person to whom you need not apologize for nit-picking, it's this guy.
Inspections find six cases across `RegionCreateFunction` and `CreateRegionCommand`. Done and 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by pdxrunner <gi...@git.apache.org>.
Github user pdxrunner commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135639047
--- Diff: geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java ---
@@ -171,28 +171,37 @@ private static void init() {
createTestCommand("alter disk-store --name=foo --region=xyz --disk-dirs=bar");
createTestCommand("destroy disk-store --name=foo", clusterManageDisk);
- // DurableClientCommands
+ // CloseDurableClientCommand, CloseDurableCQsCommand, CountDurableCQEventsCommand,
+ // ListDurableClientCQsCommand
createTestCommand("close durable-client --durable-client-id=client1", clusterManageQuery);
createTestCommand("close durable-cq --durable-client-id=client1 --durable-cq-name=cq1",
clusterManageQuery);
createTestCommand("show subscription-queue-size --durable-client-id=client1", clusterRead);
createTestCommand("list durable-cqs --durable-client-id=client1", clusterRead);
- // ExportIMportSharedConfigurationCommands
+ // ExportImportSharedConfigurationCommands
createTestCommand("export cluster-configuration --zip-file-name=mySharedConfig.zip",
clusterRead);
createTestCommand("import cluster-configuration --zip-file-name=value.zip", clusterManage);
- // FunctionCommands
+ // DestroyFunctionCommand, ExecuteFunctionCommand, ListFunctionCommand
+ // TODO PSR: the `destroy function` command is interactive (in its interceptor) when both
--- End diff --
Have got a resolution on these TODO's?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #745: GEODE-3436: Restore reverted GFSH command refactoring
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied commented on the issue:
https://github.com/apache/geode/pull/745
Precheckin fully green.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by pdxrunner <gi...@git.apache.org>.
Github user pdxrunner commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135633281
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java ---
@@ -156,7 +132,7 @@ public Result executeFunction(
// if user wish to execute on locator then he can choose --member or --group option
Set<DistributedMember> dsMembers = CliUtil.getAllNormalMembers(cache);
if (dsMembers.size() > 0) {
- function = new UserFunctionExecution();
+ new UserFunctionExecution();
--- End diff --
Where is this object used?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied closed the pull request at:
https://github.com/apache/geode/pull/745
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135635795
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommand.java ---
@@ -794,7 +84,7 @@ public Result showMetrics(
}
if (regionName != null && !regionName.isEmpty()) {
- if (!org.apache.geode.internal.lang.StringUtils.isBlank(cacheServerPortString)) {
+ if (!StringUtils.isBlank(cacheServerPortString)) {
--- End diff --
Let's leave these things for other cleanup tasks. The focus of this change set is to break these command into separate classes. I know we all can't resist the urge of making everything perfect from time to time, but one thing at a time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135605262
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java ---
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.functions.CreateDefinedIndexesFunction;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ErrorResultData;
+import org.apache.geode.management.internal.cli.result.InfoResultData;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class CreateDefinedIndexesCommand implements GfshCommand {
+ private static final CreateDefinedIndexesFunction createDefinedIndexesFunction =
+ new CreateDefinedIndexesFunction();
+
+ @CliCommand(value = CliStrings.CREATE_DEFINED_INDEXES, help = CliStrings.CREATE_DEFINED__HELP)
+ @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA})
+ @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+ operation = ResourcePermission.Operation.MANAGE, target = ResourcePermission.Target.QUERY)
+ // TODO : Add optionContext for indexName
+ public Result createDefinedIndexes(
+
+ @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+ optionContext = ConverterHint.MEMBERIDNAME,
+ help = CliStrings.CREATE_DEFINED_INDEXES__MEMBER__HELP) final String[] memberNameOrID,
+
+ @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+ optionContext = ConverterHint.MEMBERGROUP,
+ help = CliStrings.CREATE_DEFINED_INDEXES__GROUP__HELP) final String[] group) {
+
+ Result result;
+ AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+
+ if (IndexDefinition.indexDefinitions.isEmpty()) {
+ final InfoResultData infoResult = ResultBuilder.createInfoResultData();
+ infoResult.addLine(CliStrings.DEFINE_INDEX__FAILURE__MSG);
+ return ResultBuilder.buildResult(infoResult);
+ }
+
+ try {
+ final Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrID);
+
+ if (targetMembers.isEmpty()) {
+ return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+ }
+
+ // TODO PSR: is this safe to remove?
+ CacheFactory.getAnyInstance();
--- End diff --
I favor removal of all CacheFactory.getAnyInstance() calls anywhere in which the return is not used.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135599224
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java ---
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.functions.CreateDefinedIndexesFunction;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ErrorResultData;
+import org.apache.geode.management.internal.cli.result.InfoResultData;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class CreateDefinedIndexesCommand implements GfshCommand {
+ private static final CreateDefinedIndexesFunction createDefinedIndexesFunction =
+ new CreateDefinedIndexesFunction();
+
+ @CliCommand(value = CliStrings.CREATE_DEFINED_INDEXES, help = CliStrings.CREATE_DEFINED__HELP)
+ @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA})
+ @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+ operation = ResourcePermission.Operation.MANAGE, target = ResourcePermission.Target.QUERY)
+ // TODO : Add optionContext for indexName
+ public Result createDefinedIndexes(
+
+ @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+ optionContext = ConverterHint.MEMBERIDNAME,
+ help = CliStrings.CREATE_DEFINED_INDEXES__MEMBER__HELP) final String[] memberNameOrID,
+
+ @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+ optionContext = ConverterHint.MEMBERGROUP,
+ help = CliStrings.CREATE_DEFINED_INDEXES__GROUP__HELP) final String[] group) {
+
+ Result result;
+ AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+
+ if (IndexDefinition.indexDefinitions.isEmpty()) {
+ final InfoResultData infoResult = ResultBuilder.createInfoResultData();
+ infoResult.addLine(CliStrings.DEFINE_INDEX__FAILURE__MSG);
+ return ResultBuilder.buildResult(infoResult);
+ }
+
+ try {
+ final Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrID);
+
+ if (targetMembers.isEmpty()) {
+ return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+ }
+
+ // TODO PSR: is this safe to remove?
+ CacheFactory.getAnyInstance();
--- End diff --
Oh no I forgot to address all my TODOs!
That was there to remind me to spike that command out and see the right way to get rid of it. Thanks for catching that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135641458
--- Diff: geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java ---
@@ -171,28 +171,37 @@ private static void init() {
createTestCommand("alter disk-store --name=foo --region=xyz --disk-dirs=bar");
createTestCommand("destroy disk-store --name=foo", clusterManageDisk);
- // DurableClientCommands
+ // CloseDurableClientCommand, CloseDurableCQsCommand, CountDurableCQEventsCommand,
+ // ListDurableClientCQsCommand
createTestCommand("close durable-client --durable-client-id=client1", clusterManageQuery);
createTestCommand("close durable-cq --durable-client-id=client1 --durable-cq-name=cq1",
clusterManageQuery);
createTestCommand("show subscription-queue-size --durable-client-id=client1", clusterRead);
createTestCommand("list durable-cqs --durable-client-id=client1", clusterRead);
- // ExportIMportSharedConfigurationCommands
+ // ExportImportSharedConfigurationCommands
createTestCommand("export cluster-configuration --zip-file-name=mySharedConfig.zip",
clusterRead);
createTestCommand("import cluster-configuration --zip-file-name=value.zip", clusterManage);
- // FunctionCommands
+ // DestroyFunctionCommand, ExecuteFunctionCommand, ListFunctionCommand
+ // TODO PSR: the `destroy function` command is interactive (in its interceptor) when both
--- End diff --
Killed the TODO, left the function commented out since it'll throw an error before authentication in that test.
I think that and the associated classes need a refactor in any case. I'll address the commented-out-ness of the several functions then.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by PurelyApplied <gi...@git.apache.org>.
GitHub user PurelyApplied reopened a pull request:
https://github.com/apache/geode/pull/745
GEODE-3436: Restore reverted GFSH command refactoring
Given that this would restore multiple tickets worth of commits, it was unclear if this PR should squash all commits or if the history would be clearer without one enormous commit.
Precheckin returned green excepting spotless and some other minor adjustments (import ordering correction, etc). Running another precheckin for due course.
-----
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`)?
- [ ] Is your initial contribution a single, squashed commit?
- [x] Does `gradlew build` run cleanly?
- [n/a] Have you written or updated unit tests to verify your changes? [Existing tests provide coverage for refactored code.]
- [n/a] 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/PurelyApplied/geode geode-3436
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/745.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 #745
----
commit caeb8375db68d68ec0ddbca6b81fd6ca6c91a73f
Author: YehEmily <em...@gmail.com>
Date: 2017-08-22T17:57:05Z
GEODE-3436: Restore refactoring of MemberCommands
* See initial commit GEODE-3264 (d27f8b956de7d9c5d95ebdc68dfc67ee8b2d7b51)
commit 44cc0e8bb7927e61bb3e7613612703518fc87db6
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T18:52:14Z
GEODE-3436: Restore refactoring of DurableClientCommands
* See initial commit GEODE-3259 (440c87f81fab96f9ce38a2d53ded75e5fe8390d7)
commit 1e9eea1f1b2d2c620679283aaf6e6c5ac12e1728
Author: YehEmily <em...@gmail.com>
Date: 2017-08-04T17:47:48Z
GEODE-3436: Restore refactoring of PDXCommands
* See initial commit GEODE-3266 (67185abcdd68b908dea6888cb94286b8aa9ea49f)
commit 691700c2e51aef5b18ba5feec711bddf5ae9a05e
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T20:58:08Z
GEODE-3436: Restore refactoring of RegionCommands
* See initial commits GEODE-3268 (64de3b69c2aecb4930bcfd0a1161569b1d5fda89), GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890)
commit 0acf8be27f5ab5f6d9d5009bb7ec6740c1f7ae1b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T21:47:15Z
GEODE-3436: Restore refactoring of QueueCommands
* See initial commit GEODE-3267 (fd47ed660168864a6f81b2a4cd7dbceebc99a282)
commit 7594ebdd5e593179f0b33a94c30c7d5949fabf96
Author: YehEmily <em...@gmail.com>
Date: 2017-08-24T22:42:12Z
GEODE-3436: Restore refactoring of GfshHelpCommands
* See initial commit GEODE-3261 (cf91426692349d0c81ce77394935576d9cc336e8)
commit 5daeb011cbeebfa2a71c225ed3dc3abc2047e3dd
Author: YehEmily <em...@gmail.com>
Date: 2017-08-24T23:11:13Z
GEODE-3436: Restore refactoring of CreateAlterDestroyRegionCommands
* See initial commit GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890)
commit a1823d323c26adebacf7536448e2cadf054d9528
Author: YehEmily <em...@gmail.com>
Date: 2017-07-26T18:07:09Z
GEODE-3436: Restore refactoring of ConfigCommands
* See initial commit GEODE-3254 (97c4e9a59f17c7bc914e39dd048b0a4cd96293c4)
commit 78ec3cb687313799a911ed3d1ca6e560cb358de8
Author: YehEmily <em...@gmail.com>
Date: 2017-08-03T16:00:08Z
GEODE-3436: Restore refactoring of DiskStoreCommands
* See initial commit GEODE-3258 (5d6cad7755ec3c4fe931e3d0f8e89fb181038543)
commit 1ecbd6fa08343a8de50bf30839c9ef47c02e73aa
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T19:35:14Z
GEODE-3436: Restore refactoring of IndexCommands
* See initial commit GEODE-3262 (ed293e817e547fb5ecd399bf4ba10d694af51e0a)
commit b562bdafc46045bd2679c233cf591549fb016d8b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T19:56:17Z
GEODE-3436: Restore refactoring of Refactoring FunctionCommands
* See initial commit GEODE-3260 (90f5440de8ec747f301a309a0a34101e8defcd29)
commit e1ed96e4130d7b8894c71a2a4d497a6a628d8f16
Author: YehEmily <em...@gmail.com>
Date: 2017-08-07T22:37:23Z
GEODE-3436: Restore refactoring of Refactoring MiscellaneousCommands
* See initial commit GEODE-3265 (63169699e933f6e0fdd90b95ed039e4e3c92c32c)
commit a9f76c2db17f9cf18ccb49a36b827328424cb23f
Author: YehEmily <em...@gmail.com>
Date: 2017-08-03T00:28:10Z
GEODE-3436: Restore refactoring of DeployCommands
* See initial commit GEODE-3257 (9d967446a44a78b612f605b6a8f8eedcfc625b3a)
commit dcb2f57adc2d3bb5d5d5b0483f815e585eab6f3b
Author: YehEmily <em...@gmail.com>
Date: 2017-08-28T17:08:26Z
GEODE-3436: Restore refactoring of StatusCommands
* See initial commit GEODE-3270 (359e3fff6482ecfb375939d387f4dad3a636246b)
commit f5ea7c75a740a9ddd32649beae1d2415970cf51b
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-26T00:18:17Z
GEODE-3436: Corrected import order of touched files.
* Spotless and some minor improvements based on IDE inspections
commit 5652d7e3d7979c339ac15a553f40ce7834ab16c6
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-28T18:47:50Z
GEODE-3436: Removed unused interceptor, unused calls to getCache() and CacheFactory.getAnyInstance(), and lingering TODOs
commit 93d581c7c8c31dc64d88df2da6b73124d5a9b70e
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-28T20:58:23Z
GEODE-3436: Additional PR suggested changes.
commit f5a348f3b44e204544f2143671001f8258135e06
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-28T21:12:39Z
GEODE-3436: Making suggested changes in real time.
commit ddab82e48112542d797588217336a092d8a29f09
Author: Patrick Rhomberg <pr...@pivotal.io>
Date: 2017-08-28T22:11:28Z
empty commit to trigger Travis.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by pdxrunner <gi...@git.apache.org>.
Github user pdxrunner commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135634811
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommand.java ---
@@ -794,7 +84,7 @@ public Result showMetrics(
}
if (regionName != null && !regionName.isEmpty()) {
- if (!org.apache.geode.internal.lang.StringUtils.isBlank(cacheServerPortString)) {
+ if (!StringUtils.isBlank(cacheServerPortString)) {
--- End diff --
Use apache.commons.StringUtils.isNotBlank() instead of the deprecated Geode version of StringUtils
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by pdxrunner <gi...@git.apache.org>.
Github user pdxrunner commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135597926
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java ---
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.functions.CreateDefinedIndexesFunction;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ErrorResultData;
+import org.apache.geode.management.internal.cli.result.InfoResultData;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class CreateDefinedIndexesCommand implements GfshCommand {
+ private static final CreateDefinedIndexesFunction createDefinedIndexesFunction =
+ new CreateDefinedIndexesFunction();
+
+ @CliCommand(value = CliStrings.CREATE_DEFINED_INDEXES, help = CliStrings.CREATE_DEFINED__HELP)
+ @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_REGION, CliStrings.TOPIC_GEODE_DATA})
+ @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+ operation = ResourcePermission.Operation.MANAGE, target = ResourcePermission.Target.QUERY)
+ // TODO : Add optionContext for indexName
+ public Result createDefinedIndexes(
+
+ @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+ optionContext = ConverterHint.MEMBERIDNAME,
+ help = CliStrings.CREATE_DEFINED_INDEXES__MEMBER__HELP) final String[] memberNameOrID,
+
+ @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+ optionContext = ConverterHint.MEMBERGROUP,
+ help = CliStrings.CREATE_DEFINED_INDEXES__GROUP__HELP) final String[] group) {
+
+ Result result;
+ AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+
+ if (IndexDefinition.indexDefinitions.isEmpty()) {
+ final InfoResultData infoResult = ResultBuilder.createInfoResultData();
+ infoResult.addLine(CliStrings.DEFINE_INDEX__FAILURE__MSG);
+ return ResultBuilder.buildResult(infoResult);
+ }
+
+ try {
+ final Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrID);
+
+ if (targetMembers.isEmpty()) {
+ return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+ }
+
+ // TODO PSR: is this safe to remove?
+ CacheFactory.getAnyInstance();
--- End diff --
I think this should be replaced by calling the GfshCommand interface's getCache() to limit the places where this potentially deadlock prone call is made. . Most (or at least many) other gfsh commands call the interface's method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/geode/pull/745
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
Posted by pdxrunner <gi...@git.apache.org>.
Github user pdxrunner commented on a diff in the pull request:
https://github.com/apache/geode/pull/745#discussion_r135629055
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java ---
@@ -247,7 +207,7 @@ public Result createRegion(
new Object[] {CliStrings.CREATE_REGION__USEATTRIBUTESFROM, useAttributesFrom}));
--- End diff --
Bit of a nit-pick, but we have been trying to clean up these cases where we unnecessarily create new object arrays to pass to varargs methods. This occurs several places in the class
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---