> On March 11, 2016, 12:37 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, > > lines 570-572 > > <https://reviews.apache.org/r/44171/diff/4/?file=1293936#file1293936line570> > > > > Can we make set of LensQuery methods and another set of ProxyLensQuery > > methods and compare the sets instead of comparing counts and printing all > > in the message in case of failure? > > Puneet Gupta wrote: > OK. > > > Another thought which i discarded initially was using a Dynamic proxy > (sample code below), but would need to convert LensQuery to an interface > class which is an over kill and code would have if/else for method execution. > What do you think? Any new methods will automatically be handled in proxy > with a default behaviour of delegation to actaual object. > > LensQuery getProxyLensQuery(LensStatement statement, QueryHandle > queryHandle) { > return > (LensQuery)Proxy.newProxyInstance(statement.getClass().getClassLoader(), new > Class[]{LensQuery.class }, > new ProxyLensQueryInvocationHandler(statement, queryHandle)); > } > > @RequiredArgsConstructor > class ProxyLensQueryInvocationHandler implements InvocationHandler { > private final LensStatement statement; > private final QueryHandle queryHandle; > private LensQuery lensQuery; > > @Override > public Object invoke(Object proxy, Method method, Object[] args) > throws Throwable { > String methodName = method.getName(); > if (methodName.equals("getQueryHandle")) { > return queryHandle; > } else if (methodName.equals("getSubmittedUser")) { > return statement.getUser(); > } else { > Class<?>[] argTypes = new Class[args.length]; > int i = 0; > for ( Object arg : args) { > argTypes[i] = arg.getClass(); > } > return lensQuery.getClass().getMethod(methodName, > argTypes).invoke(getQuery(), args); > } > } > > /** > * Gets cached lens query. If nothing is cached, creates and caches > it first. > */ > private synchronized LensQuery getQuery() { > if (lensQuery == null) { > this.lensQuery = statement.getQuery(queryHandle); > } > return this.lensQuery; > } > }
The main purpose here is to ensure the proxy class implements all the methods of the actual class. You're solving that by counting declared methods in test cases, which IMO is not the correct and foolproof way. Alternatives would be: 1. Create an interface and have LensQuery and the Proxy class implement it. But the proxy class will delegate almost all the methods. Any addition in LensQuery will require addition of a delegator method in the proxy class. 2. Dynamic proxy, which will not require changes when further additions are made to LensQuery class. This also avoids an unnecessary interface creation. Option 2 seems better to me, despite having worse performance than 1, since it'll be a one-time change. - Rajat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44171/#review123107 ----------------------------------------------------------- On March 10, 2016, 10:54 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44171/ > ----------------------------------------------------------- > > (Updated March 10, 2016, 10:54 a.m.) > > > Review request for lens. > > > Bugs: LENS_915 > https://issues.apache.org/jira/browse/LENS_915 > > > Repository: lens > > > Description > ------- > > Incremental patch on top of LENS-964 (will re-submit once lens-964 is > committed) > - - Updated cli query execution to use EXECUTE_WITH_TIMEOUT > - Added execute with timeout option to LensClient > - Default timeout value is 10 secs ( can be configured by client) > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/LensQuery.java e0ee761 > lens-api/src/main/java/org/apache/lens/api/query/QueryHandle.java 88e4b0f > lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java > be1ca12 > > lens-cli/src/main/java/org/apache/lens/cli/commands/LensConnectionCommands.java > b760dad > lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java > 2c7b17f > > lens-cli/src/main/java/org/apache/lens/cli/config/LensCliConfigConstants.java > PRE-CREATION > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java > 616bf5d > lens-client/src/main/java/org/apache/lens/client/LensClient.java ea0cd76 > lens-client/src/main/java/org/apache/lens/client/LensConnection.java > eeb473a > lens-client/src/main/java/org/apache/lens/client/LensStatement.java 33c26e1 > > lens-client/src/main/java/org/apache/lens/client/jdbc/LensJdbcStatement.java > 10f7155 > lens-client/src/main/java/org/apache/lens/client/model/ProxyLensQuery.java > PRE-CREATION > lens-client/src/main/resources/lens-client-default.xml e8dbd2c > lens-examples/src/main/java/org/apache/lens/examples/SampleQueries.java > 805a282 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 49ab241 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 699fa68 > src/site/apt/user/client-config.apt 714db18 > > Diff: https://reviews.apache.org/r/44171/diff/ > > > Testing > ------- > > 1. Tested new test cases > ------------------------------------------------------- > T E S T S > ------------------------------------------------------- > Running org.apache.lens.cli.TestLensQueryCommands > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 88.629 sec - > in org.apache.lens.cli.TestLensQueryCommands > > 2. Tested CLI > lens-shell>set lens.query.enable.persistent.resultset.indriver=false > lens-shell>set lens.query.enable.persistent.resultset=true > Set param succeeded > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:08:58 [Spring Shell] INFO cliLogger - Executing query with > timeout of 10000 milliseconds > sample_dim.name sample_dim2.name > Result available in memory, attaching here: > > third thirteen > six sixteen > second twelve > fourth fourteen > first eleven > fifth fifteen > eight eighteen > 7 rows processed in (8) seconds. > lens-shell>set lens.cli.query.execute.timeout.millis=1000 > Client side Set lens.cli.query.execute.timeout.millis=1000 > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:08:38 [Spring Shell] INFO cliLogger - Executing query with > timeout of 1000 milliseconds > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Couldn't complete query > execution within timeout. Waiting for completion > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Query handle: > b071a81f-eea3-4da2-a0fd-451623dc23af > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - User query: 'cube > select sample_dim.name, sample_dim2.name from sample_dim join sample_dim2 on > sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC' was submitted > to hive/hive1 > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Driver query: 'SELECT > ( sample_dim . name ), ( sample_dim2 . name ) FROM db2.local_dim_table > sample_dim inner JOIN db2.local_dim_table2 sample_dim2 ON (( sample_dim . > d2id ) = ( sample_dim2 . id )) AND ((sample_dim.dt = 'latest')) AND > ((sample_dim2.dt = 'latest')) ORDER BY sample_dim . name desc' and Driver > handle: OperationHandle [opType=EXECUTE_STATEMENT, > getHandleIdentifier()=58a8efb4-08cf-4e3e-a279-87125bec9444] > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Query Status:Progress: > 1.0 > Status: SUCCESSFUL > Status Message: Query is successful! > Is Result Set Available: true > > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/b071a81f-eea3-4da2-a0fd-451623dc23af.csv 7 rows > processed in (9) seconds. > > > lens-shell>set lens.query.enable.persistent.resultset.indriver=true > lens-shell>set lens.query.enable.persistent.resultset=true > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:06:37 [Spring Shell] INFO cliLogger - Executing query with > timeout of 10000 milliseconds > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/ce0bcc86-bbb2-4a1c-b457-3eac18ca7cfb.csv 7 rows > processed in (8) seconds. > lens-shell>set lens.cli.query.execute.timeout.millis=1000 > Client side Set lens.cli.query.execute.timeout.millis=1000 > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:07:05 [Spring Shell] INFO cliLogger - Executing query with > timeout of 1000 milliseconds > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Couldn't complete query > execution within timeout. Waiting for completion > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Query handle: > 1eb498e5-13c5-454f-a093-0fc08171d539 > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - User query: 'cube > select sample_dim.name, sample_dim2.name from sample_dim join sample_dim2 on > sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC' was submitted > to hive/hive1 > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Driver query: 'INSERT > OVERWRITE DIRECTORY > "file:/tmp/lensreports/hdfsout/1eb498e5-13c5-454f-a093-0fc08171d539" SELECT > ( sample_dim . name ), ( sample_dim2 . name ) FROM db2.local_dim_table > sample_dim inner JOIN db2.local_dim_table2 sample_dim2 ON (( sample_dim . > d2id ) = ( sample_dim2 . id )) AND ((sample_dim.dt = 'latest')) AND > ((sample_dim2.dt = 'latest')) ORDER BY sample_dim . name desc ' and Driver > handle: OperationHandle [opType=EXECUTE_STATEMENT, > getHandleIdentifier()=e28bb36f-7243-4568-8b2f-3dfbbf34e6cf] > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/1eb498e5-13c5-454f-a093-0fc08171d539.csv 7 rows > processed in (9) seconds. > > > Thanks, > > Puneet Gupta > >
