-
Notifications
You must be signed in to change notification settings - Fork 168
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
Support SET SESSION AUTHORIZATION on trino-python-client #349
Support SET SESSION AUTHORIZATION on trino-python-client #349
Conversation
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Baohe Zhang.
|
fe284f3
to
5e34051
Compare
5e34051
to
000e303
Compare
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.
@baohe-zhang Thanks for the PR.
Once the change is available on the server please add some functional tests as well and we can then merge this PR.
@baohe-zhang Seems the change was merged in server. Once Trino 426 with this change is available please add some tests. |
@baohe-zhang The server side change was merged. Can you add some tests here and we can merge this. |
000e303
to
faf00b6
Compare
Rebased to resolve conflicts. We agreed to not add integration tests since it requires configuring a Trino catalog with a connector that supports impersonation and would require some mechanism to build such customized docker image for tests. Verifying that the header is propagated correctly in the requests should be sufficient on the client and testing whether the header behaves is already tested on the server. |
faf00b6
to
5f75870
Compare
Description
This PR is to support
SET SESSION AUTHORIZATION user
andRESET SESSION AUTHORIZATION
sql commands on the python client. Those 2 sql commands are supported on the server side from this PR trinodb/trino#16067Related to issue trinodb/trino#2512
Tested locally with the script:
Non-technical explanation
Release notes
( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: