You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dominikhei (via GitHub)" <gi...@apache.org> on 2025/07/10 12:59:18 UTC
[PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
dominikhei opened a new pull request, #53134:
URL: https://github.com/apache/airflow/pull/53134
---
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:
- password and jwt auth can be set at the same time, but password will be preferred
- password and kerberos can't
- password and cert can't
- jwt and kerberos or cert can, but jwt will be preferred
- kerberos and cert can, but kerberos will be preferred
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #53134:
URL: https://github.com/apache/airflow/pull/53134#issuecomment-3061858594
> or were you thinking of a separate note below the changelog, such that it will be included in the next release?
That
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "dominikhei (via GitHub)" <gi...@apache.org>.
dominikhei commented on code in PR #53134:
URL: https://github.com/apache/airflow/pull/53134#discussion_r2202480625
##########
providers/trino/docs/changelog.rst:
##########
@@ -27,6 +27,11 @@
Changelog
---------
+Bug Fixes
+~~~~~~~~~
+
+* ``Fix authentication behavior in Trino provider by allowing only one auth type per connection (#53134)``
Review Comment:
Thanks for the explanation!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "dominikhei (via GitHub)" <gi...@apache.org>.
dominikhei commented on PR #53134:
URL: https://github.com/apache/airflow/pull/53134#issuecomment-3061294228
> > Question: Could this change be considered a bugfix?
>
> 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](https://github.com/apache/airflow/blob/main/providers/trino/docs/changelog.rst?plain=1#L32), or were you thinking of a separate note [below the changelog](https://github.com/apache/airflow/blob/main/providers/trino/docs/changelog.rst?plain=1#L29), such that it will be included in the next release?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #53134:
URL: https://github.com/apache/airflow/pull/53134#issuecomment-3057370760
> Question: Could this change be considered a bugfix?
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #53134:
URL: https://github.com/apache/airflow/pull/53134
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #53134:
URL: https://github.com/apache/airflow/pull/53134#discussion_r2202380799
##########
providers/trino/docs/changelog.rst:
##########
@@ -27,6 +27,11 @@
Changelog
---------
+Bug Fixes
+~~~~~~~~~
+
+* ``Fix authentication behavior in Trino provider by allowing only one auth type per connection (#53134)``
Review Comment:
That is a note that we automatically generated.
What I meant is something like this
https://github.com/apache/airflow/blob/4c66ebd06029bf28b2cbeb7fed0f0c7f7f74e211/providers/cncf/kubernetes/docs/changelog.rst#L369-L370
a warning that explains the details of the change. while we don't consider this a breaking change some users might be hit here and need to take action so we want these users to have an understanding of what they need to do and why they hit this to begin with so their migration to the newer version would be easier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #53134:
URL: https://github.com/apache/airflow/pull/53134#issuecomment-3149648092
@dominikhei can you rebase and resolve conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added functionality to only allow one auth method simultaneously in the TrinoHook [airflow]
Posted by "dominikhei (via GitHub)" <gi...@apache.org>.
dominikhei commented on PR #53134:
URL: https://github.com/apache/airflow/pull/53134#issuecomment-3150018329
> @dominikhei can you rebase and resolve conflicts?
Thanks for the reminder, I was on holidays the last 2weeks. Will take care of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org