> 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. > > Himanshu Gahlaut wrote: > 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.
I aggree with the readability thing, but database exist call only in the client without a supporting server side call doesn't make sense. We should add a server side database exists call in metastore API and corresponding call in client. However, that is out of scope for this patch. We can take it up in another JIRA. - Jaideep ----------------------------------------------------------- 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 > >
