Gora-444 : Add #size() to Result API #132
Conversation
…ongoDB, OrientDB, JCache, Solr, Mock, Mem.
Set batch size using query.limit
|
@carlosrmng great 👍 |
|
I am +1 for committing this. @carlosrmng it would be great if we could add tests to DataStoreTestBase.java to accommodate this proposed addition to the core API. |
|
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. |
|
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 |
|
+1 |
|
Thanks for your support, I will start working in some tests and documentation. |
| public int size() { | ||
| return cacheKeySet.size(); | ||
| } | ||
|
|
There was a problem hiding this comment.
@carlosrmng It is a good practice if you can avoid empty lines like this :)
-Implement tests within DataStoreTestBase and DataStoreTestUtil. -Fix size() implementation for MemStore, Accumulo, Solr, JCache and OrientDB.
| return map.navigableKeySet().size(); | ||
| int totalSize = map.navigableKeySet().size(); | ||
| int intLimit = (int)this.limit; | ||
| return intLimit > 0 && totalSize>intLimit ? intLimit : totalSize; |
There was a problem hiding this comment.
Please reformat the code with desired spacing.
|
Hi Carlos, This is great. One thing to note is when sending PRs please make sure code is well formatted and all required Java docs are added for public methods. These improvements will show a good standard in your coding :) |
|
+1 |
|
+1, can someone merge into master? |
| return intLimit > 0 && totalSize>intLimit ? intLimit : totalSize; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add the ending new line
|
Is this okay for merge? |
|
If any one have concerns please raise, otherwise I will proceed merging this PR to master. |
|
I just merged your PR to master. Awesome work Carlos 👍 One minor thing to point out, it s good if you can be consistent with commit messages. Have a look at this post. |
6 participants
This PR adds #size() to Result API, it implements size() for existing backends. The complete disscusion can be found in 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