starburstdata / metabase-driver

Starburst Metabase driver
Apache License 2.0
65 stars 10 forks source link

Allow users to override the source value #51

Closed aakashnand closed 1 year ago

aakashnand commented 2 years ago

Currently the default value of source value is set to Starburst Metabase 1.0.6 https://github.com/starburstdata/metabase-driver/blob/main/drivers/starburst/src/metabase/driver/implementation/connectivity.clj#L106

When I tried to change this value using advance options from Metabase UI to something else I got this error

Caused by: java.sql.SQLException: Connection property 'source' is both in the URL and an argument
    at io.trino.jdbc.TrinoDriverUri.mergeConnectionProperties(TrinoDriverUri.java:464)
    at io.trino.jdbc.TrinoDriverUri.<init>(TrinoDriverUri.java:130)
    at io.trino.jdbc.TrinoDriverUri.<init>(TrinoDriverUri.java:122)
    at io.trino.jdbc.TrinoDriverUri.create(TrinoDriverUri.java:143)

It would be very useful if we could allow users to change the value but keep the default value to Starburst Metabase 1.0.6

image

leniartek commented 2 years ago

Hi @aakashnand thanks for your suggestion! Are you using Starburst or Trino? Looks like there is a conflict with source field, used in URL (Metronome) and as argument (Trino).

Can you elaborate about your use case? Idea behind the same source for all queries is for the admin to be able to distinguish who is using which tool and which version. If we allow all end users to manipulate the source this can end up in a mess.

aakashnand commented 2 years ago

@leniartek I am using Open source trino version.

Idea behind the same source for all queries is for the admin to be able to distinguish who is using which tool and which version. If we allow all end users to manipulate the source this can end up in a mess.

Apologies for the confusion, the title of the issue is a little bit misleading, by users I mean the "Metabase admins" or "trino-admins" or whoever configures the trino-> Metabase connection.

Currently, it's hardcoded to Starburst Metabase 1.0.6 but I want to be able to change this to simple metabase or anything which is useful in my team's context.

The screenshot I shared is the screen of metebase where the connection is configured, on that screen, there is an option for admins to add additional JDBC properties and the source should be configurable through that option and we can keep the default value as Starburst Metabase 1.0.6

leniartek commented 1 year ago

Hi @aakashnand thanks again for your feedback. We decided not to implement the above feature request.

source field is hardcoded on purpose to allow us to internally identify Client Tool and Version for all incoming queries.

It is easy enough to strip the "Starburst" and "X.X.X" in your downstream reporting and just leave "Metabase".