-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Added functionality to only allow one auth method simultaneously in the TrinoHook #53134
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
Conversation
Yes but please add entry to the main of the change log explaining the details of the change for users. You can see examples in some of the providers change logs. |
Just to clarify, should I add the entry under the latest release's 'Bug Fixes' section, or were you thinking of a separate note below the changelog, such that it will be included in the next release? |
That |
|
@dominikhei can you rebase and resolve conflicts? |
Thanks for the reminder, I was on holidays the last 2weeks. Will take care of it. |
…ltiple auth types in the connection
f20bad7 to
3c827d7
Compare
…he TrinoHook (apache#53134) * Added functionality to only allow one auth method simultaneously in the trino Hook * Adjusted the changelog and logging structure of the hook incase of multiple auth types in the connection
…he TrinoHook (apache#53134) * Added functionality to only allow one auth method simultaneously in the trino Hook * Adjusted the changelog and logging structure of the hook incase of multiple auth types in the connection
…he TrinoHook (apache#53134) * Added functionality to only allow one auth method simultaneously in the trino Hook * Adjusted the changelog and logging structure of the hook incase of multiple auth types in the connection
…he TrinoHook (apache#53134) * Added functionality to only allow one auth method simultaneously in the trino Hook * Adjusted the changelog and logging structure of the hook incase of multiple auth types in the connection
I haven't really found a standard way of how multiple auth types in connection are handled, but the Trino provider currently has some opaque behavior, is there an explanation why it is done this way?
currently:
I've simplified the logic so that only one authentication method may be specified per connection. If multiple are provided, an error is raised. This avoids silent misconfigurations and makes the behavior explicit and predictable. If someone thinks, that this is a bad idea and can explain me the rationale behind the current logic, please let me know :)
Question: Could this change be considered a bugfix? I’m hesitant to introduce a breaking change for this issue. If this is considered breaking, I would prefer enhancing logging instead, to clearly inform users which authentication type is actually used