-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Feat: Add username, password to Pinot Connection #46826
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
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
a140da0 to
1f0f9ab
Compare
|
it passed the pre-commit and breeze test. |
|
regular tests fail now. |
a8e579e to
1f0f9ab
Compare
…rflow UI connection behavior) test: set login/password as empty string not to return mock
|
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
* feat: add username, password to Pinot Connection * fix: rename params conn.user to conn.login to align with Airflow Connection * fix: don't add login/password to PinotCLI if None or empty string (Airflow UI connection behavior) test: set login/password as empty string not to return mock
Background:
The
PinotDbApiHookandPinotAdminHookwere previously not handling authentication properly when connecting to a secured Pinot cluster. In scenarios where authentication was required, the hooks would fail to pass login and password details, leading to connection and query failures.Changes:
PinotDbApiHook:
PinotDbApiHookto support authentication for both connection and query execution.loginandpasswordfields from the connection parameters to properly construct the connection URI and ensure authentication when connecting to the Pinot cluster.PinotAdminHook:
PinotAdminHookto handle authentication when running Pinot Admin CLI commands.-userand-passwordflags, using the credentials from the connection object.Test Cases:
PinotDbApiHookandPinotAdminHookto verify the authentication logic:PinotDbApiHook, tests ensure thatloginandpasswordare correctly used to generate the connection URI.PinotAdminHook, a test was added to verify that authentication details are passed to the CLI when running theadd_schemacommand, for example.Impact:
PinotDbApiHookandPinotAdminHookhas been improved, ensuring that the hooks can now properly authenticate with Pinot clusters requiring login and password.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.