[GitHub] geode issue #748: GEODE-3472: Remove a great deal of commented-out code.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/748 Precheckin 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/730 --- 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 #730: GEODE-3472: Remove a great deal of commented-out code.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/730 Due to significant changes, this pull request is replaced by #748. --- 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 #748: GEODE-3472: Remove a great deal of commented-out co...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/748 GEODE-3472: Remove a great deal of commented-out code. This pull request replaces #730. Nontrivial conflicts were introduced after rebasing against GEODE-3436 (`be8a13`). This pull request includes all suggestions made in #730, as well as a number of other minor refactorings. Precheckin running. - * Additionally refactored touched files, driven by suggested changes in a previous pull request and IDE inspections. 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? - [n/a: refactoring] Have you written or updated unit tests to verify your changes? - [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-3472-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/748.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 #748 commit 4fecf79d76f2caec59f0c8e5afb0744954e5e203 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-18T20:59:44Z GEODE-3472: Remove a great deal of commented-out code. * Additionally refactored touched files, driven by suggested changes in a previous pull request and 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #745: GEODE-3436: Restore reverted GFSH command refactoring
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
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 <emilyyeh1...@gmail.com> Date: 2017-08-22T17:57:05Z GEODE-3436: Restore refactoring of MemberCommands * See initial commit GEODE-3264 (d27f8b956de7d9c5d95ebdc68dfc67ee8b2d7b51) commit 44cc0e8bb7927e61bb3e7613612703518fc87db6 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T18:52:14Z GEODE-3436: Restore refactoring of DurableClientCommands * See initial commit GEODE-3259 (440c87f81fab96f9ce38a2d53ded75e5fe8390d7) commit 1e9eea1f1b2d2c620679283aaf6e6c5ac12e1728 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-04T17:47:48Z GEODE-3436: Restore refactoring of PDXCommands * See initial commit GEODE-3266 (67185abcdd68b908dea6888cb94286b8aa9ea49f) commit 691700c2e51aef5b18ba5feec711bddf5ae9a05e Author: YehEmily <emilyyeh1...@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 <emilyyeh1...@gmail.com> Date: 2017-08-07T21:47:15Z GEODE-3436: Restore refactoring of QueueCommands * See initial commit GEODE-3267 (fd47ed660168864a6f81b2a4cd7dbceebc99a282) commit 7594ebdd5e593179f0b33a94c30c7d5949fabf96 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-24T22:42:12Z GEODE-3436: Restore refactoring of GfshHelpCommands * See initial commit GEODE-3261 (cf91426692349d0c81ce77394935576d9cc336e8) commit 5daeb011cbeebfa2a71c225ed3dc3abc2047e3dd Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-24T23:11:13Z GEODE-3436: Restore refactoring of CreateAlterDestroyRegionCommands * See initial commit GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890) commit a1823d323c26adebacf7536448e2cadf054d9528 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-26T18:07:09Z GEODE-3436: Restore refactoring of ConfigCommands * See initial commit GEODE-3254 (97c4e9a59f17c7bc914e39dd048b0a4cd96293c4) commit 78ec3cb687313799a911ed3d1ca6e560cb358de8 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-03T16:00:08Z GEODE-3436: Restore refactoring of DiskStoreCommands * See initial commit GEODE-3258 (5d6cad7755ec3c4fe931e3d0f8e89fb181038543) commit 1ecbd6fa08343a8de50bf30839c9ef47c02e73aa Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T19:35:14Z GEODE-3436: Restore refactoring of IndexCommands * See initial commit GEODE-3262 (ed293e817e547fb5ecd399bf4ba10d694af51e0a) commit b562bdafc46045bd2679c233cf591549fb016d8b Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T19:56:17Z GEODE-3436: Restore refactoring of Ref
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/730 --- 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 #730: GEODE-3472: Remove a great deal of commented-out co...
GitHub user PurelyApplied reopened a pull request: https://github.com/apache/geode/pull/730 GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, 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: - [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? - [n/a] Have you written or updated unit tests to verify your changes? [n/a: Non-functional changes.] - [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-3472 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/730.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 #730 commit 80f44577b8f1431c14934a935f6d426f28ed0dea Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-18T20:59:44Z GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, etc. commit 7a0db737bc7327adea58a5c972ad04856850c8b7 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-28T20:41:19Z GEODE-3472: Suggested changes in pull request. * Corrected incorrect correction of a typo in Gfsh.java * Removed dead, trivial method * Moved constant field from MBeanJMXAdapter to its use in VMStatsMonitor commit 1324ed15533dbf5fe95a81e214d81d2355f239fd Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-28T22:10:51Z 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/730 --- 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 #730: GEODE-3472: Remove a great deal of commented-out co...
GitHub user PurelyApplied reopened a pull request: https://github.com/apache/geode/pull/730 GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, 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: - [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? - [n/a] Have you written or updated unit tests to verify your changes? [n/a: Non-functional changes.] - [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-3472 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/730.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 #730 commit 80f44577b8f1431c14934a935f6d426f28ed0dea Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-18T20:59:44Z GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, etc. commit 7a0db737bc7327adea58a5c972ad04856850c8b7 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-28T20:41:19Z GEODE-3472: Suggested changes in pull request. * Corrected incorrect correction of a typo in Gfsh.java * Removed dead, trivial method * Moved constant field from MBeanJMXAdapter to its use in VMStatsMonitor commit 1324ed15533dbf5fe95a81e214d81d2355f239fd Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-28T22:10:51Z 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #730: GEODE-3472: Remove a great deal of commented-out code.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/730 Precheckin returned green. Open/Close PR to trigger Travis again. Hopefully they've pulled `develop` recently... --- 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 #730: GEODE-3472: Remove a great deal of commented-out code.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/730 Travis failures appear related to `f41ca9d7d2fa7c045ec439df9478335233f1d95e`. Is there a way to force Travis to rerun without applying a new commit? --- 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 #745: GEODE-3436: Restore reverted GFSH command refactori...
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #745: GEODE-3436: Restore reverted GFSH command refactori...
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/730#discussion_r135626288
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
---
@@ -212,8 +211,7 @@ public static void bytesToFiles(byte[][] fileData,
String parentDirPath, boolean
}
public static boolean isValidFileName(String filePath, String extension)
{
-boolean isValid = true;
-return isValid;
+return true;
--- End diff --
Looks unused. Must've just condensed the return from IntelliJ's inspection
window and not looked too closely at this silly method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied commented on a diff in the pull request: https://github.com/apache/geode/pull/730#discussion_r135625955 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java --- @@ -158,7 +156,7 @@ private Thread runner; private boolean debugON; private Terminal terminal; - private boolean supressScriptCmdOutput; + private boolean suppressScriptCadOutput; --- End diff -- IntelliJ Autocorrect strikes again! Good catch. --- 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 #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/730#discussion_r135625773
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
---
@@ -415,9 +413,8 @@ public Result toCommandResult() {
toCommandResult_isPut(section, table);
} else if (isRemove()) {
toCommandResult_isRemove(section, table);
-} else if (isSelect()) {
- // its moved to its separate method
}
+// isSelect() moved to a separate method
--- End diff --
It was a little intentional to leave it. In the context, I couldn't tell
if it was actually helpful to explicitly comment that "No, we didn't forget
about `isSelect()`", but I also didn't investigate too closely to see why
`toCommandResult()` necessarily won't git on a select command.
---
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 #734: GEODE-3508: Remove unused internal deprecated class...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/734 --- 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 #745: GEODE-3436: Restore reverted GFSH command refactori...
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 = 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 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 n
[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 <emilyyeh1...@gmail.com> Date: 2017-08-22T17:57:05Z GEODE-3436: Restore refactoring of MemberCommands * See initial commit GEODE-3264 (d27f8b956de7d9c5d95ebdc68dfc67ee8b2d7b51) commit 44cc0e8bb7927e61bb3e7613612703518fc87db6 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T18:52:14Z GEODE-3436: Restore refactoring of DurableClientCommands * See initial commit GEODE-3259 (440c87f81fab96f9ce38a2d53ded75e5fe8390d7) commit 1e9eea1f1b2d2c620679283aaf6e6c5ac12e1728 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-04T17:47:48Z GEODE-3436: Restore refactoring of PDXCommands * See initial commit GEODE-3266 (67185abcdd68b908dea6888cb94286b8aa9ea49f) commit 691700c2e51aef5b18ba5feec711bddf5ae9a05e Author: YehEmily <emilyyeh1...@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 <emilyyeh1...@gmail.com> Date: 2017-08-07T21:47:15Z GEODE-3436: Restore refactoring of QueueCommands * See initial commit GEODE-3267 (fd47ed660168864a6f81b2a4cd7dbceebc99a282) commit 7594ebdd5e593179f0b33a94c30c7d5949fabf96 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-24T22:42:12Z GEODE-3436: Restore refactoring of GfshHelpCommands * See initial commit GEODE-3261 (cf91426692349d0c81ce77394935576d9cc336e8) commit 5daeb011cbeebfa2a71c225ed3dc3abc2047e3dd Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-24T23:11:13Z GEODE-3436: Restore refactoring of CreateAlterDestroyRegionCommands * See initial commit GEODE-3255 (756efe77c86bb03ac9984655e7bd040659e85890) commit a1823d323c26adebacf7536448e2cadf054d9528 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-07-26T18:07:09Z GEODE-3436: Restore refactoring of ConfigCommands * See initial commit GEODE-3254 (97c4e9a59f17c7bc914e39dd048b0a4cd96293c4) commit 78ec3cb687313799a911ed3d1ca6e560cb358de8 Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-03T16:00:08Z GEODE-3436: Restore refactoring of DiskStoreCommands * See initial commit GEODE-3258 (5d6cad7755ec3c4fe931e3d0f8e89fb181038543) commit 1ecbd6fa08343a8de50bf30839c9ef47c02e73aa Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T19:35:14Z GEODE-3436: Restore refactoring of IndexCommands * See initial commit GEODE-3262 (ed293e817e547fb5ecd399bf4ba10d694af51e0a) commit b562bdafc46045bd2679c233cf591549fb016d8b Author: YehEmily <emilyyeh1...@gmail.com> Date: 2017-08-07T19:56:17Z GEODE-3436: Restore refactoring of Ref
[GitHub] geode issue #734: GEODE-3508: Remove unused internal deprecated classes.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/734 Precheckin 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #731: GEODE-2842: Removed redundant default annotation parameter...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/731 Precheckin green, with some flakiness observed in `org.apache.geode.internal.cache.ha.Bug48571DUnitTest`. --- 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 #734: GEODE-3508: Remove unused internal deprecated class...
Github user PurelyApplied commented on a diff in the pull request: https://github.com/apache/geode/pull/734#discussion_r134881817 --- Diff: geode-core/src/main/java/org/apache/geode/internal/process/ClusterConfigurationNotAvailableException.java --- @@ -1,33 +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.internal.process; - -/** - * Exception thrown during server startup when it requests the locators for shared configuration and - * does not receive it. - * - * @since GemFire 8.0 - * @deprecated Please use --- End diff -- Looks like `@deprecated` wants to be one, too. Better use those backticks. I definitely misread those annotations, yes. I should get in the habit of checking the `git blame` to be sure about the deprecation date. Other than the reason you mentioned -- possibly using it during a rolling upgrade -- is there a good reason to deprecate an internal class instead of simply removing it? It was my understanding that internal packages didn't hold any promise of API stability. Or could there be a backwards-compatibility issue that I'm not seeing? --- 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 #734: GEODE-3508: Remove unused internal deprecated class...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/734 GEODE-3508: Remove unused internal deprecated classes. * Update ClusterConfigurationNotAvailableException to extend Exception directly, rather than the deprecated class. -- 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? - [n/a] Have you written or updated unit tests to verify your changes? - [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-3508 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/734.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 #734 commit 86e6daf53fa1660ea4744415a3d9f613f4b5a3f5 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-23T17:49:32Z GEODE-3508: Remove three unused internal deprecated classes. * Update ClusterConfigurationNotAvailableException to extend Exception directly, rather than the deprecated class. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #730: GEODE-3472: Remove a great deal of commented-out co...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/730#discussion_r134618712
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/AbstractResultData.java
---
@@ -147,11 +147,10 @@ public ResultData addAsFile(String fileName, String
fileContents, String message
public ResultData addAsFile(String fileName, byte[] data, int fileType,
String message,
boolean addTimeStampToName) {
-byte[] bytes = data;
--- End diff --
I might by projecting my Python and C expectations into my relatively new
Java experience, but I thought this assignment wouldn't change the mutability
of `data`. Wouldn't this need `byte[] bytes = data.clone();` to actually
provided immutability to `data`? Or is Java more defensive than I realized?
---
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 #731: GEODE-2842: Removed redundant default annotation pa...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/731 GEODE-2842: Removed redundant default annotation parameter values. Driven by IntelliJ inspection, I have extended the reach of this ticket to include any annotation, not only `@CliOption`. 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? - [n/a] Have you written or updated unit tests to verify your changes? - [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-2842 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/731.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 #731 commit 2f27ae65508935fcf3b4145f2412b450d65019f4 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-22T21:20:12Z GEODE-2842: Removed redundant default annotation parameter values. --- 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 #730: GEODE-3472: Remove a great deal of commented-out code.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/730 Precheckin green after merge with `be4551191a17b9f50169f375798785ab5373e975`. Subsequently rebased again with minor merge conflict resolved. --- 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 #730: GEODE-3472: Remove a great deal of commented-out co...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/730 GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, 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: - [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? - [n/a] Have you written or updated unit tests to verify your changes? [n/a: Non-functional changes.] - [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-3472 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/730.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 #730 commit 9fb4414b363e9bbc584b67456158ad3f880e269b Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-08-18T20:59:44Z GEODE-3472: Remove a great deal of commented-out code. * Correct imports in touched files * Correct several misspellings * Removed redundant modifier and specifiers * Corrected modifier orderings to adhere to style standard * Removed several //TODO comments * Removed "throws CheckedException" from methods that no longer throw the checked exception. * Minor refactorings: iterator loops to for-each, simplified conditionals, 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 #699: GEODE-3413: overhaul launcher and process classes a...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/699#discussion_r132307061
--- Diff:
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -157,7 +162,7 @@ protected static Properties loadGemFireProperties(final
URL url) {
if (url != null) {
try {
properties.load(new FileReader(new File(url.toURI(;
- } catch (Exception e) {
+ } catch (Exception ignored) {
--- End diff --
Are we really ignoring the exception if we're doing something in response
to it?
Ditto lines 272, 438.
---
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 #699: GEODE-3413: overhaul launcher and process classes a...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/699#discussion_r132312668
--- Diff:
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -1848,8 +1884,7 @@ public LocatorLauncher build() {
private final String name;
Command(final String name, final String... options) {
- assert !StringUtils
- .isBlank(name) : "The name of the locator launcher command must
be specified!";
+ assert !isBlank(name) : "The name of the locator launcher command
must be specified!";
--- End diff --
As long as we're touching this, we could consider using `isNotBlank` for
better 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 pull request #699: GEODE-3413: overhaul launcher and process classes a...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/699#discussion_r132312583
--- Diff:
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -502,7 +507,7 @@ protected static Integer identifyPid() {
}
}
-// TODO refactor the logic in this method into a DateTimeFormatUtils
class
+// consider refactoring the logic in this method into a
DateTimeFormatUtils class
--- End diff --
I'd remove this and the line 510 comments altogether. If the suggestion
has value, we should open a ticket for it.
Likewise `LocatorLauncher:382`, `ServerLauncher:523`
---
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 #699: GEODE-3413: overhaul launcher and process classes a...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/699#discussion_r132311304
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/process/MBeanProcessController.java
---
@@ -228,9 +230,9 @@ private void disconnect() {
* @throws PidUnavailableException if parsing the pid from the
RuntimeMXBean name fails
*/
boolean checkPidMatches() throws IllegalStateException, IOException,
PidUnavailableException {
--- End diff --
This appears to be unused? The JavaDoc has it as "NOT USED EXCEPT IN
TEST." This is perhaps no longer true?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...
Github user PurelyApplied commented on a diff in the pull request: https://github.com/apache/geode/pull/699#discussion_r132306579 --- Diff: geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java --- @@ -31,7 +31,7 @@ import org.apache.geode.internal.admin.remote.DistributionLocatorId; import org.apache.geode.internal.i18n.LocalizedStrings; import org.apache.geode.internal.logging.LogService; -import org.apache.geode.internal.process.ClusterConfigurationNotAvailableException; +import org.apache.geode.config.internal.ClusterConfigurationNotAvailableException; --- End diff -- Classic Pat nitpick: don't forget to reorder / optimize your imports to go `statics`, `java`, `javax`, (other), `org.apache.geode`. This applies to the following files: ``` geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java geode-core/src/main/java/org/apache/geode/internal/process/LocalProcessLauncher.java geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java ``` --- 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 PurelyApplied commented on a diff in the pull request: https://github.com/apache/geode/pull/690#discussion_r131790263 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java --- @@ -0,0 +1,197 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.management.internal.cli.commands; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicReference; + +import org.springframework.shell.core.annotation.CliCommand; +import org.springframework.shell.core.annotation.CliOption; + +import org.apache.geode.cache.Cache; --- End diff -- My eternal nitpicking: This import appears to be unused. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #691: GEODE-3260: Refactoring FunctionCommands
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/691 +1. A far-from-critical nitpick: there are several places in `ExecuteFunctionCommand` and one in `DestroyFunctionCommand` that are of the form ``` result = ResultBuilder.[...]; return result; ``` In other instances, the result is returned directly ``` return ResultBuilder.[...]; ``` I'd change those few assign-and-immediately-return to just return the object directly. You might then be able to delete the declaration of the variable as well. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/693#discussion_r131775864
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeRegionCommand.java
---
@@ -333,9 +247,8 @@ private void
writeCommonAttributesToTable(TabularResultData table, String attrib
}
}
- private boolean writeFixedPartitionAttributesToTable(TabularResultData
table,
- String attributeType, List fpaList,
String member,
- boolean isMemberNameAdded) {
+ private void writeFixedPartitionAttributesToTable(TabularResultData
table,
--- End diff --
In this function, a couple of low-hanging cleanups could be:
(1) Change the big `if` to a guard clause
```
if (fpaList == null) {
return;
}
```
(2) Change the iterated `while` loop to a `for:` loop.
Also, both of these can be cleanly easily handled by your IDE (although you
might need to move the `Iterator fpaIter =
fpaList.iterator();` next to the `while` first).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/693#discussion_r131776451
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeRegionCommand.java
---
@@ -359,7 +272,7 @@ private boolean
writeFixedPartitionAttributesToTable(TabularResultData table,
fpaBuilder.append(fpa.getNumBuckets());
if (!isTypeAdded) {
- type = attributeType;
+ type = "";
--- End diff --
I agree with the removal of a constant parameter, but in this case, we're
not distinguishing between `type = "";` here vs `type = blank; // = ""` a few
lines below.
These could be removed altogether, with `""` passed directly to the
`writeAttributeToTable` call at the end of this method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/693#discussion_r131783322
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
---
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
ANY KIND, either express
+ * or implied. See the License for the specific language governing
permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import java.util.TreeSet;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.execute.FunctionInvocationTargetException;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.domain.RegionInformation;
+import
org.apache.geode.management.internal.cli.functions.GetRegionsFunction;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.TabularResultData;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class ListRegionCommand implements GfshCommand {
+ private static final GetRegionsFunction getRegionsFunction = new
GetRegionsFunction();
+
+ @CliCommand(value = {CliStrings.LIST_REGION}, help =
CliStrings.LIST_REGION__HELP)
+ @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION)
+ @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+ operation = ResourcePermission.Operation.READ)
+ public Result listRegion(
+ @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
+ optionContext = ConverterHint.MEMBERGROUP,
+ help = CliStrings.LIST_REGION__GROUP__HELP) String[] group,
+ @CliOption(key = {CliStrings.MEMBER, CliStrings.MEMBERS},
+ optionContext = ConverterHint.MEMBERIDNAME,
+ help = CliStrings.LIST_REGION__MEMBER__HELP) String[]
memberNameOrId) {
+Result result = null;
+try {
+ Set regionInfoSet = new LinkedHashSet<>();
+ ResultCollector rc;
+ Set targetMembers = CliUtil.findMembers(group,
memberNameOrId);
+
+ if (targetMembers.isEmpty()) {
+return
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+ }
+
+ TabularResultData resultData =
ResultBuilder.createTabularResultData();
+ rc = CliUtil.executeFunction(getRegionsFunction, null,
targetMembers);
+ ArrayList resultList = (ArrayList) rc.getResult();
+
+ if (resultList != null) {
--- End diff --
It is arguably beyond the scope of this ticket to refactor this, but this
sort of nesting makes my skin crawl.
Dropping line 72 because `instanceof` requires non-`null`.
([Docs](http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.20.2))
I'm pretty confident this is equivalent, but it would certainly warrant a
new precheckin.
```
Set regionInfoSet;
if (resultList != null) {
regionInfoSet = resultList.stream()
.filter(resultObj -> resultObj instanceof Object[])
.map(resultObj -> (Object[]) resultObj)
.flatMap(Arrays::stream)
.filter(regionInfo -> regionInfo instanceof RegionInformation)
.map(regionInfo -> (RegionInformation) regionInfo)
.collect(Collectors.toCollection(LinkedHashSet::new));
// ...
} // ends if (result != null)
[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands
Github user PurelyApplied commented on a diff in the pull request: https://github.com/apache/geode/pull/693#discussion_r131775100 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java --- @@ -26,11 +26,12 @@ import org.springframework.web.context.request.WebRequest; /** - * The RegionCommands class implements GemFire Management REST API web service endpoints for the - * Gfsh Region Commands. + * The ListRegionCommand and DescribeRegionCommand classes implement GemFire Management REST API web --- End diff -- My eternal nitpick: make sure you optimize / reorder imports in the files you touch, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #652: Geode-2971: Introduce ExitCode to resolve inconsistency of...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/652 Precheckin came back fully green. PR squashed and rebased. --- 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 #666: GEODE-3329: Changed logging output of modify_war script
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/666 Thanks for the quick turn around on this! +0.5, since I don't really know this corner of the code, but it looks good to me! --- 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 #660: GEODE-2924 Revise authorization permissions
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/660 +1. For future reference, tickets GEODE-2920 through GEODE-2926 were committed under the overarching ticket [GEODE-2919](https://issues.apache.org/jira/browse/GEODE-2919) in commit `451d12e8...`. --- 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 #652: Geode-2971: Introduce ExitCode to resolve inconsistency of...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/652 @jaredjstewart I don't know why I didn't think to put my questions here instead of imbedding them in my code, just to be removed later. I clearly wasn't on my A-game. 1. In my addition to `GfshRule` to extend the classpath, it was unclear to me whether using `ProcessBuilder`'s `environment` method was properly OS agnostic. (Looking at it again, it also appears that I left variable names `p` and `m` in place instead of more meaningful names. Ugh.) 2. In the `GfshExitCodeStatuscommandsDUnitTest`, I couldn't decide if the utility functions generating the actual `gfsh` command would look better inlined into the test function or as a function call. 3. Akin to the "Should `gfsh help` actually be returning non-zero", existing behavior is to exit with a zero when gfsh is interrupted (`Launcher.java:227`). Is an interruption assumed to be intentional and we should return a zero, or should that actually be returning a nonzero? --- 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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/652#discussion_r129656012
--- Diff: geode-core/src/main/java/org/apache/geode/internal/ExitCode.java
---
@@ -0,0 +1,61 @@
+/*
+ * 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.internal;
+
+import java.util.Arrays;
+
+import org.springframework.shell.core.ExitShellRequest;
+
+
+public enum ExitCode {
+
+ // The choice of redundant values here is to be consistent with existing
behavior
+ // while allowing for future extensibility. JVM_TERMINATED_EXIT(99)
exists for coverage of
+ // Spring's ExitShellRequest values in fromSpring.
+ NORMAL(0),
+ FATAL(1),
+ DEPENDENCY_GRAPH_FAILURE(-1),
+ COULD_NOT_EXECUTE_COMMAND(1),
+ INVALID_COMMAND(1),
--- End diff --
I agree in principle, but was hesitant to alter existing behavior for
system exit calls. I didn't gain any traction in my message to the dev list
about using any exit codes other than `0` and `1`, but at the same time felt
that some distinction was warranted in the different places that `System.exit`
had been called.
Then again, I just finished ripping out dead code that had been in place
"for future development," so perhaps I should keep it simple for now and not do
the work of future extensibility that may or may not ever arrive.
---
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 #653: GEODE-3290: Removed effectively-dead classes Filter...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/653 GEODE-3290: Removed effectively-dead classes FilterChain, et al Removed `FilterChain`, `LocalFilterChain`, and `RemoteFilterChain`. Minor cleanup of touched files. Precheckin running. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-3290 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/653.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 #653 commit 2bd128e784de07b558a5a438b1de8eea6cb9fd24 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-24T21:00:43Z GEODE-3290: Removed effectively-dead classes FilterChain, LocalFilterChain, and RemoteFilterChain. --- 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 #652: Geode-2971: Introduce ExitCode to resolve inconsistency of...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/652 Precheckin running. --- 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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/652 Geode-2971: Introduce ExitCode to resolve inconsistency of shell exit values There are a few sections containing `// TODO PSRhomberg`. Most are there to mark a specific request for feedback, and two are there to explain a change that counterintuitively respects existing behavior. - [ ] Have you removed your `TODO` comments? - [ ] Is your initial contribution a single, squashed commit? You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2971 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/652.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 #652 commit f8b1685dcecd023bbaa17fec8024dc32af648f33 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-20T19:36:36Z Added ExitCode to handle all System.exit calls. * Significant refactoring of Launcher.java and its use of exit states. commit 60a1bcd93fe3c4e67d0735b3fce6c9d3bed0d9f6 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-20T19:40:34Z All System.exit calls replaced by an appropriate call of ExitCode.[code].doSystemExit(); commit 57df2484d9a1fc6001dc5ef12522a24483f126f6 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-20T22:54:06Z Added AcceptanceTest for gfsh member status commands commit c82b0115099b98bbbfca1eb6e3539a82e6fe054b Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-20T19:42:18Z Correction of inconsistency in exit status of member status commands. commit 92da642dcf1b5039def0ecf3d0f317c652a48515 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-21T22:05:48Z Removed dead code and many needless asides from touched file SystemFailure.java commit 830d3ee0af16c765b99271e231377439cd659b26 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-07-21T22:15:27Z Removed redundant declarations and much dead code from touched file StatArchiveReader.java --- 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 #646: GEODE-3213: Refactor ProtoBuf handler flow
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/646 If you are using IntelliJ as your IDE, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. If you are using Eclipse, please import both style guidefiles, also located in `geode/etc/`. After having done so, please optimize the imports to the correct order and to eliminate the use of the wildcard imports from the 25 offending files this pull request touches. --- 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 #647: GEODE-3271: Refactor WanCommands
Github user PurelyApplied commented on the issue:
https://github.com/apache/geode/pull/647
+1 as it stands.
Unimportant nitpicks, rambling observations, and "it could be even better
if...":
* `punePort` appears in these tests a lot. I have no idea what `pune` is
supposed to mean. Maybe `dsIdPort` would be better, glancing at the
implementation of `createFirstLocatorWithDSId`?
* There are five instances in `WANCommandsTestBase` that has a loop like
this
```
for (GatewaySender s : senders) {
if (s.getId().equals(senderId)) {
sender = (AbstractGatewaySender) s;
break;
}
}
```
that my hatred of `break` as function control thinks it would read a lot
better as
```
AbstractGatewaySender sender =
(AbstractGatewaySender) senders.stream().filter(s ->
s.getId().equalsIgnoreCase(senderId)).findFirst().orElse(null);
```
* I'd take the Inspection-suggested change in `WANCommandBaseTest.java:489`
to turn the string `indexOf(...) != -1` to be `contains(...)`.
---
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 #601: GEODE-3117: fix Gateway authentication with legacy securit...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/601 If you are using IntelliJ as your IDE, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. After having done so, please optimize the imports to the correct order and to eliminate the use of the wildcard imports. --- 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 #603: GEODE-3128: Changed the batch size to 1000 when creating A...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/603 If you are using IntelliJ as your IDE and have not already done so, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. After having done so, please optimize the imports to the correct order of imports in `geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java`. --- 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 #609: GEODE-2886 : sent IllegalStateException instead of throwin...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/609 If you are using IntelliJ as your IDE, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. After having done so, please optimize the imports to the correct order and to eliminate the use of the wildcard imports. --- 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 #612: GEODE-3121: ensure that the protobuf protocol works over S...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/612 If you are using IntelliJ as your IDE, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. After having done so, please optimize the imports to the correct order and to eliminate the use of the wildcard imports. --- 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 #613: GEODE-3151: Internal Region Registration in JMX as per con...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/613 If you are using IntelliJ as your IDE, please remember to update your style file to that located in `geode/etc/intellij-java-modified-google-style.xml` to be consistent with Geode's established style guide. This file was updated June 13 with commit `a561bd12` to be consistent with documented expectations. After having done so, please optimize the imports to the correct order and to eliminate the use of the wildcard imports. --- 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 #601: GEODE-3117: fix Gateway authentication with legacy securit...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/601 +1. Huzzah for more test coverage. --- 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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/596 --- 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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/596 Excepting one flaky test, precheckin is green through `14298a`. Precheckin is currently very unhappy, though, and starting new test runs is not going well. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/596#discussion_r123832520
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ClientCommands.java
---
@@ -109,12 +107,10 @@ public Result listClient() {
}
String memberSeparator = "; ";
- Iterator<Entry<String, List>> it =
clientServerMap.entrySet().iterator();
- while (it.hasNext()) {
-Map.Entry<String, List> pairs = (Map.Entry<String,
List>) it.next();
-String client = (String) pairs.getKey();
-List servers = (List) pairs.getValue();
+ for (Entry<String, List> pairs : clientServerMap.entrySet())
{
+String client = pairs.getKey();
+List servers = pairs.getValue();
StringBuilder serverListForClient = new StringBuilder();
--- End diff --
Added in commit `b4bb08b`.
---
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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/596#discussion_r123832527
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java
---
@@ -130,31 +125,8 @@ public Result executeFunction(
return result;
}
- if (onRegion != null && onMember != null && onGroups != null) {
-ErrorResultData errorResultData =
-
ResultBuilder.createErrorResultData().setErrorCode(ResultBuilder.ERRORCODE_DEFAULT)
-.addLine(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
-result = ResultBuilder.buildResult(errorResultData);
-return result;
- } else if (onRegion != null && onMember != null) {
-ErrorResultData errorResultData =
-
ResultBuilder.createErrorResultData().setErrorCode(ResultBuilder.ERRORCODE_DEFAULT)
-.addLine(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
-result = ResultBuilder.buildResult(errorResultData);
-return result;
- } else if (onMember != null && onGroups != null) {
-ErrorResultData errorResultData =
-
ResultBuilder.createErrorResultData().setErrorCode(ResultBuilder.ERRORCODE_DEFAULT)
-.addLine(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
-result = ResultBuilder.buildResult(errorResultData);
-return result;
- } else if (onRegion != null && onGroups != null) {
-ErrorResultData errorResultData =
-
ResultBuilder.createErrorResultData().setErrorCode(ResultBuilder.ERRORCODE_DEFAULT)
-.addLine(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
-result = ResultBuilder.buildResult(errorResultData);
-return result;
- } else if (onRegion != null && onMember != null && onGroups != null)
{
+ if (isMoreThanOneIsTrue(onRegion != null, onMember != null, onGroups
!= null)) {
--- End diff --
Good call. Added in commit `b4bb08b`.
---
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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/596#discussion_r123830383
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBean.java
---
@@ -148,7 +148,12 @@ public long getInitialImageTime() {
@Override
public int getInitialImagesInProgres() {
--- End diff --
This is our internal implementation of
`GEODE/geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java`.
I was hesitant to remove the typo'd version from the public-facing interface
without at least one version with it `@Deprecated`.
---
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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/596#discussion_r123829882
--- Diff:
geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
---
@@ -57,7 +56,48 @@ public void setUp() throws Exception {
}
@Test
- @ConnectionConfiguration(user = "data-admin", password = "1234567")
+ @ConnectionConfiguration(user = "clusterRead", password = "clusterRead")
+ public void testClusterReadAccess() throws Exception {
+assertThatThrownBy(() ->
bean.flush()).hasMessageContaining(TestCommand.diskManage.toString());
--- End diff --
Thank you regex. Done and done in commit `97de2f3`.
For what it's worth / future reference, `s/\(\) ->
(\w+)\.(\w+)\(\)\)/$1::$2\)` would make 3,232 test replacements, but hits no
production files, so that's nice. Maybe something to add to spotless down the
line.
---
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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/596 Precheckin running. --- 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 #596: GEODE-2920 - GEODE-2925: Finer Grained Security
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/596 GEODE-2920 - GEODE-2925: Finer Grained Security Due to the size of this commit and for your convenience of review, I have not yet squashed my commits. Do note that I have not individually tested each individual commit for stability and each internal commit is meant only for ease of review. The commit message to be included in the final, squashed version of this PR is present in the `8fe19ca`... commit, and reproduced below. TODO: [ ] Is your initial contribution a single, squashed commit? - This commit represents the actual Finer Grained Security changes. GEODE-2920 - GEODE-2925: Migration of security from DATA:MANAGE * DATA:MANAGE -> CLUSTER:MANAGE * * configure pdx * import cluster-configuration * LockServiceMXBean.becomeLockGrantor * * DATA:MANAGE -> CLUSTER:MANAGE:DISK * * compact disk-store * create disk-store * destroy disk-store * revoke missing-disk-store * DiskStoreMXBean.forceRoll * DiskStoreMXBean.forceCompaction * DiskStoreMXBean.flush * DiskStoreMXBean.setDiskUsageWarningPercentage * DiskStoreMXBean.setDiskUsageCriticalPercentage * DistributedSystemMXBean.revokeMissingDiskStores * MemberMXBean.compactAllDistStores * * DATA:MANAGE -> CLUSTER:MANAGE:GATEWAY * * create gateway-receiver * create gateway-sender * destroy gateway-sender * load-balance gateway-sender * pause gateway-sender * resume gateway-sender * start gateway-receiver * start gateway-sender * stop gateway-receiver * stop gateway-sender * GatewayReceiverMXBean.start * GatewayReceiverMXBean.stop * GatewaySenderMXBean.start * GatewaySenderMXBean.stop * GatewaySenderMXBean.pause * GatewaySenderMXBean.resume * GatewaySenderMXBean.rebalance * * DATA:MANAGE -> CLUSTER:MANAGE:JAR * * create async-event-queue (Requires CLUSTER:WRITE:DISK if persistent) * destroy function * undeploy * * DATA:MANAGE -> CLUSTER:MANAGE:QUERY * * clear defined indexes * close durable-client * close durable-cq * create defined indexes * stop continuous-query * CacheServerMXBean.closeAllContinuousQuery * CacheServerMXBean.closeContinuousQuery * DistributedSystemMXBean.setQueryResultSetLimit * DistributedSystemMXBean.setQueryCollectionsDepth * * DATA:READ -> CLUSTER:READ * * list region * * DATA:MANAGE -> [None] * * pdx rename * * DATA:READ -> DATA:READ and CLUSTER:WRITE:DISK * * backup disk-store * DistributedSystemMXBean.backupAllMembers * * DATA:MANAGE:RegionName -> CLUSTER:MANAGE:QUERY * * create index * create lucene index (also requires CLUSTER:WRITE:DISK) * define index * destroy lucene index * * DATA:MANAGE, DATA:WRITE, CLUSTER:MANAGE, and CLUSTER:WRITE -> CLUSTER:MANAGE:JAR * * deploy * * DATA:MANAGE or DATA:MANAGE:RegionName -> CLUSTER:MANAGE:QUERY * * destroy index * * CLUSTER:READ -> CLUSTER:READ:QUERY * * describe lucene index * list index * list lucene indexes * * DATA:WRITE -> DATA:READ:Region * * search lucene index * * DATA:MANAGE -> DATA:MANAGE and CLUSTER:WRITE:DISK if persistent * * create region You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2924 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/596.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 #596 commit 8b14112094fd49bfc7638cc952f8d322d5bd50e7 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-06-21T17:51:32Z For ease of viewing, this commit covers all necessary imports. commit 8fe19ca6ff3e51aed601537afb8e23c3e85569a1 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-06-21T18:59:13Z This commit represents the actual Finer Grained Security changes. GEODE-2920 - GEODE-2925: Migration of security from DATA:MANAGE * DATA:MANAGE -> CLUSTER:MANAGE * * configure pdx * import cluster-configuration * LockServiceMXBean.becomeLockGrantor * * DATA:MANAGE -> CLUSTER:MANAGE:DISK * * compact disk-store * create disk-store * destroy disk-store * revoke missing-disk-store * DiskStoreMXBean.forceRoll * DiskStoreMXBean.forceCompaction * DiskStoreMXBean.flush * DiskStoreMXBean.setDiskUsageWarningPercentage * DiskStoreMXBean.setDiskUsageCriticalPercentage * DistributedSystemMXBean.revokeMissingDiskStores * MemberMXBean.compactAll
[GitHub] geode pull request #576: Geode 2920, 2921, 2922, 2924
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/576 --- 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 #576: Geode 2920, 2921, 2922, 2924
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/576 Closing this PR to reopen the finalized form. --- 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 PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/591#discussion_r122849064
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
---
@@ -941,17 +940,16 @@ private SelectResults
prepareEmptyResultSet(ExecutionContext context, boolean ig
SelectResults results;
if (this.distinct || !this.count) {
--- End diff --
This could stand to be flattened as well, although it will be more
difficult that the above. I could go either way on this chunk.
---
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 PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/591#discussion_r122848112
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
---
@@ -1415,11 +1413,7 @@ public boolean evaluateCq(ExecutionContext context)
throws FunctionDomainExcepti
// add UNDEFINED to results only for NOT EQUALS queries
if (this.whereClause.getType() == COMPARISON) {
int operator = ((Filter) this.whereClause).getOperator();
- if ((operator != TOK_NE && operator != TOK_NE_ALT)) {
-return false;
- } else {
-return true;
- }
+ return !(operator != TOK_NE && operator != TOK_NE_ALT);
--- End diff --
That's a lot of negatives. `return operator == TOK_NE || operator ==
TOK_NE_ALT` read better, in my opinion.
---
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 #592: GEODE-2707: Removing TXLockToken
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/592 After the fix Ken pointed out, `++n;` --- 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 PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/578#discussion_r121799720
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java ---
@@ -1909,10 +1900,7 @@ private static boolean needsSysDir(String cmd) {
if (cmd.equalsIgnoreCase("version")) {
return false;
}
-if (cmd.equalsIgnoreCase("help")) {
- return false;
-}
-return true;
+return !cmd.equalsIgnoreCase("help");
--- End diff --
If you want to collapse this block, I think this might read a little better.
```
if (cmd.equalsIgnoreCase("stats")
|| cmd.equalsIgnoreCase("merge-logs")
|| cmd.equalsIgnoreCase("version")
|| cmd.equalsIgnoreCase("help")) {
return false;
}
return true;
```
I guess you could collapse that to a one-liner, but I don't think that
would read as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #578: GEODE-1958: Removing/deprecating PasswordUtil
Github user PurelyApplied commented on a diff in the pull request:
https://github.com/apache/geode/pull/578#discussion_r121798399
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/util/PasswordUtil.java ---
@@ -44,72 +42,29 @@
private static byte[] init = "string".getBytes();
/**
- * Encrypts a password string
- *
- * @param password String to be encrypted.
- * @return String encrypted String
- */
- public static String encrypt(String password) {
-return encrypt(password, true);
- }
-
- /**
- *
- * @param password String to be encrypted
- * @param echo if true prints result to system.out
- * @return String encrypted String
+ * Decrypts an encrypted password string.
+ *
+ * @param password String to be decrypted
+ * @return String decrypted String
*/
- public static String encrypt(String password, boolean echo) {
-String encryptedString = null;
+ @Deprecated
+ public static String decrypt(String password) {
+String toDecrypt;
+if (password.startsWith("encrypted(") && password.endsWith(")"))
--- End diff --
I don't think it gets caught by spotless, but officially we prefer to wrap
every `if` and `else` in curly braces, even when they're just one-liners.
---
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 #576: Geode 2920, 2921, 2922, 2924
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/576 Geode 2920, 2921, 2922, 2924 This PR is intended for initial review, not to be actively considered for acceptance. Sufficient testing is still required and `develop` shifted enough today that I need to rebase again. This currently addresses most of the finer-grained security as listed [here](https://cwiki.apache.org/confluence/display/GEODE/Finer+grained+security). As yet unaddressed remains: * alter disk-store * disconnect * echo * encrypt password * execute function * import cluster-configuration * DistributedSystemMXBean.backupAllMembers Additional points of concern: * compact offline-disk-store is not actually updated? [None -> None] * Should destroy region also require CLUSTER:WRITE:DISK if persistent? * These do not exist: *GatewayReceiverMXBean.pauseDATA:MANAGE CLUSTER:MANAGE:GATEWAY *GatewayReceiverMXBean.rebalanceDATA:MANAGE CLUSTER:MANAGE:GATEWAY *GatewayReceiverMXBean.resume DATA:MANAGE CLUSTER:MANAGE:GATEWAY * `execute function` is listed twice, with different "original" permissions. Do these refer to different executions? For your convenience, the functional diff is the second commit. The first commit only resolves `imports`, and the third commit is a general cleanup of touched files. == Current commit log: GEODE-292*: Migration of security from DATA:MANAGE * DATA:MANAGE -> CLUSTER:MANAGE * * configure pdx * LockServiceMXBean.becomeLockGrantor * * DATA:MANAGE -> CLUSTER:MANAGE:DISK * * compact disk-store * create disk-store * destroy disk-store * revoke missing-disk-store * DiskStoreMXBean.forceRoll * DiskStoreMXBean.forceCompaction * DiskStoreMXBean.flush * DiskStoreMXBean.setDiskUsageWarningPercentage * DiskStoreMXBean.setDiskUsageCriticalPercentage * DistributedSystemMXBean.revokeMissingDiskStores * MemberMXBean.compactAllDistStores * * DATA:MANAGE -> CLUSTER:MANAGE:GATEWAY * * create gateway-receiver * create gateway-sender * destroy gateway-sender * load-balance gateway-sender * pause gateway-sender * resume gateway-sender * start gateway-receiver * start gateway-sender * stop gateway-receiver * stop gateway-sender * GatewayReceiverMXBean.start * GatewayReceiverMXBean.stop * GatewaySenderMXBean.start * GatewaySenderMXBean.stop * GatewaySenderMXBean.pause * GatewaySenderMXBean.resume * GatewaySenderMXBean.rebalance * * DATA:MANAGE -> CLUSTER:MANAGE:JAR * * create async-event-queue (Requires CLUSTER:WRITE:DISK if persistent) * destroy function * undeploy * * DATA:MANAGE -> CLUSTER:MANAGE:QUERY * * clear defined indexes * close durable-client * close durable-cq * create defined indexes * stop continuous-query * CacheServerMXBean.closeAllContinuousQuery * CacheServerMXBean.closeContinuousQuery * DistributedSystemMXBean.setQueryResultSetLimit * DistributedSystemMXBean.setQueryCollectionsDepth * * DATA:READ -> CLUSTER:READ * * list region * * DATA:MANAGE -> [None] * * pdx rename * * DATA:READ -> DATA:READ and CLUSTER:WRITE:DISK * * backup disk-store * * DATA:MANAGE:RegionName -> CLUSTER:MANAGE:QUERY * * create index * create lucene index (also requires CLUSTER:WRITE:DISK) * define index * destroy lucene index * * DATA:MANAGE, DATA:WRITE, CLUSTER:MANAGE, and CLUSTER:WRITE -> CLUSTER:MANAGE:JAR * * deploy * * DATA:MANAGE or DATA:MANAGE:RegionName -> CLUSTER:MANAGE:QUERY * * destroy index * * CLUSTER:READ -> CLUSTER:READ:QUERY * * describe lucene index * list lucene indexes * * DATA:WRITE -> DATA:READ:Region * * search lucene index * * DATA:MANAGE -> DATA:MANAGE and CLUSTER:WRITE:DISK if persistent * create region You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2924 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/576.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 #576 commit d
[GitHub] geode pull request #551: GEODE-2971: consistency in shell exit codes for sta...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/551 --- 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 #551: GEODE-2971: consistency in shell exit codes for status com...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/551 After significant testing redesign, this PR will be closed. The ticket will be resolved after additional `@Rule` resources are added that more accurately represent real `gfsh` usage. --- 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 #569: Resolve Eclipse/IntelliJ import order disagreement
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/569 Resolve Eclipse/IntelliJ import order disagreement The style guides contained in `etc/` are not consistent. This commit modifies `etc/intellij-java-google-style.xml` to adhere to the import order specified in `etc/eclipseOrganizeImports.importorder`. This seems a significant enough deviation from the original Google style guide to warrant a name change in the XML. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-import-order Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/569.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 #569 commit 41c8f7b1651914d61a428f6b8519a8277f2c414d Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-06-08T18:48:47Z Resolve Eclipse/IntelliJ import order disagreement in style files. The style files contained in `etc/` are not consistent between IDEs. This commit modifies `etc/intellij-java-google-style.xml` to adhere to the import order specified in `etc/eclipseOrganizeImports.importorder`. This seems a significant enough deviation from the original Google style guide to warrant a name change in the XML. --- 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 #551: GEODE-2971: consistency in shell exit codes for sta...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/551 GEODE-2971: consistency in shell exit codes for status commands. * Replaced all System.exit parameters with ShellExitCode references for extensibility * Added FakeLaunchers to Locator- and ServerStarterRules to spoof member status and to register the instance to the Launcher class. The spoofed status is of mixed reliability and only used when connected via jmx (via `--name`, not `--dir` or `--pid`). * Expanded starter rules to spawn PID files in working directory. * Made `getMemberMXBean` static, added public `isMemberMXBeanAvailable` to allow tests to delay tests until server is ready. * Code cleanup of touched files. Precheckin ran nearly-fully-green. One test failure which I suspect to be unrelated to these changes. This test passed cleanly when run with `gradlew -distributedTest.single=...`. Currently rerunning DUnits together to verify. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode gem-1370 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/551.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 #551 commit 91ee3d79102fafa9521a219aa8d3527bea519922 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-05-30T21:25:47Z GEODE-2971: consistency in shell exit codes for status commands. * Replaced all System.exit calls with ShellExitCode references for extensibility * Added FakeLaunchers to Locator- and ServerStarterRules to spoof member status made via jmx (not --pid or --dir options) * Expanded starter rules to spawn PID files * Made getMemberMXBean static, made public isMemberMXBeanAvailable to delay tests until server is ready. * Code cleanup of touched files. --- 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 #528: GEODE-1994, revisited: Removed guaranteed failures.
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/528 --- 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 #528: GEODE-1994, revisited: Removed guaranteed failures.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/528 Green across the board. --- 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 #528: GEODE-1994, revisited: Removed guaranteed failures.
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/528 Precheckin running... --- 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 #528: GEODE-1994, revisited: Removed guaranteed failures.
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/528 GEODE-1994, revisited: Removed guaranteed failures. Removed two uses of ServerLauncher.Builder.setMemberName that are guaranteed to throw under the changes introduced by GEODE-1994. With the previous update to StringUtils, ServerLauncher.Builder.setMemberName throws when its input is blank. These two uses are behind conditionals and only occur when the member name is blank, guaranteeing failure. This results in the `status server` and `stop server` commands functioning only when `--name` is used, but failing with `--dir` and `--pid`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-1994.revised Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/528.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 #528 commit 071780ed1aa2b12068f12291d032c30a930320ac Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-05-23T22:07:11Z GEODE-1994, revisited: Removed two references to ServerLauncher.setMemberName that are guaranteed to throw under the changes introduced by GEODE-1994. --- 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 #522: GEODE-2953: Imports optimized in every file with a ...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/522 --- 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 #522: GEODE-2953: Imports optimized in every file with a wildcar...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/522 Yeah, while I do firmly believe that there is value in good form and adherence to best practices, this PR ended up a lot bigger than I had initially expected. --- 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 #522: GEODE-2953: Imports optimized in every file with a wildcar...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/522 Please review the diff, or run a regex search matching `import .*\*;\n`. This regex hits 1024 occurrences in production and 2205 occurrences in test code. Some of these occurrences are commented out. Matching `^import .*\*;\n` hits 827 production and 2175 test occurrences. 1388 classes going against best practices is indeed alarming, but I do not believe it is incorrect. --- 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 #522: GEODE-2953: Imports optimized in every file with a ...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/522 GEODE-2953: Imports optimized in every file with a wildcard import. This is a purely semantic change. Imports have been optimized by my IDE. This may have the side-effect of changing some imports to static imports where applicable, as well as the ongoing disagreement between IntelliJ and Eclipse as to where `java` and `javax` imports belong. All the same, precheckin up and running. Tests and Legacy already back green. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2953 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/522.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 #522 commit fc6ceffec0931575c9150087d11e7083fe9481c5 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-05-19T18:18:47Z GEODE-2953: Imports optimized in every file with a wildcard import. --- 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 #521: GEODE-1994: Overhaul of internal.lang.StringUtils to exten...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/521 precheckin has cleared most tests. Waiting on DistributedTest to finish. :geode-core cleared, currently waiting on :geode-wan. I've seen some :geode-wan failures on a fresh develop (a lot of Connection Refused), but will reply to this thread tomorrow with the DUnit results. --- 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 #521: GEODE-1994: Overhaul of internal.lang.StringUtils t...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/521 GEODE-1994: Overhaul of internal.lang.StringUtils to extend and heavily use commons.lang.StringUtils * geode.internal.lang.StringUtils has been deprecated. In the interim, it has been heavily refactored and extends commons.lang.StringUtils. * * Renamed: * -- EMPTY_STRING -> EMPTY (inherited) * -- toUpperCase -> upperCase (inherited) * -- toLowerCase -> lowerCase (inherited) * -- padEnding-> rightPad (inherited) * * Removed: * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY * -- SPACES * -- UTF_8; rare usage replaced with raw string * -- concat; usage replaced with commons.lang.join, refactoring as necessary. * -- getLettersOnly * -- getSpaces * -- truncate * -- valueOf; usage refactored to use defaultString * * Refactored * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons. * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase. * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited. * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited. * * Code Cleanup: * -- Many uses of !isBlank -> isNotBlank * -- Changes suggested by Inspections on most touched files. * -- Explicit -> <> when type is inferable * -- while loops operating on iterators converted to for each loops * -- for loops operating on array indices converted to for each loops * -- Various string typos corrected. * -- isEmpty(s.trim()) -> isBlank(s) * -- s.trim().isEmpty() -> isEmpty(s) * -- Removed some instances of 'dead' code * -- Optimized imports in every touched file * * Qualitative Changes: * -- The following functions now throw an error when called with a null string input: * -- * LocatorLauncher.Builder.setMemberName * -- * ServerLauncher.Builder.setMemberName * -- * ServerLauncher.Builder.setHostnameForClients * -- (Unit tests added to capture these changes) * * Notes: * -- StringUtils.wraps may be inherited from Apache Commons when the dependency is updated. * -- AbstractLauncher.getMember has the documented behavior of returning null when both MemberName and ID are blank. Is this the best behavior for this method? You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-1994 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/521.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 #521 commit 327e1ae4b7b6e3503b0e295e6c883c387fa03b47 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-05-17T23:57:07Z GEODE-1994: Overhaul of internal.lang.StringUtils to extend and heavily use commons.lang.StringUtils * geode.internal.lang.StringUtils has been deprecated. In the interim, it has been heavily refactored and extends commons.lang.StringUtils. * * Renamed: * -- EMPTY_STRING -> EMPTY (inherited) * -- toUpperCase -> upperCase (inherited) * -- toLowerCase -> lowerCase (inherited) * -- padEnding-> rightPad (inherited) * * Removed: * -- EMPTY_STRING_ARRAY; usage replaced with commons.lang.ArrayUtils.EMPTY_STRING_ARRAY * -- SPACES * -- UTF_8; rare usage replaced with raw string * -- concat; usage replaced with commons.lang.join, refactoring as necessary. * -- getLettersOnly * -- getSpaces * -- truncate * -- valueOf; usage refactored to use defaultString * * Refactored * -- defaultIfBlank: previously relied on varargs and could return null. Usage refactored to allow inheritance from commons. * -- defaultString(s, EMPTY) refactored to use standard signature defaultString(s) for consistency throughout codebase. * -- isBlank: usage refactored to resolve discrepancies with commons.lang.isBlank, which is now inherited. * -- isEmpty: usage refactored to resolve discrepancies with commons.lang.isEmpty, which is now inherited. * * Code Cleanup: * -- Many uses of !isBlank -> isNotBlank * -- Changes suggested by Inspections on most touched files. * -- Explicit -> <> when type is inferable * -- while loops operating on iterators converted to for ea
[GitHub] geode issue #500: GEODE-2662: Gfsh displays field value on wrong line ...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/500 precheckin returns green across the board. --- 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 #500: GEODE-2662: Gfsh displays field value on wrong line...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/500 GEODE-2662: Gfsh displays field value on wrong line ... ... when receiving objects with missing fields. DataCommandResult.buildTable refactored to scan for all necessary fields and build rows, padding with MISSING_VALUE as necessary. Additionally, significant refactoring for readability. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2662 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/500.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 #500 --- 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 #435: GEODE-2738: Corrected spelling of "occured" to occurred
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/435 Re-rebased and conflict resolved. --- 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 #435: GEODE-2738: Corrected spelling of "occured" to occurred
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/435 Rebased and resoled conflicts. --- 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 #454: GEODE-2775: Corrected setting of Pulse SSL Manager ...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/454 GEODE-2775: Corrected setting of Pulse SSL Manager flag Flag now set from System properties instead of pulse.properties when running in embedded mode. Precheckin returned fully green. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2775 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/454.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 #454 commit 9748668b562963716cd8b6bb5d7aa87c6d40d414 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-04-12T18:39:11Z GEODE-2775: Corrected setting of Pulse SSL Manager flag from System properties instead of pulse.properties when running in embedded mode. --- 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 #438: GEODE-2725: export logs now honors --dir when connected vi...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/438 precheckin failed with many errors, most of which seem unrelated. - Three failures in `PartitionedRegionQueryEvaluatorTest` - 94 failures in `GlobalRegionOffHeapDUnitTest`, and of those, 92 failed with `Operation either timed out, was stopped or Locator does not exist.` Mismatch in golden file against these changes in help strings has been corrected. An additional precheckin has been 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 #445: GEODE-2767: Added DUnitTests to validate export log option...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/445 Precheckin comes back green across the board, excepting one flaky failure in `MemoryThresholdsOffHeapDUnitTest.testDRLoadRejection` --- 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 #446: GEODE-1274: Migration from PulseLogWriter to Log4j ...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/446 GEODE-1274: Migration from PulseLogWriter to Log4j standard and removal of associated classes. * To avoid dependency on geode-core, the pulse loggers are instantiated directly from LogManager, rather than canonical LogService (which itself extends LogManager). * Significant reduction of logging level state checks, relying on Log4j handling. * Significant reduction of string concatenation, relying on Log4j2 string substitutions. * Reduction of logging using an exception e.getMessage, favoring instead passing the exception itself for the stacktrace. * Multiple identical exception blocks collapsed. precheckin green on Test/IntegrationTest/Legacy. Previous runs green on Distributed, but current still running. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode bugfix/GEODE-1274 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/446.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 #446 commit 49f5f9f57768eaf005364149c212c20404f6d197 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-03-22T20:27:34Z GEODE-1274: Migration from PulseLogWriter to Log4j standard and removal of associated classes. To avoid dependency on geode-core, the pulse loggers are instantiated directly from LogManager, rather than canonical LogService (which itself extends LogManager). Significant reduction of logging level state checks, relying on Log4j handling. Significant reduction of string concatenation, relying on Log4j2 string substitutions. Reduction of logging using an exception e.getMessage, favoring instead passing the exception itself for the stacktrace. Multiple identical exception blocks collapsed. commit dd14d6b0bdce628cc13b3237de5efa0dc18c5661 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-04-10T21:10:52Z Corrected merge error regarding random ports. --- 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 #445: GEODE-2767: Added DUnitTests to validate export log...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/445 GEODE-2767: Added DUnitTests to validate export log options --group and --member. Only affects test code. precheckin running. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode geode-2767 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/445.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 #445 commit 3a5a30cda052359c92b47373f2c9a27d9c0998e3 Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-04-10T21:49:33Z GEODE-2767: Added DUnitTests to validate export log options --group and --member. commit 0686dc8f3a54ce0623e176fd3bdf8cb4dd2cdfbf Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-04-10T22:04:53Z Spotless --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #432: GEODE-1274: Migration from PulseLogWriter to Log4j ...
Github user PurelyApplied closed the pull request at: https://github.com/apache/geode/pull/432 --- 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 #438: GEODE-2725: export logs now honors --dir when connected vi...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/438 Full green precheckin completed. --- 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 #439: GEODE-2716: export logs default behavior changed from filt...
Github user PurelyApplied commented on the issue: https://github.com/apache/geode/pull/439 Full green precheckin completed. --- 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 #439: GEODE-2716: export logs default behavior changed fr...
GitHub user PurelyApplied opened a pull request: https://github.com/apache/geode/pull/439 GEODE-2716: export logs default behavior changed from filtering at log level INFO to ALL. Moved location of export default log level constant from LogService to ExportLogCommand. Updated associated help strings. Tests ran clean previously, new set running after latest merge to develop. Trivial merge conflicts expect these tests to return green as well. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PurelyApplied/geode bugfix/GEODE-2716 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/439.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 #439 commit c9a1b60ba9e97c22f3299b5d78e6ec782a422d7b Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-03-28T22:12:02Z GEODE-2716: export logs default behavior changed from filtering at log level INFO to ALL. Removed reliance on LogService.DEFAULT_LOG_LEVEL, added ExportLogCommand.DEFAULT_EXPORT_LOG_LEVEL. Added DUnit test that fails under previous behavior. commit 5de335d0640b263b1ac3e336ea9876b86aa3f07c Author: Patrick Rhomberg <prhomb...@pivotal.io> Date: 2017-04-05T20:20:16Z Merge branch 'develop' into bugfix/GEODE-2716 --- 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. ---
