You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Yash Sharma <ya...@gmail.com> on 2015/06/18 04:08:48 UTC
Review Request 35589: LENS-513 - add jar should be able to take regex
path and should be able to add multiple jars
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
Review request for lens.
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs
-----
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing
-------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 19, 2015, 5:19 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 131
> > <https://reviews.apache.org/r/35589/diff/1/?file=986665#file986665line131>
> >
> > Let's do something like the following
> >
> > ```
> > Path p = statuses[count].getPath();
> > matchedPaths.put(path.getName(), p.makeQualified(p.getFileSystem(new Configuration()).getUri(), null);
> > ```
*`p.getName()`
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88526
-----------------------------------------------------------
On June 18, 2015, 7:38 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 7:38 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88526
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 128)
<https://reviews.apache.org/r/35589/#comment141108>
Let's do something like the following
```
Path p = statuses[count].getPath();
matchedPaths.put(path.getName(), p.makeQualified(p.getFileSystem(new Configuration()).getUri(), null);
```
- Rajat Khandelwal
On June 18, 2015, 7:38 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 7:38 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88345
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java (line 159)
<https://reviews.apache.org/r/35589/#comment140823>
This is just a hack. We need to investigate as to why `file:/` is causing problems.
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 162)
<https://reviews.apache.org/r/35589/#comment140825>
+1 for taking arguments instead of using member variables of class.
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java (line 146)
<https://reviews.apache.org/r/35589/#comment140824>
Any one of them has `jar_order`?
Also, add similar test for `add file`, using `glob_order` file
- Rajat Khandelwal
On June 18, 2015, 7:38 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 7:38 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
> On June 22, 2015, 6:23 a.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java, line 160
> > <https://reviews.apache.org/r/35589/diff/2/?file=989046#file989046line160>
> >
> > Instead of this, let's go for something like this:
> >
> > https://github.com/apache/incubator-lens/blob/master/lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java#L2539
Fixed.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88736
-----------------------------------------------------------
On June 22, 2015, 11:31 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 11:31 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88736
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java (line 160)
<https://reviews.apache.org/r/35589/#comment141315>
Instead of this, let's go for something like this:
https://github.com/apache/incubator-lens/blob/master/lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java#L2539
- Rajat Khandelwal
On June 21, 2015, 8:41 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 21, 2015, 8:41 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
> On June 22, 2015, 11:52 a.m., Rajat Khandelwal wrote:
> > lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java, line 190
> > <https://reviews.apache.org/r/35589/diff/3/?file=989668#file989668line190>
> >
> > createNewPath can take `String...`, the `.append` is repeated a lot of times.
Done!
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88767
-----------------------------------------------------------
On June 22, 2015, 1 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 1 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88767
-----------------------------------------------------------
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java (line 190)
<https://reviews.apache.org/r/35589/#comment141351>
createNewPath can take `String...`, the `.append` is repeated a lot of times.
- Rajat Khandelwal
On June 22, 2015, 5:01 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 5:01 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
(Updated June 22, 2015, 1:48 p.m.)
Review request for lens.
Changes
-------
Test results
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs
-----
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing (updated)
-------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Lens Checkstyle Rules .............................. SUCCESS [ 2.740 s]
[INFO] Lens ............................................... SUCCESS [ 2.991 s]
[INFO] Lens API ........................................... SUCCESS [ 20.431 s]
[INFO] Lens API for server and extensions ................. SUCCESS [ 19.884 s]
[INFO] Lens Cube .......................................... SUCCESS [03:11 min]
[INFO] Lens DB storage .................................... SUCCESS [ 21.711 s]
[INFO] Lens Query Library ................................. SUCCESS [ 17.132 s]
[INFO] Lens Hive Driver ................................... SUCCESS [02:44 min]
[INFO] Lens Driver for JDBC ............................... SUCCESS [ 35.909 s]
[INFO] Lens Server ........................................ SUCCESS [04:56 min]
[INFO] Lens client ........................................ SUCCESS [ 36.522 s]
[INFO] Lens CLI ........................................... SUCCESS [02:07 min]
[INFO] Lens Examples ...................................... SUCCESS [ 10.233 s]
[INFO] Lens Distribution .................................. SUCCESS [ 7.797 s]
[INFO] Lens ML Lib ........................................ SUCCESS [01:18 min]
[INFO] Lens ML Ext Distribution ........................... SUCCESS [ 2.379 s]
[INFO] Lens Regression .................................... SUCCESS [ 11.238 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 17:28 min
[INFO] Finished at: 2015-06-22T19:18:15+05:30
[INFO] Final Memory: 125M/414M
[INFO] ------------------------------------------------------------------------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
(Updated June 22, 2015, 1:37 p.m.)
Review request for lens.
Changes
-------
Newer review comments.
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing
-------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
> On June 22, 2015, 1:11 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java, line 150
> > <https://reviews.apache.org/r/35589/diff/4/?file=989676#file989676line150>
> >
> > This check is not needed. Can be covered by Line 165. Similar for delete.
Removed!
> On June 22, 2015, 1:11 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java, line 155
> > <https://reviews.apache.org/r/35589/diff/4/?file=989676#file989676line155>
> >
> > instead of obtaining `iterator()` beforehand, we can only obtain a `ScannedPaths` object and then use foreach loop here.
Implemented.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88777
-----------------------------------------------------------
On June 22, 2015, 1:37 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 1:37 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88777
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java (line 150)
<https://reviews.apache.org/r/35589/#comment141365>
This check is not needed. Can be covered by Line 165. Similar for delete.
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java (line 155)
<https://reviews.apache.org/r/35589/#comment141366>
instead of obtaining `iterator()` beforehand, we can only obtain a `ScannedPaths` object and then use foreach loop here.
- Rajat Khandelwal
On June 22, 2015, 6:30 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 6:30 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
(Updated June 22, 2015, 1 p.m.)
Review request for lens.
Changes
-------
Implemented new review comments.
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing
-------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88769
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 166)
<https://reviews.apache.org/r/35589/#comment141353>
Let's not use null at all. Use empty list.
- Rajat Khandelwal
On June 22, 2015, 5:01 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 5:01 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
> On June 22, 2015, 12:13 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 87
> > <https://reviews.apache.org/r/35589/diff/3/?file=989667#file989667line87>
> >
> > Do we need an `exists` check before `isFile` and `isDir`?
Added !
> On June 22, 2015, 12:13 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 156
> > <https://reviews.apache.org/r/35589/diff/3/?file=989667#file989667line156>
> >
> > Can you make sure when a non-existant non-regex path is provided, `ScannedPaths` doesn't break.
Added new testcase.
> On June 22, 2015, 12:13 p.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 186
> > <https://reviews.apache.org/r/35589/diff/3/?file=989667#file989667line186>
> >
> > Let's keep this logic outside of `ScannedPaths`. the resource can take care of this, ScannedPaths can be a separate entity.
Done. Moved back to sessionresource.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88770
-----------------------------------------------------------
On June 22, 2015, 1 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 1 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 4d82a06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88770
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 76)
<https://reviews.apache.org/r/35589/#comment141356>
Do we need an `exists` check before `isFile` and `isDir`?
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 131)
<https://reviews.apache.org/r/35589/#comment141355>
Can you make sure when a non-existant non-regex path is provided, `ScannedPaths` doesn't break.
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 154)
<https://reviews.apache.org/r/35589/#comment141354>
Let's keep this logic outside of `ScannedPaths`. the resource can take care of this, ScannedPaths can be a separate entity.
- Rajat Khandelwal
On June 22, 2015, 5:01 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 5:01 p.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
(Updated June 22, 2015, 11:31 a.m.)
Review request for lens.
Changes
-------
Updating patch which scans the path recursively - credits @prongs.
Added new testcases for new test scenarios.
Would be posting the entire build results with all testcases in couple of minutes.
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing
-------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 22, 2015, 11:50 a.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 46
> > <https://reviews.apache.org/r/35589/diff/2/?file=989047#file989047line46>
> >
> > parent_dir
> > |
> > |__ a_dir
> > | |
> > | |__ jar_1
> > | |
> > | |__ jar_2
> > | |
> > | |__ jar_order(*1\n*2)
> > |
> > |__ b_dir
> > | |
> > | |__ jar_3
> > | |
> > | |__ jar_4
> > | |
> > | |__ glob_order(*4\n*3)
> > |
> > |__ c_dir
> > | |
> > | |__ jar_5
> > | |
> > | |__ jar_6
> > |
> > |__ jar_order(a*\nb*\nc*)
> >
> >
> > new ScannedPaths("parent_*")
> >
> >
> >
> > Will this work?
parent_dir
|
|__ a_dir
| |
| |__ jar_1
| |
| |__ jar_2
| |
| |__ jar_order(*1\n*2)
|
|__ b_dir
| |
| |__ jar_3
| |
| |__ jar_4
| |
| |__ glob_order(*4\n*3)
|
|__ c_dir
| |
| |__ jar_5
| |
| |__ jar_6
|
|__ jar_order(a*\nb*\nc*)
new ScannedPaths("parent_*")
(markdown causing problems)
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88735
-----------------------------------------------------------
On June 21, 2015, 8:41 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 21, 2015, 8:41 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
> On June 22, 2015, 6:20 a.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 95
> > <https://reviews.apache.org/r/35589/diff/2/?file=989047#file989047line95>
> >
> > Don't need to have return values for functions `removeDirs`, `filterByJarType`. They are taking a collection and modifying itself.
Earlier removeDirs was working on an unmodifiable list so was returning a new copy of the filtered list. Now have merged both and removed dependency on unmodifiable list. No return values now.
> On June 22, 2015, 6:20 a.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java, line 46
> > <https://reviews.apache.org/r/35589/diff/2/?file=989047#file989047line46>
> >
> > parent_dir
> > |
> > |__ a_dir
> > | |
> > | |__ jar_1
> > | |
> > | |__ jar_2
> > | |
> > | |__ jar_order(*1\n*2)
> > |
> > |__ b_dir
> > | |
> > | |__ jar_3
> > | |
> > | |__ jar_4
> > | |
> > | |__ glob_order(*4\n*3)
> > |
> > |__ c_dir
> > | |
> > | |__ jar_5
> > | |
> > | |__ jar_6
> > |
> > |__ jar_order(a*\nb*\nc*)
> >
> >
> > new ScannedPaths("parent_*")
> >
> >
> >
> > Will this work?
>
> Rajat Khandelwal wrote:
> parent_dir
> |
> |__ a_dir
> | |
> | |__ jar_1
> | |
> | |__ jar_2
> | |
> | |__ jar_order(*1\n*2)
> |
> |__ b_dir
> | |
> | |__ jar_3
> | |
> | |__ jar_4
> | |
> | |__ glob_order(*4\n*3)
> |
> |__ c_dir
> | |
> | |__ jar_5
> | |
> | |__ jar_6
> |
> |__ jar_order(a*\nb*\nc*)
>
>
> new ScannedPaths("parent_*")
>
>
> (markdown causing problems)
Added new testcase for checcking coverage of the scenario.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88735
-----------------------------------------------------------
On June 22, 2015, 11:31 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 11:31 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88735
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 44)
<https://reviews.apache.org/r/35589/#comment141314>
parent_dir
|
|__ a_dir
| |
| |__ jar_1
| |
| |__ jar_2
| |
| |__ jar_order(*1\n*2)
|
|__ b_dir
| |
| |__ jar_3
| |
| |__ jar_4
| |
| |__ glob_order(*4\n*3)
|
|__ c_dir
| |
| |__ jar_5
| |
| |__ jar_6
|
|__ jar_order(a*\nb*\nc*)
new ScannedPaths("parent_*")
Will this work?
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java (line 85)
<https://reviews.apache.org/r/35589/#comment141312>
Don't need to have return values for functions `removeDirs`, `filterByJarType`. They are taking a collection and modifying itself.
- Rajat Khandelwal
On June 21, 2015, 8:41 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 21, 2015, 8:41 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/
-----------------------------------------------------------
(Updated June 21, 2015, 3:11 a.m.)
Review request for lens.
Changes
-------
Complete design rewamp for LENS-513.
Supporting regex in jar_order/glob_order.
Please Review.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Lens Checkstyle Rules .............................. SUCCESS [ 2.546 s]
[INFO] Lens ............................................... SUCCESS [ 4.492 s]
[INFO] Lens API ........................................... SUCCESS [ 23.622 s]
[INFO] Lens API for server and extensions ................. SUCCESS [ 26.609 s]
[INFO] Lens Cube .......................................... SUCCESS [03:14 min]
[INFO] Lens DB storage .................................... SUCCESS [ 20.053 s]
[INFO] Lens Query Library ................................. SUCCESS [ 16.111 s]
[INFO] Lens Hive Driver ................................... SUCCESS [02:36 min]
[INFO] Lens Driver for JDBC ............................... SUCCESS [ 36.424 s]
[INFO] Lens Server ........................................ SUCCESS [04:56 min]
[INFO] Lens client ........................................ SUCCESS [ 38.366 s]
[INFO] Lens CLI ........................................... SUCCESS [02:07 min]
[INFO] Lens Examples ...................................... SUCCESS [ 9.603 s]
[INFO] Lens Distribution .................................. SUCCESS [ 7.810 s]
[INFO] Lens ML Lib ........................................ SUCCESS [01:19 min]
[INFO] Lens ML Ext Distribution ........................... SUCCESS [ 2.896 s]
[INFO] Lens Regression .................................... SUCCESS [ 11.626 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 17:35 min
[INFO] Finished at: 2015-06-21T01:46:31+05:30
[INFO] Final Memory: 125M/418M
[INFO] ------------------------------------------------------------------------
Repository: lens
Description
-------
Add jar should be able to take regex path and should be able to add multiple jars.
https://issues.apache.org/jira/browse/LENS-513
New review comments for reopened jira.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
Diff: https://reviews.apache.org/r/35589/diff/
Testing
-------
Thanks,
Yash Sharma
Re: Review Request 35589: LENS-513 - add jar should be able to take
regex path and should be able to add multiple jars
Posted by Yash Sharma <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35589/#review88350
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java (line 159)
<https://reviews.apache.org/r/35589/#comment140829>
Will be taking this up in parallel. Could you please share any error stack you have hit ?
lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java (line 146)
<https://reviews.apache.org/r/35589/#comment140828>
Yes they do. jar_order is present in all 3 dirs dir1/dir2/dir3.
glob_order is present in dir1/dir3.
I delete the jar_order before triggering the test for glob_order
- Yash Sharma
On June 18, 2015, 2:08 a.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35589/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 2:08 a.m.)
>
>
> Review request for lens.
>
>
> Repository: lens
>
>
> Description
> -------
>
> Add jar should be able to take regex path and should be able to add multiple jars.
>
> https://issues.apache.org/jira/browse/LENS-513
>
> New review comments for reopened jira.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 15a8e06
> lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java ac773fe
> lens-server/src/test/java/org/apache/lens/server/util/TestScannedPaths.java 5073634
>
> Diff: https://reviews.apache.org/r/35589/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yash Sharma
>
>