You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Balu Vellanki <bv...@hortonworks.com> on 2015/09/15 02:03:42 UTC
Review Request 38387: validation of read/write endpoints is not
reliable - fix this
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/
-----------------------------------------------------------
Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
Bugs: FALCON-1343
https://issues.apache.org/jira/browse/FALCON-1343
Repository: falcon-git
Description
-------
A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
{code}
HadoopClientFactory.get().createProxiedFileSystem(conf);
{code}
I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
Diffs
-----
common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84
common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java a36623c
common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9
common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml 1d15e16
Diff: https://reviews.apache.org/r/38387/diff/
Testing
-------
Tested end2end, added a unit test.
Thanks,
Balu Vellanki
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Sept. 15, 2015, 11:17 p.m., Sowmya Ramesh wrote:
> > In ClusterEntityParser we have validateLocations for validate the locations passed in cluster entity. Isn't that same as this?
If the interface endpoints have a typo, the error thrown by validateLocations does not help users immediately pinpoint the source of problem. So I recommend having a very basic validation.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review99122
-----------------------------------------------------------
On Sept. 21, 2015, 11:38 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 11:38 p.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 6bfcb98
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 6179855
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 2bafac9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review99122
-----------------------------------------------------------
In ClusterEntityParser we have validateLocations for validate the locations passed in cluster entity. Isn't that same as this?
- Sowmya Ramesh
On Sept. 15, 2015, 12:09 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 12:09 a.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java a36623c
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review99922
-----------------------------------------------------------
Ship it!
Please update the description of the review request on the fix to match the implementation
- Venkat Ranganathan
On Sept. 21, 2015, 4:38 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 4:38 p.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 6bfcb98
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 6179855
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 2bafac9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review99958
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Sept. 21, 2015, 11:38 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 11:38 p.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 6bfcb98
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 6179855
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 2bafac9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/
-----------------------------------------------------------
(Updated Sept. 21, 2015, 11:38 p.m.)
Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
Changes
-------
Incorporated feedback from Sowmya Ramesh and Venkat Ranganathan. This patch contains
- Basic validation to ensure readonly and read-write end point are accessible.
- relevant unit tests
Bugs: FALCON-1343
https://issues.apache.org/jira/browse/FALCON-1343
Repository: falcon-git
Description
-------
A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
{code}
HadoopClientFactory.get().createProxiedFileSystem(conf);
{code}
I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 6bfcb98
common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 6179855
common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 2bafac9
common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
Diff: https://reviews.apache.org/r/38387/diff/
Testing
-------
Tested end2end, added a unit test.
Thanks,
Balu Vellanki
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Sept. 15, 2015, 3:52 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java, line 138
> > <https://reviews.apache.org/r/38387/diff/2/?file=1073236#file1073236line138>
> >
> > Instead of using static path like "/tmp", shall we use "temp" element value in cluster entity for validation.
/tmp always exists in HDFS, not so for /temp. For readonly, Falcon do not need to create any files. Falcon just has to verify that user can connect to namenode. So /tmp is better option.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review98988
-----------------------------------------------------------
On Sept. 15, 2015, 12:09 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 12:09 a.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java a36623c
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/#review98988
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java (line 138)
<https://reviews.apache.org/r/38387/#comment155777>
Instead of using static path like "/tmp", shall we use "temp" element value in cluster entity for validation.
- Peeyush Bishnoi
On Sept. 15, 2015, 12:09 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38387/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 12:09 a.m.)
>
>
> Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
>
>
> Bugs: FALCON-1343
> https://issues.apache.org/jira/browse/FALCON-1343
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
> {code}
> HadoopClientFactory.get().createProxiedFileSystem(conf);
> {code}
>
> I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84
> common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java a36623c
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9
> common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38387/diff/
>
>
> Testing
> -------
>
> Tested end2end, added a unit test.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 38387: validation of read/write endpoints is not
reliable - fix this
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38387/
-----------------------------------------------------------
(Updated Sept. 15, 2015, 12:09 a.m.)
Review request for Falcon, Ajay Yadava and Sowmya Ramesh.
Bugs: FALCON-1343
https://issues.apache.org/jira/browse/FALCON-1343
Repository: falcon-git
Description
-------
A read/write endpoint is currently validated by creating a filesystem with the endpoint url.
{code}
HadoopClientFactory.get().createProxiedFileSystem(conf);
{code}
I confirmed with a HDFS team member that it is not sufficient validation. Ideally check if the end user has access by doing atleast a list /tmp call after creating proxiedFileSystem.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84
common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java a36623c
common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9
common/src/test/resources/config/cluster/cluster-bad-write-endpoint.xml PRE-CREATION
Diff: https://reviews.apache.org/r/38387/diff/
Testing
-------
Tested end2end, added a unit test.
Thanks,
Balu Vellanki