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.

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

add param proxy user for hive #36221

Merged
merged 4 commits into from
Dec 19, 2023
Merged

Conversation

romsharon98
Copy link
Contributor

@romsharon98 romsharon98 commented Dec 14, 2023

Know there are three options to add proxy user to hive cli connection: by dag owner, by connection logging or by custom proxy (as extra in the connection).

This PR add a fourth option param proxy user. by setting proxy_user=param in connection allow us to pass the user in HiveOperator.
I think this PR is needed from some option:

  1. The user proxy is not as the same name as the owner, we want to seperate the "owner" and the "proxy_user"
  2. If we want to use few HiveOperator with different proxy_user in the same dag

Added an option to sent proxy_param in HiveOperator when allow it in the connection.


^ 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.rst or {issue_number}.significant.rst, in newsfragments.

@romsharon98 romsharon98 force-pushed the feature/hive-proxy-user branch from 26eac1c to 2a25768 Compare December 14, 2023 14:48
@romsharon98 romsharon98 force-pushed the feature/hive-proxy-user branch 3 times, most recently from 483a317 to 9c83bfe Compare December 16, 2023 21:24
@potiuk
Copy link
Member

potiuk commented Dec 17, 2023

Hmm. Isn't it the same as (undocumented in the docstring) run_as ? Would not it be better to just document run_as (or if we think proxy_user is better name also documenting run_as and possibly deprecating it ?

@potiuk
Copy link
Member

potiuk commented Dec 17, 2023

I see it's NOT the same... But I also have. feeling this is a bit cryptic now : when extra_dejson = param => choose proxy user, when owner -> choos run_as. Shoudl we document it better at the very least :) ? Or maybe it's already documented somwhere?

@romsharon98 romsharon98 force-pushed the feature/hive-proxy-user branch from 9c83bfe to c2305d6 Compare December 18, 2023 17:45
@romsharon98
Copy link
Contributor Author

romsharon98 commented Dec 18, 2023

I see it's NOT the same... But I also have. feeling this is a bit cryptic now : when extra_dejson = param => choose proxy user, when owner -> choos run_as. Shoudl we document it better at the very least :) ? Or maybe it's already documented somwhere?

I added a better documentation.
I think this PR should followed by other PR that will change this extra_param called proxy_user.
Because now if we want to use some proxy_user we need to validate this variable in the connection.
I think it should be one place to change to run HQL with proxy_user.
I didn't do it in this PR because probably it will cause breaking changes

@romsharon98 romsharon98 force-pushed the feature/hive-proxy-user branch from c2305d6 to b03716c Compare December 18, 2023 21:07
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Yep. Agree this is a nice thing to add and follow-up with making thigs simpler should come next.

@potiuk potiuk merged commit 135265d into apache:main Dec 19, 2023
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants