Skip to content

Conversation

@AlexVialaBellander
Copy link

@AlexVialaBellander AlexVialaBellander commented Sep 2, 2024

This PR implements the OAuth changes implemented by shrodingers at Brigad.

So the credits are a bit complicated, but originally, this driver was developed by Fernando Goncalves and Rajesh Kumar Ravi. Their original repository is no longer around. However, github user relferreira, kindly updates it here. However, his solution
does not allow for OAuth Secrets, which was something solved by shrodingers at Brigad.

Thus, this work is a combination of two somewhat actively maintained repositories. All that I do is to merge the two solutions and update the driver to work with Metabase 0.50.23.

Best here is if we could separate the two drivers and have this become the central repo for databricks metabase.

Stefani and others added 12 commits January 30, 2024 18:12
Co-authored-by: Stefani <>
* fix: bad aliasing

* feat: added unity capabilities to metabase

---------

Co-authored-by: Stefani <>
* fix: bad aliasing

* feat: utilities + version

* chore: handle databricks V2

* chore :added ignore

* remove jar

* fix: respect metabase version guideline

---------

Co-authored-by: Stefani <>
* fix: bad aliasing

* feat: utilities + version

* chore: handle databricks V2

* chore :added ignore

* remove jar

* fix: respect metabase version guideline

* fix: class naming and loading for name clashes

---------

Co-authored-by: Stefani <>
* fix: pr comms

* fix: jar naming

---------

Co-authored-by: Stefani <>
* fix: bad aliasing

* fix: redme updated

---------

Co-authored-by: Stefani <>
As described in
https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/java/sql/Driver.html#connect(java.lang.String,java.util.Properties),
when connect is called on a driver, it should return nil if it
realizes it is the wrong kind of driver to connect to the given
URL. With this change, when this happens, nil is passed on to the
driver manager so that it can try other drivers.
@AlexVialaBellander AlexVialaBellander changed the title Origin/updated Update to 50.23 and implement Brigad OAuth Databricks Driver Changes Sep 2, 2024
@sos3k
Copy link

sos3k commented Sep 13, 2024

is it possible to optimise the usage of sql serverless warehouses to do not perform select 1 and describe table actions so frequently? that makes the cost of using serverless warehouses significantly high. Do you have any estimate when this PR will be merged into the master? we are having issues with postgres database constraints when trying to sync databricks UC tables in metabase due to the constraints violation

@shrodingers
Copy link

Oh just realized this PR was up for 3 weeks !
Would gladly help here if needed, would be nice to see that merged in teh main repo ! (and to eventually backport some updates made in the fork we did)
For the usage @sos3k , we disabled auto-refreshes and caching by metabase, it is not tied to the driver but the way metabase handles schema updates, it can totally be disabled or set to only run on daily (we had the same cost issue on first deploy :) )

:right-join true
:left-join true
:inner-join true
:window-functions/offset true}]

Choose a reason for hiding this comment

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

Suggested change
:window-functions/offset true}]
:window-functions/offset false}]

metabase/metabase#40991 -> There is some work to be done on the driver to enable window functions I think

Copy link
Author

Choose a reason for hiding this comment

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

I got offset to work in this build. But maybe there are edgecases?

Choose a reason for hiding this comment

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

i didnt test, but seems that there is some work to be done to make it work on the window function side (at least for sparksql driver on the main repo)
If it works thats for the best then !

@AlexVialaBellander
Copy link
Author

main on my repo works but just bumping to 0.50.26 doesnt, see here:
AlexVialaBellander#1

@borft
Copy link
Contributor

borft commented Sep 24, 2024

main on my repo works but just bumping to 0.50.26 doesnt, see here: AlexVialaBellander#1

Hmm, you are right,my bad, same problem here. It works fine for 0.50.25 though. (

@@ -1,5 +1,5 @@
#Copied from https://github.com/dacort/metabase-athena-driver/blob/d7572cd99551ea998a011f8f00a1e39c1eaa59b8/Dockerfile
ARG METABASE_VERSION=v0.46.6.2
ARG METABASE_VERSION=v0.50.26
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, thi suggestion might have been a mistake. I figured that if latest master of relferrerira built againt 0.50.26, this would as well

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, fair assumption tbh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants