-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IN-PROGRESS] GORA-698 Geode DataStore #273
Conversation
private Region<K, T> region; | ||
private Properties geodeProperties; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove extra spaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra spaces removed
geodeProperties = properties; | ||
|
||
Properties clientProperties = clientCache.getDistributedSystem().getProperties(); | ||
if (userName != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if username is null and password is not? How about throwing an exception in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some code to throw exception
|
||
@Override | ||
public Result<K, T> execute(Query<K, T> query) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space removed
@@ -0,0 +1,187 @@ | |||
package org.apache.gora.geode.store; | |||
|
|||
import org.apache.geode.cache.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please not use wildcard imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced wildcard import statements with only required ones.
* Tests extending {@link DataStoreTestBase} | ||
* which run the base JUnit test suite for Gora. | ||
*/ | ||
public class TestGeodeStore extends DataStoreTestBase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add more tests?
@CalvinKirs Do you have any input on the PR related to Geode work? |
I will take a look later :) |
@himanshuacharya95 Can you please address the remaining request changes? |
Theres some in complete work left in this PR, I will merge this PR for now and take things forward. |
Pending Items Test Cases