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

Use TrinoRequestUser to get the user for the query #524

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Use TrinoRequestUser to get the user for the query #524

merged 1 commit into from
Oct 25, 2024

Conversation

vishalya
Copy link
Member

Description

  • Let's try to use the newly added TrinoRequestUser to extract the user from the query

Additional context and related issues

  • Fix the issue with TrinoRequestUser, the user is not extracted correctly as the whole header is parsed instead of the token

Release notes

  • (X) 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.
  • ( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Oct 11, 2024
@vishalya vishalya requested a review from willmostly October 11, 2024 19:51
@vishalya vishalya closed this Oct 16, 2024
@vishalya vishalya reopened this Oct 17, 2024
@vishalya vishalya force-pushed the userJWT branch 2 times, most recently from 0966803 to 2168a08 Compare October 17, 2024 14:15
@vishalya vishalya force-pushed the userJWT branch 2 times, most recently from 377a688 to c41b0aa Compare October 18, 2024 19:00
@vishalya
Copy link
Member Author

All the review comments are addressed.

@ebyhr ebyhr requested a review from willmostly October 22, 2024 21:43
willmostly
willmostly approved these changes Oct 23, 2024
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - there are a few static imports to add but I think it is ready to merge once those are updated

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestTrinoRequestUser.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/router/TestTrinoRequestUser.java Outdated Show resolved Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/router/TestTrinoRequestUser.java Outdated Show resolved Hide resolved
@ebyhr ebyhr merged commit d8ecc25 into trinodb:main Oct 25, 2024
2 checks passed
@github-actions github-actions bot added this to the 12 milestone Oct 25, 2024
@mosabua mosabua mentioned this pull request Oct 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr left review comments

@willmostly willmostly willmostly approved these changes

Assignees
No one assigned
Labels
cla-signed
Milestone
12
Development

Successfully merging this pull request may close these issues.
3 participants
@vishalya @ebyhr @willmostly