-
Notifications
You must be signed in to change notification settings - Fork 12
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
Merging release/4.3 to develop #823
Conversation
Fix compilation in ExploreTest.java.
1. Added grant and revoke methods that uses sentry client to grant and revoke privileges on entities to users. 2. Modified tests in BasicAuthorizationTest to test authorization checks for the following entity types. - namespace - dataset - datasetType - streams - no more autogrants in cdap 3. Please use the new runtime option specified in README.
Modified integration tests to use the new authorization model
fix compiler error
Fix checkstyle.
integration-test-remote/pom.xml
Outdated
<dependencies> | ||
<dependency> | ||
<groupId>co.cask.cdap</groupId> | ||
<artifactId>cdap-sentry-model</artifactId> | ||
<version>0.6.0-SNAPSHOT</version> |
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.
what should be the version for these 3 dependencies on develop branch?
@rohitsinha54 @yaojiefeng @poornachandra
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.
I think we can keep it as 0.6.0-SNAPSHOT, this will get removed when we migrate our tests to use wildcard privileges
…htest-to-new-model Migrate old auth test to use the new auth model
Fix compilation.
Bump cdap dependency to 4.3.0
integration-test-remote/pom.xml
Outdated
<dependencies> | ||
<dependency> | ||
<groupId>co.cask.cdap</groupId> | ||
<artifactId>cdap-sentry-model</artifactId> | ||
<version>0.6.0-SNAPSHOT</version> |
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.
Shall we update this to the latest version?
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.
To 0.6.0?
And then this'll get removed later right?
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.
Yeah, it should get removed once the I have refactored the latest integration pr.
Just one comment about the version, rest LGTM |
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.
LGTM
Merging release/4.3 to develop