You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gora.apache.org by carlosrmng <gi...@git.apache.org> on 2018/03/23 05:30:27 UTC
[GitHub] gora pull request #132: Gora-444 : Add #size() to Result API
GitHub user carlosrmng opened a pull request:
https://github.com/apache/gora/pull/132
Gora-444 : Add #size() to Result API
This PR adds #size() to Result API, it implements size() for existing backends. The complete disscusion can be found in [GORA-444](https://issues.apache.org/jira/browse/GORA-444).
Some issues with this PR:
-new tests are need.
-Backends: Accumulo, Avro, HBase, don't support size() functions, so an approach using batchSize was used instead.
Code review welcome, thanks
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/carlosrmng/gora GORA-444
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/gora/pull/132.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 #132
----
commit 76cc89cddd2cbda42518de8e304e92e3f341a744
Author: Carlos Muñoz <ca...@...>
Date: 2018-03-22T05:06:56Z
Add #size() to Result API: Aerospike, Cassandra, CouchDB, DynamoDB, MongoDB, OrientDB, JCache, Solr, Mock, Mem.
commit 0ee48ab3d96445a280852a1fd04206efb883baa9
Author: Carlos Muñoz <ca...@...>
Date: 2018-03-22T05:59:02Z
Add #size() to Result API: Accumulo, Avro, HBase
Set batch size using query.limit
----
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:
https://github.com/apache/gora/pull/132
Yep tests would be great. IMHO, as this is something we want to test for every Datastore, I suggest it be implemented within [DataStoreTestUtil.java](https://github.com/apache/gora/blob/master/gora-core/src/test/java/org/apache/gora/store/DataStoreTestUtil.java)
---
[GitHub] gora pull request #132: Gora-444 : Add #size() to Result API
Posted by nishadi <gi...@git.apache.org>.
Github user nishadi commented on a diff in the pull request:
https://github.com/apache/gora/pull/132#discussion_r185691756
--- Diff: gora-jcache/src/main/java/org/apache/gora/jcache/query/JCacheResult.java ---
@@ -88,4 +88,4 @@ public int size() {
int intLimit = (int)this.limit;
return intLimit > 0 && totalSize>intLimit ? intLimit : totalSize;
}
-}
+}
--- End diff --
Please add the ending new line
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by djkevincr <gi...@git.apache.org>.
Github user djkevincr commented on the issue:
https://github.com/apache/gora/pull/132
+1
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by cguzel <gi...@git.apache.org>.
Github user cguzel commented on the issue:
https://github.com/apache/gora/pull/132
+1
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by carlosrmng <gi...@git.apache.org>.
Github user carlosrmng commented on the issue:
https://github.com/apache/gora/pull/132
Thanks for your support, I will start working in some tests and documentation.
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by djkevincr <gi...@git.apache.org>.
Github user djkevincr commented on the issue:
https://github.com/apache/gora/pull/132
If any one have concerns please raise, otherwise I will proceed merging this PR to master.
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by nishadi <gi...@git.apache.org>.
Github user nishadi commented on the issue:
https://github.com/apache/gora/pull/132
This is great work...!!! As Lewis mentioned, shall we add tests for this? As well we will need to document all the public APIs as well.
---
[GitHub] gora pull request #132: Gora-444 : Add #size() to Result API
Posted by nishadi <gi...@git.apache.org>.
Github user nishadi commented on a diff in the pull request:
https://github.com/apache/gora/pull/132#discussion_r185170628
--- Diff: gora-core/src/main/java/org/apache/gora/memory/store/MemStore.java ---
@@ -92,7 +92,9 @@ public boolean nextInner() throws IOException {
@Override
public int size() {
- return map.navigableKeySet().size();
+ int totalSize = map.navigableKeySet().size();
+ int intLimit = (int)this.limit;
+ return intLimit > 0 && totalSize>intLimit ? intLimit : totalSize;
--- End diff --
Please reformat the code with desired spacing.
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by carlosrmng <gi...@git.apache.org>.
Github user carlosrmng commented on the issue:
https://github.com/apache/gora/pull/132
Is this okay for merge?
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by djkevincr <gi...@git.apache.org>.
Github user djkevincr commented on the issue:
https://github.com/apache/gora/pull/132
Awesome work Carlos :+1: One minor thing to point out, it s good if you can be consistent with commit messages. Have look at this post.
[1] https://chris.beams.io/posts/git-commit/
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by nishadi <gi...@git.apache.org>.
Github user nishadi commented on the issue:
https://github.com/apache/gora/pull/132
Hi Carlos, This is great. One thing to note is when sending PRs please make sure code is well formatted, all required Java docs is added for public methods. These improvements will show case a good standard in your coding :)
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:
https://github.com/apache/gora/pull/132
@carlosrmng great 👍
Can you please format 2 space indents the same as the other source code?
Thanks
---
[GitHub] gora pull request #132: Gora-444 : Add #size() to Result API
Posted by nishadi <gi...@git.apache.org>.
Github user nishadi commented on a diff in the pull request:
https://github.com/apache/gora/pull/132#discussion_r177727883
--- Diff: gora-jcache/src/main/java/org/apache/gora/jcache/query/JCacheResult.java ---
@@ -82,4 +82,9 @@ protected boolean nextInner() throws IOException {
return true;
}
+ @Override
+ public int size() {
+ return cacheKeySet.size();
+ }
+
--- End diff --
@carlosrmng It is a good practice if you can avoid empty lines like this :)
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:
https://github.com/apache/gora/pull/132
+1
---
[GitHub] gora pull request #132: Gora-444 : Add #size() to Result API
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/gora/pull/132
---
[GitHub] gora issue #132: Gora-444 : Add #size() to Result API
Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:
https://github.com/apache/gora/pull/132
I am +1 for committing this. @carlosrmng it would be great if we could add tests to [DataStoreTestBase.java](https://github.com/apache/gora/blob/master/gora-core/src/test/java/org/apache/gora/store/DataStoreTestBase.java) to accommodate this proposed addition to the core API.
---