Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

2016-08-29 Thread Anne Yu 


> On Aug. 29, 2016, 6:22 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 447
> > 
> >
> > User needs to have execute bit set to get to the children. So we will 
> > need 771.

Sure will change it. Thanks.


- Anne


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


On Aug. 29, 2016, 6:41 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 29, 2016, 6:41 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1454
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1454
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  a2aead0c55cbee8aca9608539ad2681891acf57d 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

2016-08-29 Thread Anne Yu 

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

(Updated Aug. 29, 2016, 6:41 p.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

Addressed Haohao's comments.


Bugs: SENTRY-1429 and SENTRY-1454
https://issues.apache.org/jira/browse/SENTRY-1429
https://issues.apache.org/jira/browse/SENTRY-1454


Repository: sentry


Description
---

SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
test class, improve its cleanUp and setUp for temp HDFS dirs; divide test class 
into smoke test class and p2 test class; in post commit run, we will only run 
smoke tests.


Diffs (updated)
-

  pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 a2aead0c55cbee8aca9608539ad2681891acf57d 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 be6d0822ea56509ae9171d2b79026b1c952c2ec9 

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


Testing
---

Local.


Thanks,

Anne Yu

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

2016-08-29 Thread Sravya Tirukkovalur 

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


Fix it, then Ship it!





sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 (line 447)


User needs to have execute bit set to get to the children. So we will need 
771.


- Sravya Tirukkovalur


On Aug. 27, 2016, 12:42 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:42 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

2016-08-26 Thread Anne Yu 


> On Aug. 27, 2016, 12:39 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 440
> > 
> >
> > Do we really want to create a partition location in setup function? Not 
> > all tests need this right?

But half of them need this partition path. It will be duplicate to create 
partition dir in each of test, for new tests, we might forget to create it. 

BTW, it's time-consuming to create a patch then submit to sentry jira to pass 
hadoop-qa. So will leave this review for a while until get all the reviewers's 
comments. Thanks.


- Anne


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


On Aug. 27, 2016, 12:42 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:42 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

2016-08-26 Thread Sravya Tirukkovalur 


> On Aug. 27, 2016, 12:30 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 447
> > 
> >
> > Why is this 777? Seems too open for a security test.
> 
> Anne Yu wrote:
> Not sure why originally most tests configured it as 777. Will change it 
> to drwxrwx---.

770 might be too restrictive. We may need 771.


- Sravya


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


On Aug. 27, 2016, 12:42 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:42 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>