You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by davinash <gi...@git.apache.org> on 2016/10/29 14:29:05 UTC
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
GitHub user davinash opened a pull request:
https://github.com/apache/incubator-geode/pull/278
Feature/geode 1896 unable to specify a PartitionResolver in gfsh
GEODE-1896: Added a argument in Create Region to specify Partition Resolver.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/davinash/incubator-geode feature/GEODE-1896
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-geode/pull/278.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 #278
----
commit 0541cff3445a1f57fb48e642faa955829f6f7872
Author: adongre <av...@ampool.io>
Date: 2016-10-28T10:34:15Z
Adding option partition-resolver in gfsh while creating a region.
commit 00f7f5faf92dd44aa174648557d2ce4ac64ce17b
Author: adongre <av...@ampool.io>
Date: 2016-10-29T03:05:39Z
Fixing test org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest testSerializables
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86684713
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
+ partitionResolverFailure = true;
+ } catch (IllegalAccessException e) {
+ partitionResolverFailure = true;
+ } catch (ClassNotFoundException e) {
+ partitionResolverFailure = true;
+ }
+ if (partitionResolverFailure || !(partitionResolver instanceof PartitionResolver)) {
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-geode/pull/278
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86684707
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
+ partitionResolverFailure = true;
+ } catch (IllegalAccessException e) {
+ partitionResolverFailure = true;
+ } catch (ClassNotFoundException e) {
+ partitionResolverFailure = true;
+ }
+ if (partitionResolverFailure || !(partitionResolver instanceof PartitionResolver)) {
+ throw new IllegalArgumentException(
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86039193
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
+ partitionResolverFailure = true;
+ } catch (IllegalAccessException e) {
+ partitionResolverFailure = true;
+ } catch (ClassNotFoundException e) {
+ partitionResolverFailure = true;
+ }
+ if (partitionResolverFailure || !(partitionResolver instanceof PartitionResolver)) {
--- End diff --
By using Class<PartitionResolver> above, you can eliminate the instanceof check here.
Combine all of these changes and you end up with:
```
try {
...
} catch (InstantiationException| IllegalAccessException| ClassNotFoundException e) {
throw new IllegalArgumentException(
CliStrings.format(CliStrings.CREATE_REGION__MSG__INVALID_PARTITION_RESOLVER,
new Object[] {regionFunctionArgs.getCompressor()}), e);
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86033253
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
--- End diff --
I'd really like to see us keep the original cause instead of discarding it. That will facilitate debugging. You can also you the multiple-catch syntax here:
Exception cause = null;
try {
...
} catch (InstantiationException| IllegalAccessException| ClassNotFoundException e) {
cause = e;
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86039620
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java ---
@@ -405,9 +408,48 @@ private CliFunctionResult handleException(final String memberNameOrId, final Str
prAttrFactory.setStartupRecoveryDelay(partitionArgs.getPrStartupRecoveryDelay());
}
+ if (regionCreateArgs.isPartitionResolverSet()) {
+ Class<?> partitionResolverClass = forName(regionCreateArgs.getPartitionResolver(),
+ CliStrings.CREATE_REGION__PARTITION_RESOLVER);
+ prAttrFactory
+ .setPartitionResolver((PartitionResolver<K, V>) newInstance(partitionResolverClass,
+ CliStrings.CREATE_REGION__PARTITION_RESOLVER));
+ }
return prAttrFactory.create();
}
+
+ private static Class<?> forName(String className, String neededFor) {
--- End diff --
Class<?> could change to Class<PartitionResolver> everywhere in this class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on the issue:
https://github.com/apache/incubator-geode/pull/278
As a committer (https://people.apache.org/phonebook.html?uid=adongre) you should have write access. If that isn't working send a note to private@geode.incubator.apache.org and we will help sort that out.
If you'd have someone else merge the PR, let me know and I will follow the steps at https://cwiki.apache.org/confluence/display/GEODE/Code+contributions.
Cheers!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on the issue:
https://github.com/apache/incubator-geode/pull/278
Thanks @kirklund and @upthewaterspout for details review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86684704
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/278
+1 Looks good to me after addressing Kirk's comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86684743
--- Diff: geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java ---
@@ -1091,6 +1086,126 @@ public void testDestroyRegionWithSharedConfig() {
});
}
+
+ final String PR_STRING = " package com.cadrdunit;"
--- End diff --
@kirklund Thanks for the details review. I did not understand this comment. Could you please help me to understand this. I followed other examples from deploy jar tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86038578
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
--- End diff --
Variable name is probably wrong(?) or copied from other code.
Since the class is of a known type, we should specify that here instead of using '?':
String partitionResolverClassName = PRResolver.class.getName();
Class<PartitionResolver> resolverClass =
(Class<PartitionResolver>)ClassPathLoader.getLatest().forName(partitionResolverClassName);
PartitionResolver partitionResolver = resolverClass.newInstance();
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86033727
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -907,6 +903,31 @@ private void validateRegionFunctionArgs(Cache cache, RegionFunctionArgs regionFu
new Object[] {regionFunctionArgs.getCompressor()}));
}
}
+
+ if (regionFunctionArgs.hasPartitionAttributes()) {
+ boolean partitionResolverFailure = false;
+ if (regionFunctionArgs.isPartitionResolverSet()) {
+ String partitionResolverClassName = regionFunctionArgs.getPartitionResolver();
+ Object partitionResolver = null;
+
+ try {
+ Class<?> compressorClass =
+ (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName);
+ partitionResolver = compressorClass.newInstance();
+ } catch (InstantiationException e) {
+ partitionResolverFailure = true;
+ } catch (IllegalAccessException e) {
+ partitionResolverFailure = true;
+ } catch (ClassNotFoundException e) {
+ partitionResolverFailure = true;
+ }
+ if (partitionResolverFailure || !(partitionResolver instanceof PartitionResolver)) {
+ throw new IllegalArgumentException(
--- End diff --
Use cause from above (you don't need to check it for null because null value is same as not having a cause):
+ throw new IllegalArgumentException(
+ CliStrings.format(CliStrings.CREATE_REGION__MSG__INVALID_PARTITION_RESOLVER,
+ new Object[] {regionFunctionArgs.getCompressor()}), cause);
+ }
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on the issue:
https://github.com/apache/incubator-geode/pull/278
Anything else pending for this PR to merge from my side ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on the issue:
https://github.com/apache/incubator-geode/pull/278
@davinash Thanks for the contribution! Looks like the review is wrapped up from the above comments. If you need help pushing the changes, let me know.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on the issue:
https://github.com/apache/incubator-geode/pull/278
@metatype Please help to merge this PR this time, from next I will following process mentioned at https://cwiki.apache.org/confluence/display/GEODE/Code+contributions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/278
+1 Looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on the issue:
https://github.com/apache/incubator-geode/pull/278
@davinash I merged the PR. Next time make sure to prefix your commit message with "Geode-NNN" so the commits are linked to the JIRA issue. I also marked the issue as resolved. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86040503
--- Diff: geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java ---
@@ -1091,6 +1086,126 @@ public void testDestroyRegionWithSharedConfig() {
});
}
+
+ final String PR_STRING = " package com.cadrdunit;"
--- End diff --
You could add a real class to a sub-package of this test in the source tree and then copy that class out to the .jar file. I'm not sure why you'd be making this so complex other than to avoid checking in a .jar (which is a disallowed binary).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86684717
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java ---
@@ -405,9 +408,48 @@ private CliFunctionResult handleException(final String memberNameOrId, final Str
prAttrFactory.setStartupRecoveryDelay(partitionArgs.getPrStartupRecoveryDelay());
}
+ if (regionCreateArgs.isPartitionResolverSet()) {
+ Class<?> partitionResolverClass = forName(regionCreateArgs.getPartitionResolver(),
+ CliStrings.CREATE_REGION__PARTITION_RESOLVER);
+ prAttrFactory
+ .setPartitionResolver((PartitionResolver<K, V>) newInstance(partitionResolverClass,
+ CliStrings.CREATE_REGION__PARTITION_RESOLVER));
+ }
return prAttrFactory.create();
}
+
+ private static Class<?> forName(String className, String neededFor) {
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...
Posted by davinash <gi...@git.apache.org>.
Github user davinash commented on the issue:
https://github.com/apache/incubator-geode/pull/278
Yes but I guess I have no write access to merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86041266
--- Diff: geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java ---
@@ -1091,6 +1086,126 @@ public void testDestroyRegionWithSharedConfig() {
});
}
+
+ final String PR_STRING = " package com.cadrdunit;"
+ + " public class TestPartitionResolver implements org.apache.geode.cache.PartitionResolver { "
+ + " @Override" + " public void close() {" + " }" + " @Override"
+ + " public Object getRoutingObject(org.apache.geode.cache.EntryOperation opDetails) { "
+ + " return null; " + " }" + " @Override" + " public String getName() { "
+ + " return \"TestPartitionResolver\";" + " }" + " }";
+
+ /**
+ * Test Description 1. Deploy a JAR with Custom Partition Resolver 2. Create Region with Partition
+ * Resolver 3. Region should get created with no Errors 4. Verify Region Partition Attributes for
+ * Partition Resolver
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testCreateRegionWithPartitionResolver() throws IOException {
+ setUpJmxManagerOnVm0ThenConnect(null);
+ VM vm = Host.getHost(0).getVM(1);
+ // Create a cache in vm 1
+ vm.invoke(() -> {
+ assertNotNull(getCache());
+ });
+
+ ClassBuilder classBuilder = new ClassBuilder();
+ // classBuilder.addToClassPath(".");
+ final File prJarFile = new File(new File(".").getAbsolutePath(), "myPartitionResolver.jar");
--- End diff --
You should start using TemporaryFolder for stuff like this so the test cleans up better after it runs.
```
@Rule
public TemporaryFolder temporaryFolder = new SerializableTemporaryFolder();
...
File prJarFile = new File(temporaryFolder.getRoot(), "myPartitionResolver.jar");
```
PS: SerializableTemporaryFolder is a a DUnit friendly version in geode-junit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #278: Feature/geode 1896 unable to specify a Pa...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/278#discussion_r86031904
--- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---
@@ -31,19 +31,13 @@
import javax.management.MalformedObjectNameException;
import javax.management.ObjectName;
+import joptsimple.internal.Strings;
--- End diff --
Let's avoid using internal APIs of 3rd party libraries. Looking down, I see that you're using Strings.isNullOrEmpty. We have a comparable method in org.apache.geode.internal.lang.StringUtils. Please change code from using Strings.isNullOrEmpty to StringUtils.isBlank.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---