-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#3489] improvement(hive-catalog): Add user authentication e2e test for Hive catalog #3525
Conversation
@@ -58,7 +58,7 @@ public Object doAs( | |||
ops.getClientPool() | |||
.run( | |||
client -> { | |||
return client.getDelegationToken(realUser.getUserName(), principal.getName()); | |||
return client.getDelegationToken(principal.getName(), realUser.getUserName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a bug, just fixed it by the way.
Got it. |
It's now ready for review. |
@qqqttt123 |
.dropTable(NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, "new_table")); | ||
|
||
// Drop schema | ||
catalog.asSchemas().dropSchema(SCHEMA_NAME, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add assertions about what you expected here. not just simply write some codes here, it is a test code, you have to verify something, not blindly throw exceptions if it fails.
…e2e test for Hive catalog (datastrato#3525) Add e2e tests to test end-to-end user authentication. Verify that user authentication works Fix: datastrato#3489 N/A Test locally.
…e2e test for Hive catalog (datastrato#3525) ### What changes were proposed in this pull request? Add e2e tests to test end-to-end user authentication. ### Why are the changes needed? Verify that user authentication works Fix: datastrato#3489 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Test locally.
What changes were proposed in this pull request?
Add e2e tests to test end-to-end user authentication.
Why are the changes needed?
Verify that user authentication works
Fix: #3489
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
Test locally.