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

Added server side parameters for thrift connection type #577

Merged

Conversation

hanna-liashchuk
Copy link
Contributor

@hanna-liashchuk hanna-liashchuk commented Dec 29, 2022

resolves #387

Description

PR allows passing spark/hive configurations for thrift connections via server_side_parameters in the profiles file.

Checklist

@cla-bot
Copy link

cla-bot bot commented Dec 29, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Hanna Liashchuk.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Dec 29, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Hanna Liashchuk.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@hanna-liashchuk
Copy link
Contributor Author

Hi @rhousewright @drewbanin, could you please review this PR?

@cla-bot
Copy link

cla-bot bot commented Jan 17, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Шкаберда Вадим Миколайович.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla:yes label Jan 17, 2023
@cla-bot
Copy link

cla-bot bot commented Jan 17, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Шкаберда Вадим Миколайович.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

1 similar comment
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Шкаберда Вадим Миколайович.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Jan 19, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Шкаберда Вадим Миколайович.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@hanna-liashchuk hanna-liashchuk force-pushed the add-server-side-params-for-thrift branch from 712dbb9 to ac1402c Compare January 19, 2023 11:09
@cla-bot
Copy link

cla-bot bot commented Jan 19, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Шкаберда Вадим Миколайович.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@hanna-liashchuk hanna-liashchuk force-pushed the add-server-side-params-for-thrift branch from ac1402c to e8bc6cd Compare January 19, 2023 11:13
@cla-bot cla-bot bot added the cla:yes label Jan 19, 2023
Copy link
Contributor

@VShkaberda VShkaberda left a comment

Choose a reason for hiding this comment

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

Description has been added already to the Apache Spark Profile in Dbt docs.

@Fleid
Copy link
Contributor

Fleid commented Feb 15, 2023

Hi @hanna-liashchuk, thanks so much for your contribution.
I see we have a duplicate issue and PR for that exact scenario (#590 and #591).

Your approach is to re-purpose server_side_parameters from the ODBC method. Their approach is to create a new configuration string spark_conf_string that then needs to be parsed.

I have a preference for your approach as it makes the profiles.yml schema consistent across methods.
Any thoughts about that?

@hanna-liashchuk
Copy link
Contributor Author

@Fleid Agree, I think keeping the same name for the parameter is more user and documentation friendly.

@Fleid
Copy link
Contributor

Fleid commented Feb 27, 2023

Hey @hanna-liashchuk, sorry for the delay, but we're still weighing options.
There is a conversation going on in the other PR if you want to chime in ;)
That's #591

@nathaniel-may nathaniel-may requested a review from a team as a code owner March 16, 2023 21:20
@nathaniel-may nathaniel-may self-requested a review March 16, 2023 21:20
@JCZuurmond
Copy link
Collaborator

@Fleid : This one is ready to merge. I expect a rebase is needed on main to fix the CI. Could you or @colin-rogers-dbt fix this?

…hrift

Minor change: line length and commas.
@tanvn
Copy link

tanvn commented Aug 13, 2023

@Fleid @JCZuurmond
Hi, we are using Thrift connections for our project and looking forward to giving this feature a try.
May I know if there is any update on this?
If there is anything I can do, please feel free to let me know.

@colin-rogers-dbt colin-rogers-dbt enabled auto-merge (squash) August 16, 2023 22:37
@colin-rogers-dbt colin-rogers-dbt merged commit cb39fb1 into dbt-labs:main Aug 16, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants