You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2018/10/31 04:09:30 UTC
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11837
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
IMPALA-7794: Rewrite flaky ownership authorization tests
This patch rewrites the ownership authorization tests to not depend on
delay and timeout, which can help to avoid the flakiness. The patch also
refactors some tests to reuse Sentry and Impala instances without having
to restart them, which can speed up the tests. To keep the same test
coverage, no tests were removed.
Testing:
- Ran all authorization E2E tests multiple times.
Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
---
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_owner_privileges.py
M tests/common/sentry_cache_test_suite.py
3 files changed, 298 insertions(+), 412 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/11837/3
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1226/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 04:43:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Adam Holley (Code Review)" <ge...@cloudera.org>.
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 17:45:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
IMPALA-7794: Rewrite flaky ownership authorization tests
This patch rewrites the ownership authorization tests to not depend on
delay and timeout, which can help to avoid the flakiness. The patch also
refactors some tests to reuse Sentry and Impala instances without having
to restart them, which can speed up the tests. To keep the same test
coverage, no tests were removed.
Testing:
- Ran all authorization E2E tests multiple times.
Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Reviewed-on: http://gerrit.cloudera.org:8080/11837
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_owner_privileges.py
M tests/common/sentry_cache_test_suite.py
3 files changed, 287 insertions(+), 416 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
IMPALA-7794: Rewrite flaky ownership authorization tests
This patch rewrites the ownership authorization tests to not depend on
delay and timeout, which can help to avoid the flakiness. The patch also
refactors some tests to reuse Sentry and Impala instances without having
to restart them, which can speed up the tests. To keep the same test
coverage, no tests were removed.
Testing:
- Ran all authorization E2E tests multiple times.
Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
---
M tests/authorization/test_grant_revoke.py
M tests/authorization/test_owner_privileges.py
M tests/common/sentry_cache_test_suite.py
3 files changed, 287 insertions(+), 416 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/11837/4
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Nov 2018 05:34:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py@116
PS3, Line 116:
> Could you "for privilege in ["all", "select"]"?
Done
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97
PS3, Line 97: validate_no_user_privileges(s
> You can remove the "query" parameter, and just use the user to run "show gr
Done
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97
PS3, Line 97: e_met
> count always seems to be 0 in usage. Any reason to keep that, or just rena
Done
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py
File tests/common/sentry_cache_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@40
PS3, Line 40: for row in result.data:
> If it's NULL after invalidate, that would be a problem.
Done
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@56
PS3, Line 56:
> You could get rid of the "query" parameter in this method and add an option
I'll keep the query here since it actually makes the tests harder to read since we need "name" and "for_role".
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@68
PS3, Line 68:
> "specified client" - incorrect in original comment.
Done
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 17:04:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Nov 2018 09:28:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Adam Holley (Code Review)" <ge...@cloudera.org>.
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py@116
PS3, Line 116: "all"
Could you "for privilege in ["all", "select"]"?
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97
PS3, Line 97: count
count always seems to be 0 in usage. Any reason to keep that, or just rename to "validate_no_user_privileges"?
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97
PS3, Line 97: validate_user_privilege_count
You can remove the "query" parameter, and just use the user to run "show grant user %s" % user.
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py
File tests/common/sentry_cache_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@40
PS3, Line 40: # We ignore the create_time column since it can be NULL.
If it's NULL after invalidate, that would be a problem.
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@56
PS3, Line 56: validate_privileges
You could get rid of the "query" parameter in this method and add an optional "for_role" parameter for the cases where you want to show role privileges.
http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@68
PS3, Line 68: root user
"specified client" - incorrect in original comment.
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 15:35:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3409/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Nov 2018 05:34:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1229/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 17:45:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7794: Rewrite flaky ownership authorization tests
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Nov 2018 05:32:27 +0000
Gerrit-HasComments: No