-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Added a way to grant/revoke schema privileges from hive metadata #8436
Added a way to grant/revoke schema privileges from hive metadata #8436
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla. |
@@ -2759,6 +2759,18 @@ public void revokeRoles(ConnectorSession session, Set<String> roles, Set<TrinoPr | |||
return accessControlMetadata.listEnabledRoles(session); |
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.
Please follow https://github.com/trinodb/trino/blob/master/DEVELOPMENT.md#format-git-commit-messages when writing commit messages.
Please avoid merge commits, we always rebase. Also FYI #8411
@@ -2774,6 +2774,18 @@ public void revokeRoles(ConnectorSession session, Set<String> roles, Set<TrinoPr | |||
return accessControlMetadata.listEnabledRoles(session); |
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.
The convention for commit messages is to use imperative mode, like "Add grant/revoke..." and to keep the headline relatively short (I know it can be difficult at times - I struggle with it myself ;) )
(And it's annoying how GitHub does not allow commenting on the commit message)
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla. |
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla. |
/** | ||
* Grants the specified privilege to the specified user on the specified table | ||
*/ | ||
default void grantTablePrivileges(ConnectorSession session, SchemaTableName tableName, Set<Privilege> privileges, HivePrincipal grantee, boolean grantOption) | ||
{ | ||
throw new TrinoException(NOT_SUPPORTED, "This connector does not support grants"); | ||
throw new TrinoException(NOT_SUPPORTED, "This connector does not support grants on tables"); |
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.
nit: I guess this can be captured in a different commit.
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 this is fine, because without this commit the change is not needed. Doing it sooner or later would be awkward to me.
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
No description provided.