8000 Set setRoles after roles are reset in trino-jdbc by homar · Pull Request #25191 · trinodb/trino · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Set setRoles after roles are reset in trino-jdbc #25191

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set setRoles after roles are reset in trino-jdbc #25191

wants to merge 1 commit into from
+184 −3

Conversation

homar
Copy link
Member
@homar homar commented Feb 28, 2025 •
edited
Loading

Description

Information from X-Trino-Set-Role should be stored even after setting or resetting authorized user.
This is also aligned with the order of operations in trino-cli

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Fix roles being cleared after resetting authorization

@cla-bot cla-bot bot added the cla-signed label Feb 28, 2025
@github-actions github-actions bot added the jdbc Relates to Trino JDBC driver label Feb 28, 2025
@hashhar
Copy link
Member
hashhar commented Feb 28, 2025

cc: @damian3031 is the trino-python-client also similarly affected?

@damian3031
Copy link
Member

cc: @damian3031 is the trino-python-client also similarly affected?

In TPC, roles aren't cleared when receiving set/reset authorization user header:
https://github.com/trinodb/trino-python-client/blob/0.333.0/trino/client.py#L727-L731

@homar
Copy link
Member Author
homar commented Feb 28, 2025

cc: @damian3031 is the trino-python-client also similarly affected?

In TPC, roles aren't cleared when receiving set/reset authorization user header: https://github.com/trinodb/trino-python-client/blob/0.333.0/trino/client.py#L727-L731

@damian3031 @hashhar shouldn't we have similar behaviour across different clients?

@hashhar
Copy link
Member
hashhar commented Feb 28, 2025

Yes we should have similar behaviour ideally. That's why i asked.

@homar homar force-pushed the homar/change_order_of_setting_roles_in_trino_connection branch 2 times, most recently from 9cfad82 to 7420510 Compare February 28, 2025 23:22
@homar homar marked this pull request as ready for review February 28, 2025 23:25
@homar
Copy link
Member Author
homar commented Feb 28, 2025

I don't like the test which I implemented but I couldn't have figured out anything better. Ideas are welcome

@homar
Set setRoles after roles are reset in trino-jdbc
b581665 
Information from X-Trino-Set-Role should be stored
even after setting or resetting authorized user.
This is also aligned with the order of operations in
trino-cli
@homar homar force-pushed the homar/change_order_of_setting_roles_in_trino_connection branch from 7420510 to b581665 Compare March 1, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kokosing kokosing Awaiting requested review from kokosing

@hashhar hashhar Awaiting requested review from hashhar

@lukasz-walkiewicz lukasz-walkiewicz Awaiting requested review from lukasz-walkiewicz

Assignees
No one assigned
Labels
cla-signed jdbc Relates to Trino JDBC driver
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@homar @hashhar @damian3031
0