-
Notifications
You must be signed in to change notification settings - Fork 169
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 Databricks profile #2339
Add Databricks profile #2339
Conversation
From discussion with @konstjar on the Atlas WG call, we decided to create a "webapi-databricks" profile and retain the "webapi-spark" profile for backwards compatibility. I'll revise this PR accordingly. |
My filling is that it is not enough to add the profile. There are few places in WebAPI where we have some custom logic for Spark dialect:
Line 106 in 3f9e90c
I think that we should add "jdbc:databricks" handling there. In the past we had few issues with Characterization and Pathway analysis executions. |
Thanks @konstjar - you've raised some important points here with respect to the code base and the "spark" profile. Recalling our conversation on the Atlas WG call, I thought that we wanted to adopt the "databricks" profile since some users may be using the "spark" driver which may target an older build of Apache Spark for Databricks while others may need the newer driver? When setting up the new or old Spark data source in Atlas/WebAPI, the source would be configured with the dbms == "spark" but the connection string might differ based on the driver. So in this way, the only check that requires extension is the one that is looking at the JDBC connection string, correct? In that case we can have a static method to detect the dialect based on either "databricks" or "spark" in the JDBC connection string and that would work? |
@konstjar - I've updated this PR with the idea that when a person configures a data source to use spark/databricks, they would create the entry in the |
@anthonysena Looks good to me. |
Thanks @konstjar - can you approve the PR then? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Per feedback from @konstjar, I've added a new profile "webapi-databricks" to use the Databricks JDBC Driver reference. Some notes on this PR:
com.databricks.client.jdbc.Driver
to the list of supported drivers to support this newer JDBC driver.