> On March 27, 2015, 7:55 p.m., Himanshu Gahlaut wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, line
> > 85
> > <https://reviews.apache.org/r/32478/diff/5/?file=907676#file907676line85>
> >
> > client.getAllDatabases().contains("testclientdb") is repeating here..
> > can we reduce code by having a containsDatabase method in LensClient and
> > and LensMetadataClient. This sort of style also improves datahiding because
> > clients who need only search (contains) does not have to get exposed to
> > entire database collection.
> >
> > public LensClient {
> >
> > public boolean containsDatabase(final String dbName) {
> > return mc.containsDatabase(dbName);
> > }
> > }
> >
> > public LensMetadataclient {
> > public boolean containsDatabase(final String dbName) {
> > return getAllDatabases(dbName).contains(dbName);
> > }
> > }
>
> Jaideep dhok wrote:
> We still needs to expose getAllDatabases (it's used to list all dbs in
> CLI), so contains call won't help much. All calls in LensClient are
> counterparts of their REST calls. However, I do feel we should add a
> 'database exists' call in the metastore resource.
Even if this change does not allow making getAllDatabases() private, creating a
isExistsDatabase() call in LensClient would reduce code in calling classes.
Instead of writing
client.getAllDatabases().contains("testclientdb") ,
we just have to write
client.isExistsDatabase("testclientdb")
a.getB().getC().performX() or a.getB().performY() leads to more code.
Individually it looks like this chaining of getters does not add up more code
but when this sort of thing is there at mulitple places, they collectively
increase code and code becomes less readable. Also giving a name like
isDatabaseExists to chain of getters makes the code more readable. However this
could be my personal opinion as well. I understand that everyone might not
find it less readable. Also your choice of name databaseExists is better than
containsDatabase.
- Himanshu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32478/#review78100
-----------------------------------------------------------
On March 27, 2015, 11:49 a.m., Jaideep dhok wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32478/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 11:49 a.m.)
>
>
> Review request for lens and Amareshwari Sriramadasu.
>
>
> Bugs: LENS-349
> https://issues.apache.org/jira/browse/LENS-349
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Set database in AbstractQueryContext at construction time. This will make
> sure that database used at the time of query submission is also used at the
> time of query execution
> 2. HiveDriver will maintain one session for a lens session + database name
> pair.
> 3. Borrowed some code related to closing class loaders and tests from the
> previous patch.
> Reply
>
>
> Diffs
> -----
>
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java
> 81a536ed8ac3f7ea550794cf188db6ea9d599c7d
> lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java
> 9e3c72393c1d9634b4ae03301b5d620b647df7ae
>
> lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java
> 8a5839b01ad20fc6ff030d55788ceb2c368d7f69
> lens-driver-jdbc/testdata/DatabaseJarSerde.java
> 03caff31916072b0e6944d1bd13161dca5cef878
>
> lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java
> 523356903933af37eb9b1deca805cc8ae76a4cbb
>
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
> 390071cdd40ccc0308a4a58852e9dbfcbddabc3a
>
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java
> 8c970821015b04e4adaff7d2949520ea5a0c4c82
> lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java
> 20856a09610d76ca8072851e00125fc50aa4343c
> lens-server/src/test/java/org/apache/lens/server/LensTestUtil.java
> e44816372e889bdc7ce2ac34a43048e8af85181e
> lens-server/src/test/java/org/apache/lens/server/TestServerRestart.java
> 6306b51cdf2607c9762aec522d663019b202760f
>
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
> 14a4eb2f09ad07bd6592f9f2e7ebff02080e0d14
> lens-server/testdata/DatabaseJarSerde.java
> 03caff31916072b0e6944d1bd13161dca5cef878
> lens-server/testdata/serde.jar ec86e49a0be7cb9872756a4313ae81bd3cb5e543
> lens-server/testdata/test.jar 1644d8cada37749f6a8c3a2a6c26b752ea7bac0f
>
> Diff: https://reviews.apache.org/r/32478/diff/
>
>
> Testing
> -------
>
> Changed existing test for db jars to validate db switch
> Latest results after lazy add change
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.314s]
> [INFO] Lens .............................................. SUCCESS [2.023s]
> [INFO] Lens API .......................................... SUCCESS [5.038s]
> [INFO] Lens API for server and extensions ................ SUCCESS [7.293s]
> [INFO] Lens Cube ......................................... SUCCESS [2:03.134s]
> [INFO] Lens DB storage ................................... SUCCESS [9.614s]
> [INFO] Lens Query Library ................................ SUCCESS [4.771s]
> [INFO] Lens Hive Driver .................................. SUCCESS [2:33.396s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [17.050s]
> [INFO] Lens Server ....................................... SUCCESS [5:49.935s]
> [INFO] Lens client ....................................... SUCCESS [21.035s]
> [INFO] Lens CLI .......................................... SUCCESS [3:04.331s]
> [INFO] Lens Examples ..................................... SUCCESS [0.857s]
> [INFO] Lens Distribution ................................. SUCCESS [10.009s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:01.179s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [2.539s]
> [INFO] Lens Regression ................................... SUCCESS [0.467s]
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Total time: 15:55.934s
> [INFO] Finished at: Fri Mar 27 11:43:57 UTC 2015
> [INFO] Final Memory: 97M/1110M
> [INFO]
> ------------------------------------------------------------------------
> [SLOCCount] Report successfully processed and all data stored
> Recording test results
> Finished: SUCCESS
>
>
> Thanks,
>
> Jaideep dhok
>
>
