You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Anne Yu <an...@cloudera.com> on 2016/08/26 23:32:32 UTC
Review Request 51471: SENTRY-1429,
SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/
-----------------------------------------------------------
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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> 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 445
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line445>
> >
> > Why are we assuming true here? If the mkdir failed then the setup essentially fails, failing the test as well isnt it?
My bad, just checked the semantics of assumeTrue, so we are basically skipping if mkdir fails. That seems fair enough.
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147050
-----------------------------------------------------------
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> 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
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line447>
> >
> > 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
>
>
Re: Review Request 51471: SENTRY-1429,
SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.
Posted by Anne Yu <an...@cloudera.com>.
> 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 445
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line445>
> >
> > Why are we assuming true here? If the mkdir failed then the setup essentially fails, failing the test as well isnt it?
>
> Sravya Tirukkovalur wrote:
> My bad, just checked the semantics of assumeTrue, so we are basically skipping if mkdir fails. That seems fair enough.
I will put assert instead of assume here.
> 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
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line447>
> >
> > Why is this 777? Seems too open for a security test.
Not sure why originally most tests configured it as 777. Will change it to drwxrwx---.
> 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 452
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line452>
> >
> > Same here. Why assume.true?
Will change it back assert.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147050
-----------------------------------------------------------
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147050
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java (line 445)
<https://reviews.apache.org/r/51471/#comment214044>
Why are we assuming true here? If the mkdir failed then the setup essentially fails, failing the test as well isnt it?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java (line 447)
<https://reviews.apache.org/r/51471/#comment214045>
Why is this 777? Seems too open for a security test.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java (line 452)
<https://reviews.apache.org/r/51471/#comment214047>
Same here. Why assume.true?
- Sravya Tirukkovalur
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Anne Yu <an...@cloudera.com>.
> 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
> > <https://reviews.apache.org/r/51471/diff/2-3/?file=1487247#file1487247line447>
> >
> > 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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/51471/#comment214282>
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.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147200
-----------------------------------------------------------
Ship it!
Ship It!
- Hao Hao
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.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
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.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
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.
Changes
-------
Addressed previous code review comments.
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 (updated)
-----
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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147047
-----------------------------------------------------------
Thanks for refactoring this Anne! It is a bit hard to review the improvements you made along with refactoring, as there is no diff of changes due to new files. Can you point us to the code changes that you made?
- Sravya Tirukkovalur
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Anne Yu <an...@cloudera.com>.
> On Aug. 27, 2016, 12:24 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java, lines 437-453
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line437>
> >
> > So this is the new improvement right?
>
> Anne Yu wrote:
> Thanks for reviewing. Correct, to ensure for each test external dirs can be re-created without correct properties.
Typo, "ensure for each test external dirs can be re-created with correct properties."
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147048
-----------------------------------------------------------
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Anne Yu <an...@cloudera.com>.
> On Aug. 27, 2016, 12:24 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java, lines 437-453
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line437>
> >
> > So this is the new improvement right?
Thanks for reviewing. Correct, to ensure for each test external dirs can be re-created without correct properties.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147048
-----------------------------------------------------------
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147048
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java (lines 437 - 453)
<https://reviews.apache.org/r/51471/#comment214042>
So this is the new improvement right?
- Sravya Tirukkovalur
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Anne Yu <an...@cloudera.com>.
> 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
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line440>
> >
> > 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.
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/#review147055
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java (line 440)
<https://reviews.apache.org/r/51471/#comment214053>
Do we really want to create a partition location in setup function? Not all tests need this right?
- Sravya Tirukkovalur
On Aug. 27, 2016, 12:13 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:13 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.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51471/
-----------------------------------------------------------
(Updated Aug. 27, 2016, 12:13 a.m.)
Review request for sentry, Hao Hao and Sravya Tirukkovalur.
Changes
-------
Add solr tests back into post commit run profile.
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 (updated)
-----
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