[jira] [Updated] (SENTRY-1473) [test]: add more tests to test sentry supports only absolute path for now: s3://bucketname/ is correct; while s3://bucket is not
[ https://issues.apache.org/jira/browse/SENTRY-1473?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anne Yu updated SENTRY-1473: Issue Type: Test (was: Bug) > [test]: add more tests to test sentry supports only absolute path for now: > s3://bucketname/ is correct; while s3://bucket is not > > > Key: SENTRY-1473 > URL: https://issues.apache.org/jira/browse/SENTRY-1473 > Project: Sentry > Issue Type: Test > Components: Sentry, Test >Affects Versions: 1.8.0 >Reporter: Anne Yu >Assignee: Anne Yu > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1473) [test]: add more tests to test sentry supports only absolute path for now: s3://bucketname/ is correct; while s3://bucket is not
[ https://issues.apache.org/jira/browse/SENTRY-1473?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anne Yu updated SENTRY-1473: Summary: [test]: add more tests to test sentry supports only absolute path for now: s3://bucketname/ is correct; while s3://bucket is not (was: sentry supports absolute path for now: s3://bucketname/ is correct; while s3://bucket is not) > [test]: add more tests to test sentry supports only absolute path for now: > s3://bucketname/ is correct; while s3://bucket is not > > > Key: SENTRY-1473 > URL: https://issues.apache.org/jira/browse/SENTRY-1473 > Project: Sentry > Issue Type: Bug > Components: Sentry, Test >Affects Versions: 1.8.0 >Reporter: Anne Yu > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1473) [test]: add more tests to test sentry supports only absolute path for now: s3://bucketname/ is correct; while s3://bucket is not
[ https://issues.apache.org/jira/browse/SENTRY-1473?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anne Yu updated SENTRY-1473: Assignee: Anne Yu Affects Version/s: 1.8.0 Component/s: Test Sentry > [test]: add more tests to test sentry supports only absolute path for now: > s3://bucketname/ is correct; while s3://bucket is not > > > Key: SENTRY-1473 > URL: https://issues.apache.org/jira/browse/SENTRY-1473 > Project: Sentry > Issue Type: Bug > Components: Sentry, Test >Affects Versions: 1.8.0 >Reporter: Anne Yu >Assignee: Anne Yu > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (SENTRY-1473) sentry supports absolute path for now: s3://bucketname/ is correct; while s3://bucket is not
Anne Yu created SENTRY-1473: --- Summary: sentry supports absolute path for now: s3://bucketname/ is correct; while s3://bucket is not Key: SENTRY-1473 URL: https://issues.apache.org/jira/browse/SENTRY-1473 Project: Sentry Issue Type: Bug Reporter: Anne Yu -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494783#comment-15494783
]
Hadoop QA commented on SENTRY-1471:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12828748/SENTRY-1471.001.patch
against master.
{color:red}Overall:{color} -1 due to 2 errors
{color:red}ERROR:{color} mvn test exited 1
{color:red}ERROR:{color} Failed:
org.apache.sentry.provider.db.generic.service.persistent.TestPrivilegeOperatePersistence
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2025/console
This message is automatically generated.
> TestHDFSIntegrationBase.java implements HDFS ACL checking and query
> verification incorrectly
>
>
> Key: SENTRY-1471
> URL: https://issues.apache.org/jira/browse/SENTRY-1471
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Reporter: Vadim Spector
>Assignee: Anne Yu
> Attachments: SENTRY-1471.001.patch
>
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When
> the number of failed retries exceeds max value (the "else" portion of "if
> (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means
> that verifyOnAllSubDirsHelper() never returns false.
> Once the problem was fixed in a local development branch, several tests in
> TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End
> tests are still ok.
> Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean
> instead of throwing the original AssertionError which contains information
> about expected vs. found ACL permissions - this information is invaluable for
> debugging. The fix is easy - the to-be-fixed "else" portion is inside the
> "catch (Throwable th)" clause, so it should just re-throw caught exception.
> This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper()
> should have "void" return type.
> The same problem exists in verifyQuery(), also in
> TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also
> assigns "isSucceeded = true;" leading to the same problem - it never fails.
> Suggested solution would be the same - just re-throw the exception. It will
> not only fix the problem, but also make debugging easier. Current code
> "Assert.assertTrue(isSucceeded);" throws away valuable information about the
> expected vs detected number of database rows.
> Note: the above problems were introduced by SENTRY-1454. Suggested fixes
> match the original code prior to SENTRY-1454.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494634#comment-15494634
]
Hadoop QA commented on SENTRY-1471:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12828735/SENTRY-1471.001.patch
against master.
{color:red}Overall:{color} -1 due to an error
{color:red}ERROR:{color} failed to apply patch (exit code 1):
The patch does not appear to apply with p0, p1, or p2
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2024/console
This message is automatically generated.
> TestHDFSIntegrationBase.java implements HDFS ACL checking and query
> verification incorrectly
>
>
> Key: SENTRY-1471
> URL: https://issues.apache.org/jira/browse/SENTRY-1471
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Reporter: Vadim Spector
>Assignee: Anne Yu
> Attachments: SENTRY-1471.001.patch
>
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When
> the number of failed retries exceeds max value (the "else" portion of "if
> (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means
> that verifyOnAllSubDirsHelper() never returns false.
> Once the problem was fixed in a local development branch, several tests in
> TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End
> tests are still ok.
> Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean
> instead of throwing the original AssertionError which contains information
> about expected vs. found ACL permissions - this information is invaluable for
> debugging. The fix is easy - the to-be-fixed "else" portion is inside the
> "catch (Throwable th)" clause, so it should just re-throw caught exception.
> This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper()
> should have "void" return type.
> The same problem exists in verifyQuery(), also in
> TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also
> assigns "isSucceeded = true;" leading to the same problem - it never fails.
> Suggested solution would be the same - just re-throw the exception. It will
> not only fix the problem, but also make debugging easier. Current code
> "Assert.assertTrue(isSucceeded);" throws away valuable information about the
> expected vs detected number of database rows.
> Note: the above problems were introduced by SENTRY-1454. Suggested fixes
> match the original code prior to SENTRY-1454.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1472) broken unit test revealed by SENTRY-1471
[ https://issues.apache.org/jira/browse/SENTRY-1472?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1472: -- Description: SENTRY-1471 fixed permissions validation logic in TestHDFSIntegrationBase.java class, which revealed that TestHDFSIntegrationAdvanced.testAuthzObjOnPartitionMultipleTables() unit test is broken. Once SENTRY-1471 is resolved, need to fix this broken test and re-enable it. (was: SENTRY-1471 fixed permissions validation logic in TestHDFSIntegrationBase.java class, which revealed that TestHDFSIntegrationAdvanced.testAuthzObjOnPartitionMultipleTables() unit test is broken. Once SENTRY-1471 is resolved, need to fix the test and re-enable it.) > broken unit test revealed by SENTRY-1471 > > > Key: SENTRY-1472 > URL: https://issues.apache.org/jira/browse/SENTRY-1472 > Project: Sentry > Issue Type: Bug >Reporter: Vadim Spector > > SENTRY-1471 fixed permissions validation logic in > TestHDFSIntegrationBase.java class, which revealed that > TestHDFSIntegrationAdvanced.testAuthzObjOnPartitionMultipleTables() unit test > is broken. Once SENTRY-1471 is resolved, need to fix this broken test and > re-enable it. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch SENTRY-1471.001.patch > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494416#comment-15494416 ] Vadim Spector commented on SENTRY-1471: --- Per conversation with [~anneyu] - disabling two unit tests testNonHDFSLocations() testAuthzObjOnPartitionMultipleTables(), both found broken after fixing the validation code - to be fixed in a separate JIRA. > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494413#comment-15494413
]
Hadoop QA commented on SENTRY-1471:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12828721/SENTRY-1471.001.patch
against master.
{color:red}Overall:{color} -1 due to an error
{color:red}ERROR:{color} failed to build with patch (exit code 1)
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2022/console
This message is automatically generated.
> TestHDFSIntegrationBase.java implements HDFS ACL checking and query
> verification incorrectly
>
>
> Key: SENTRY-1471
> URL: https://issues.apache.org/jira/browse/SENTRY-1471
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Reporter: Vadim Spector
>Assignee: Anne Yu
> Attachments: SENTRY-1471.001.patch
>
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When
> the number of failed retries exceeds max value (the "else" portion of "if
> (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means
> that verifyOnAllSubDirsHelper() never returns false.
> Once the problem was fixed in a local development branch, several tests in
> TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End
> tests are still ok.
> Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean
> instead of throwing the original AssertionError which contains information
> about expected vs. found ACL permissions - this information is invaluable for
> debugging. The fix is easy - the to-be-fixed "else" portion is inside the
> "catch (Throwable th)" clause, so it should just re-throw caught exception.
> This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper()
> should have "void" return type.
> The same problem exists in verifyQuery(), also in
> TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also
> assigns "isSucceeded = true;" leading to the same problem - it never fails.
> Suggested solution would be the same - just re-throw the exception. It will
> not only fix the problem, but also make debugging easier. Current code
> "Assert.assertTrue(isSucceeded);" throws away valuable information about the
> expected vs detected number of database rows.
> Note: the above problems were introduced by SENTRY-1454. Suggested fixes
> match the original code prior to SENTRY-1454.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1459) Alter view with HMS Client fails with "java.lang.IllegalArgumentException: Can not create a Path from a null string"
[
https://issues.apache.org/jira/browse/SENTRY-1459?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494400#comment-15494400
]
Ankur Gupta commented on SENTRY-1459:
-
Based on the comments above, I removed the changes in PathUtils because
returning can certainly be dangerous. Instead, I am handling null location in
authorizeAlterTable method. I also added a test case in TestMetastoreEndToEnd,
it doesn't check for all privileges as is done for alterTable but just one to
ensure that alterView is working.
> Alter view with HMS Client fails with "java.lang.IllegalArgumentException:
> Can not create a Path from a null string"
>
>
> Key: SENTRY-1459
> URL: https://issues.apache.org/jira/browse/SENTRY-1459
> Project: Sentry
> Issue Type: Bug
>Reporter: Sravya Tirukkovalur
>Assignee: Ankur Gupta
> Attachments: SENTRY-1459.001.patch, SENTRY-1459.002.patch
>
>
> {code}
> 2016-08-25 13:32:51,470 ERROR
> org.apache.hadoop.hive.metastore.RetryingHMSHandler: [pool-6-thread-69]:
> MetaException(message:java.lang.IllegalArgumentException: Can not create a
> Path from a null string)
> at
> org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.newMetaException(HiveMetaStore.java:5438)
> at
> org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.alter_table_core(HiveMetaStore.java:3470)
> at
> org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.alter_table_with_environment_context(HiveMetaStore.java:3430)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at
> org.apache.hadoop.hive.metastore.RetryingHMSHandler.invokeInternal(RetryingHMSHandler.java:133)
> at
> org.apache.hadoop.hive.metastore.RetryingHMSHandler.invoke(RetryingHMSHandler.java:99)
> at com.sun.proxy.$Proxy20.alter_table_with_environment_context(Unknown Source)
> at
> org.apache.hadoop.hive.metastore.api.ThriftHiveMetastore$Processor$alter_table_with_environment_context.getResult(ThriftHiveMetastore.java:9469)
> at
> org.apache.hadoop.hive.metastore.api.ThriftHiveMetastore$Processor$alter_table_with_environment_context.getResult(ThriftHiveMetastore.java:9453)
> at org.apache.thrift.ProcessFunction.process(ProcessFunction.java:39)
> at org.apache.thrift.TBaseProcessor.process(TBaseProcessor.java:39)
> at
> org.apache.hadoop.hive.thrift.HadoopThriftAuthBridge$Server$TUGIAssumingProcessor$1.run(HadoopThriftAuthBridge.java:684)
> at
> org.apache.hadoop.hive.thrift.HadoopThriftAuthBridge$Server$TUGIAssumingProcessor$1.run(HadoopThriftAuthBridge.java:679)
> at java.security.AccessController.doPrivileged(Native Method)
> at javax.security.auth.Subject.doAs(Subject.java:415)
> at
> org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1671)
> at
> org.apache.hadoop.hive.thrift.HadoopThriftAuthBridge$Server$TUGIAssumingProcessor.process(HadoopThriftAuthBridge.java:679)
> at
> org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:285)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:745)
> Caused by: java.lang.IllegalArgumentException: Can not create a Path from a
> null string
> at org.apache.hadoop.fs.Path.checkPathArg(Path.java:123)
> at org.apache.hadoop.fs.Path.(Path.java:135)
> at org.apache.sentry.core.common.utils.PathUtils.parseURI(PathUtils.java:110)
> at
> org.apache.sentry.core.common.utils.PathUtils.parseDFSURI(PathUtils.java:100)
> at
> org.apache.sentry.binding.metastore.MetastoreAuthzBinding.authorizeAlterTable(MetastoreAuthzBinding.java:284)
> at
> org.apache.sentry.binding.metastore.MetastoreAuthzBinding.onEvent(MetastoreAuthzBinding.java:188)
> at
> org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.firePreEvent(HiveMetaStore.java:2028)
> at
> org.apache.hadoop.hive.metastore.HiveMetaStore$HMSHandler.alter_table_core(HiveMetaStore.java:3449)
> ... 22 more
> {code}
> Seems like we need to add a null path check here. See
> MetastoreAuthzBindingBase. A test case can be added in TestMetastoreEndToEnd
> for alter view.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494239#comment-15494239
]
Hadoop QA commented on SENTRY-1471:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12828704/SENTRY-1471.001.patch
against master.
{color:red}Overall:{color} -1 due to an error
{color:red}ERROR:{color} failed to build with patch (exit code 1)
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2020/console
This message is automatically generated.
> TestHDFSIntegrationBase.java implements HDFS ACL checking and query
> verification incorrectly
>
>
> Key: SENTRY-1471
> URL: https://issues.apache.org/jira/browse/SENTRY-1471
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
>Reporter: Vadim Spector
>Assignee: Anne Yu
> Attachments: SENTRY-1471.001.patch
>
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When
> the number of failed retries exceeds max value (the "else" portion of "if
> (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means
> that verifyOnAllSubDirsHelper() never returns false.
> Once the problem was fixed in a local development branch, several tests in
> TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End
> tests are still ok.
> Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean
> instead of throwing the original AssertionError which contains information
> about expected vs. found ACL permissions - this information is invaluable for
> debugging. The fix is easy - the to-be-fixed "else" portion is inside the
> "catch (Throwable th)" clause, so it should just re-throw caught exception.
> This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper()
> should have "void" return type.
> The same problem exists in verifyQuery(), also in
> TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also
> assigns "isSucceeded = true;" leading to the same problem - it never fails.
> Suggested solution would be the same - just re-throw the exception. It will
> not only fix the problem, but also make debugging easier. Current code
> "Assert.assertTrue(isSucceeded);" throws away valuable information about the
> expected vs detected number of database rows.
> Note: the above problems were introduced by SENTRY-1454. Suggested fixes
> match the original code prior to SENTRY-1454.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Comment Edited] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494222#comment-15494222 ] Vadim Spector edited comment on SENTRY-1471 at 9/15/16 6:56 PM: Submitting the patch that fixes validation logic. a) testAuthzObjOnPartitionMultipleTables() is definitely broken - line 603, permissions not propagated to /tmp/external. Test marked as @Ignore'd b) testNonHDFSLocations() - in two places calls to verifyOnAllSubDirs() replace "FsAction.ALL" with "null" - - both mentioning SENTRY-546 as justification for code change. They start failing, and reverting to "FsAction.ALL" fixes the test. c) after the above changes, all tests pass I suggest proceeding with this JIRA and opening another one, just to fix testAuthzObjOnPartitionMultipleTables() test. was (Author: vspec...@gmail.com): Submitting the patch that fixes validation logic. a) testAuthzObjOnPartitionMultipleTables() is definitely broken - line 603, permissions not propagated to /tmp/external. Test marked as @Ignore'd b) testNonHDFSLocations() - in two places calls to verifyOnAllSubDirs() replace "FsAction.ALL" with "null" - - both mentioning SENTRY-546 as justification for code change. Well, they actually start failing, and switching back to "FsAction.ALL" fixes the test. c) after the above changes, all tests pass I suggest proceeding with this JIRA and opening another one, just to fix testAuthzObjOnPartitionMultipleTables() test. > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494222#comment-15494222 ] Vadim Spector commented on SENTRY-1471: --- Submitting the patch that fixes validation logic. a) testAuthzObjOnPartitionMultipleTables() is definitely broken - line 603, permissions not propagated to /tmp/external. Test marked as @Ignore'd b) testNonHDFSLocations() - in two places calls to verifyOnAllSubDirs() replace "FsAction.ALL" with "null" - - both mentioning SENTRY-546 as justification for code change. Well, they actually start failing, and switching back to "FsAction.ALL" fixes the test. c) after the above changes, all tests pass I suggest proceeding with this JIRA and opening another one, just to fix testAuthzObjOnPartitionMultipleTables() test. > TestHDFSIntegrationBase.java implements HDFS ACL checking and query > verification incorrectly > > > Key: SENTRY-1471 > URL: https://issues.apache.org/jira/browse/SENTRY-1471 > Project: Sentry > Issue Type: Bug > Components: Sentry >Reporter: Vadim Spector >Assignee: Anne Yu > Attachments: SENTRY-1471.001.patch > > > verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When > the number of failed retries exceeds max value (the "else" portion of "if > (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means > that verifyOnAllSubDirsHelper() never returns false. > Once the problem was fixed in a local development branch, several tests in > TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End > tests are still ok. > Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean > instead of throwing the original AssertionError which contains information > about expected vs. found ACL permissions - this information is invaluable for > debugging. The fix is easy - the to-be-fixed "else" portion is inside the > "catch (Throwable th)" clause, so it should just re-throw caught exception. > This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() > should have "void" return type. > The same problem exists in verifyQuery(), also in > TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also > assigns "isSucceeded = true;" leading to the same problem - it never fails. > Suggested solution would be the same - just re-throw the exception. It will > not only fix the problem, but also make debugging easier. Current code > "Assert.assertTrue(isSucceeded);" throws away valuable information about the > expected vs detected number of database rows. > Note: the above problems were introduced by SENTRY-1454. Suggested fixes > match the original code prior to SENTRY-1454. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
