-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Throw relevent error when query didn't match any selection rule #21904
base: master
Are you sure you want to change the base?
Conversation
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
Resource group can be applied to things other than users (e.g. query type or source) - the new error message would be wrong then. |
@mosabua Can you suggest something better which is accurate yet more clear? |
@@ -118,7 +118,7 @@ public void submit(ManagedQueryExecution queryExecution, SelectionContext<C> sel | |||
public SelectionContext<C> selectGroup(SelectionCriteria criteria) | |||
{ | |||
return configurationManager.get().match(criteria) | |||
.orElseThrow(() -> new TrinoException(QUERY_REJECTED, "Query did not match any selection rule")); | |||
.orElseThrow(() -> new TrinoException(QUERY_REJECTED, "User does not belong to any enabled resource group")); |
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.
Maybe
No matching resource group found with configured selection rules. No resources for query processing available.
Or only part of that even.
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.
No matching resource group found with configured selection rules
looks better. should I go with it ?
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.
sounds good to me
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.
@mosabua please review it.
Description
The error message can be more significant which will help user to understand the query failure reason.
Fixes #21895
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.
( ) Release notes are required, with the following suggested text: