You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Raghav Gautam <ra...@apache.org> on 2014/11/04 01:31:38 UTC

Review Request 27553: Add ACL tests for falcon client and ACL update

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

Review request for Falcon.


Bugs: FALCON-861
    https://issues.apache.org/jira/browse/FALCON-861


Repository: falcon-git


Description
-------

Add ACL tests for falcon client and ACL update


Diffs
-----

  falcon-regression/merlin-core/pom.xml 21641a3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
  falcon-regression/pom.xml 0424cb8 

Diff: https://reviews.apache.org/r/27553/diff/


Testing
-------

Ran locally - the tests are not passing because of existing ACL issues.


Thanks,

Raghav Gautam

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Raghav Gautam <ra...@apache.org>.

> On Nov. 4, 2014, 7:02 a.m., Ruslan Ostafiychuk wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java, line 44
> > <https://reviews.apache.org/r/27553/diff/1/?file=747895#file747895line44>
> >
> >     Can we use printPrettyXML here?

Pretty printing is done in clientSubmit() method in IEntityManager class. The reason for this is that in FileUtil.writeEntityToFile() the purpose of writing xml is not known. Typically when we pretty print xml we also say if that entity is being scheduled/modified etc. And we want to follow that convention.


- Raghav


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


On Nov. 3, 2014, 4:31 p.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27553/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2014, 4:31 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-861
>     https://issues.apache.org/jira/browse/FALCON-861
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Add ACL tests for falcon client and ACL update
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/pom.xml 21641a3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
>   falcon-regression/pom.xml 0424cb8 
> 
> Diff: https://reviews.apache.org/r/27553/diff/
> 
> 
> Testing
> -------
> 
> Ran locally - the tests are not passing because of existing ACL issues.
> 
> 
> Thanks,
> 
> Raghav Gautam
> 
>

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Ruslan Ostafiychuk <ro...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27553/#review59767
-----------------------------------------------------------



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java
<https://reviews.apache.org/r/27553/#comment101076>

    message should be documented as "expected" or named as "expectedMessage"



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java
<https://reviews.apache.org/r/27553/#comment101075>

    Whitespace expected before "in execResult:"



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java
<https://reviews.apache.org/r/27553/#comment101072>

    why getPath is used with only one parameter?



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java
<https://reviews.apache.org/r/27553/#comment101073>

    Can we use printPrettyXML here?



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java
<https://reviews.apache.org/r/27553/#comment101074>

    I think that his property should be discribed in README.md


- Ruslan Ostafiychuk


On Nov. 4, 2014, 12:31 a.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27553/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 12:31 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-861
>     https://issues.apache.org/jira/browse/FALCON-861
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Add ACL tests for falcon client and ACL update
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/pom.xml 21641a3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
>   falcon-regression/pom.xml 0424cb8 
> 
> Diff: https://reviews.apache.org/r/27553/diff/
> 
> 
> Testing
> -------
> 
> Ran locally - the tests are not passing because of existing ACL issues.
> 
> 
> Thanks,
> 
> Raghav Gautam
> 
>

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27553/#review59908
-----------------------------------------------------------


- Raghav Gautam


On Nov. 3, 2014, 4:31 p.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27553/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2014, 4:31 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-861
>     https://issues.apache.org/jira/browse/FALCON-861
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Add ACL tests for falcon client and ACL update
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/pom.xml 21641a3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
>   falcon-regression/pom.xml 0424cb8 
> 
> Diff: https://reviews.apache.org/r/27553/diff/
> 
> 
> Testing
> -------
> 
> Ran locally - the tests are not passing because of existing ACL issues.
> 
> 
> Thanks,
> 
> Raghav Gautam
> 
>

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Ruslan Ostafiychuk <ro...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27553/#review59975
-----------------------------------------------------------

Ship it!


Ship It!

- Ruslan Ostafiychuk


On Nov. 5, 2014, 1:52 a.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27553/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 1:52 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-861
>     https://issues.apache.org/jira/browse/FALCON-861
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Add ACL tests for falcon client and ACL update
> 
> 
> Diffs
> -----
> 
>   falcon-regression/README.md 12b15dd 
>   falcon-regression/merlin-core/pom.xml 21641a3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
>   falcon-regression/pom.xml 0424cb8 
> 
> Diff: https://reviews.apache.org/r/27553/diff/
> 
> 
> Testing
> -------
> 
> Ran locally - the tests are not passing because of existing ACL issues.
> 
> 
> Thanks,
> 
> Raghav Gautam
> 
>

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27553/
-----------------------------------------------------------

(Updated Nov. 4, 2014, 5:52 p.m.)


Review request for Falcon.


Bugs: FALCON-861
    https://issues.apache.org/jira/browse/FALCON-861


Repository: falcon-git


Description
-------

Add ACL tests for falcon client and ACL update


Diffs (updated)
-----

  falcon-regression/README.md 12b15dd 
  falcon-regression/merlin-core/pom.xml 21641a3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
  falcon-regression/pom.xml 0424cb8 

Diff: https://reviews.apache.org/r/27553/diff/


Testing
-------

Ran locally - the tests are not passing because of existing ACL issues.


Thanks,

Raghav Gautam

Re: Review Request 27553: Add ACL tests for falcon client and ACL update

Posted by Raghav Gautam <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27553/
-----------------------------------------------------------

(Updated Nov. 4, 2014, 5:50 p.m.)


Review request for Falcon.


Bugs: FALCON-861
    https://issues.apache.org/jira/browse/FALCON-861


Repository: falcon-git


Description
-------

Add ACL tests for falcon client and ACL update


Diffs (updated)
-----

  falcon-regression/README.md 12b15dd 
  falcon-regression/merlin-core/pom.xml 21641a3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/MerlinConstants.java dab5d2c 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/FalconClientBuilder.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/interfaces/IEntityManagerHelper.java 7264142 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/supportClasses/ExecResult.java f878aa8 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java 569d1f3 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/ExecUtil.java d240e76 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/FileUtil.java PRE-CREATION 
  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OSUtil.java 86d4d47 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/AclValidationTest.java d7f21b7 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ClusterAclTest.java affff56 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java PRE-CREATION 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FeedAclTest.java 79e5db6 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/ProcessAclTest.java f0775e7 
  falcon-regression/pom.xml 0424cb8 

Diff: https://reviews.apache.org/r/27553/diff/


Testing
-------

Ran locally - the tests are not passing because of existing ACL issues.


Thanks,

Raghav Gautam