> On July 27, 2015, 9:12 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/util/ApiUtilityMethods.java, 
> > line 68
> > <https://reviews.apache.org/r/36178/diff/6-7/?file=1018821#file1018821line68>
> >
> >     Why this change? Regular expression seems fishy to me. Can you explain? 
> > Won't `/+` work too? And why are we replacing `//////` with `/`?
> 
> Yash Sharma wrote:
>     Yes /+ should work. The only purpose is to replace all multiple slashes 
> with a single slash. This is the way the CLI sends the paths. Also this will 
> let us handle anypath that has extra slashes erronously like hdfs://// or 
> s3://///
>     
>     Will update the patch and post back.
> 
> Rajat Khandelwal wrote:
>     I think an altogether new scheme which does use say, 4 slashes will need 
> to be handled separately by us. But since we're already doing special 
> handling for hdfs, it's okay. But Let's at least document what pattern of 
> schemes would be supported.

Added comments. Also changed the way special handling is implemented for ease 
of extensions. 

Let me know your thoughts.


> On July 27, 2015, 9:12 a.m., Rajat Khandelwal wrote:
> > lens-api/src/test/java/org/apache/lens/api/TestApiUtilityMethods.java, 
> > lines 73-87
> > <https://reviews.apache.org/r/36178/diff/7/?file=1021880#file1021880line73>
> >
> >     Separate tests can be separate methods. Thus making them modular. 
> >     
> >     If a test will throw exception. Please use `@Test(expectedExceptions = 
> > ...)` instead of catch and assert.
> 
> Yash Sharma wrote:
>     Cool. Will implement. Thanks.

Done !!


- Yash


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36178/#review93101
-----------------------------------------------------------


On July 27, 2015, 7:56 p.m., Yash Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36178/
> -----------------------------------------------------------
> 
> (Updated July 27, 2015, 7:56 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-634
>     https://issues.apache.org/jira/browse/LENS-634
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Added changes for file name suggestions on Tab. Changed String file paths to 
> File.
> 
> 
> https://issues.apache.org/jira/browse/LENS-634
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/java/org/apache/lens/api/util/ApiUtilityMethods.java 
> PRE-CREATION 
>   lens-api/src/test/java/org/apache/lens/api/TestApiUtilityMethods.java 
> PRE-CREATION 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java 
> bdfae24 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensCRUDCommand.java 
> 9cb7d69 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensConnectionCommands.java
>  c059f30 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensCubeCommands.java 
> 6ba702f 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensDimensionCommands.java
>  de022c1 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensDimensionTableCommands.java
>  5fd80a1 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensFactCommands.java 
> 5493dba 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java 
> c48fabd 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensStorageCommands.java 
> 928120a 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/PhysicalTableCrudCommand.java
>  c505680 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensConnectionCliCommands.java 
> ae04b3f 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java 
> 73661e1 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDatabaseCommands.java 
> b6f96e6 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionCommands.java 
> a22862e 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionTableCommands.java
>  56e962d 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommands.java 
> fa342c8 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java
>  73f3a78 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> 319077a 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensStorageCommands.java 
> 3a04ec6 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  1316257 
>   lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java 
> 92687c3 
>   src/site/apt/user/cli.apt a28d8a4 
> 
> Diff: https://reviews.apache.org/r/36178/diff/
> 
> 
> Testing
> -------
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules .............................. SUCCESS [  1.939 
> s]
> [INFO] Lens ............................................... SUCCESS [  2.711 
> s]
> [INFO] Lens API ........................................... SUCCESS [ 22.300 
> s]
> [INFO] Lens API for server and extensions ................. SUCCESS [ 23.875 
> s]
> [INFO] Lens Cube .......................................... SUCCESS [05:27 
> min]
> [INFO] Lens DB storage .................................... SUCCESS [ 20.550 
> s]
> [INFO] Lens Query Library ................................. SUCCESS [ 15.160 
> s]
> [INFO] Lens Hive Driver ................................... SUCCESS [02:30 
> min]
> [INFO] Lens Driver for JDBC ............................... SUCCESS [ 34.568 
> s]
> [INFO] Lens Server ........................................ SUCCESS [04:56 
> min]
> [INFO] Lens client ........................................ SUCCESS [ 37.044 
> s]
> [INFO] Lens CLI ........................................... SUCCESS [02:06 
> min]
> [INFO] Lens Examples ...................................... SUCCESS [  9.571 
> s]
> [INFO] Lens Distribution .................................. SUCCESS [  9.236 
> s]
> [INFO] Lens ML Lib ........................................ SUCCESS [01:15 
> min]
> [INFO] Lens ML Ext Distribution ........................... SUCCESS [  2.492 
> s]
> [INFO] Lens Regression .................................... SUCCESS [ 12.130 
> s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 19:27 min
> [INFO] Finished at: 2015-07-28T01:21:57+05:30
> [INFO] Final Memory: 130M/382M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Yash Sharma
> 
>

Reply via email to