Skip to content

Add Service Principal OAuth for Databricks. - #33005

Merged
potiuk merged 4 commits into
apache:mainfrom
johnbrandborg:main
Aug 14, 2023
Merged

Add Service Principal OAuth for Databricks.#33005
potiuk merged 4 commits into
apache:mainfrom
johnbrandborg:main

Conversation

@johnbrandborg

@johnbrandborgjohnbrandborg commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Update the Databricks Hook so that a Service Principal OAuth Client ID and Client Secret can be used like a username and password. The Hook will perform the token retrieval as needed.

closes: #32969

@boring-cyborg

Copy link
Copy Markdown

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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@eladkaleladkal changed the title Add Service Principal Oauth for Databricks. apache/airflow#32969Add Service Principal Oauth for Databricks.Aug 4, 2023
@eladkal

Copy link
Copy Markdown
Contributor

cc @alexott
can you take a look?

@alexott

Copy link
Copy Markdown
Contributor

Really we need to prioritize of adoption of new Databricks SDK, although it doesn't have async support yet. Then we'll get all auth support automatically

@jlaneve

Copy link
Copy Markdown
Contributor

Really we need to prioritize of adoption of new Databricks SDK, although it doesn't have async support yet. Then we'll get all auth support automatically

The Databricks SDK is still in beta with an unstable interface, right? I assume we're waiting until the SDK is stable to adopt it?

@alexott

Copy link
Copy Markdown
Contributor

Authentication part is stable, unstable primarily belongs to specific APIs

@alexottalexott left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in general looks good for me, but let unify code, fix tests, etc.

Comment threadairflow/providers/databricks/hooks/databricks_base.py Outdated
Comment threadairflow/providers/databricks/hooks/databricks_base.py Outdated
Comment threadairflow/providers/databricks/hooks/databricks_base.py Outdated
Comment threadairflow/providers/databricks/hooks/databricks_base.py
@johnbrandborgjohnbrandborg changed the title Add Service Principal Oauth for Databricks.Add Service Principal OAuth for Databricks.Aug 7, 2023

@alexottalexott left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor comment on documentation.

Comment threaddocs/apache-airflow-providers-databricks/connections/databricks.rst Outdated
@alexott

Copy link
Copy Markdown
Contributor

@johnbrandborg thank you for doing it! Changes looks good for me

@potiuk

Copy link
Copy Markdown
Member

docs need fixing though

@johnbrandborg

Copy link
Copy Markdown
ContributorAuthor

docs need fixing though

Link name updated to not conflict.

@potiuk
potiuk merged commit 8bf53dd into apache:mainAug 14, 2023
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

amoghrajesh pushed a commit to amoghrajesh/airflow that referenced this pull request Aug 14, 2023
* Add Service Principal Oauth for Databricks. (apache#32969)
* Consolidate OAuth validation and storage (apache#32969)
* Formatting update and added API and Spark to Init Import
* Update documentation with doc ref and note this is for AWS deployments
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 17, 2023
* Add Service Principal Oauth for Databricks. (apache#32969)
* Consolidate OAuth validation and storage (apache#32969)
* Formatting update and added API and Spark to Init Import
* Update documentation with doc ref and note this is for AWS deployments
Sign up for freeto 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.

Databricks support for Service Principal Oauth

5 participants

@johnbrandborg@eladkal@alexott@jlaneve@potiuk