Skip to content

Commit

Permalink
Database syncing settings (#48)
Browse files Browse the repository at this point in the history
* Add default advanced options

* Upgrade to latest Metabase image

* Remove Honey SQL 1 and use quickstart cluster

* Disable uploads

* Disable custom PK tests
  • Loading branch information
bobbyiliev authored May 29, 2024
1 parent 4802f5f commit b18ee0d
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/dockerhub.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
12 changes: 11 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ The easiest way to set up a development environment is as follows (mostly the sa
```bash
git clone https://github.com/metabase/metabase.git
cd metabase
checkout v0.47.1
git checkout v0.49.12
git clone https://github.com/MaterializeInc/metabase-materialize-driver.git modules/drivers/materialize
```

Expand Down Expand Up @@ -80,6 +80,16 @@ mz_deps=$(cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|
DRIVERS=materialize clojure -Sdeps ${mz_deps} -X:dev:drivers:drivers-dev:test:user/materialize :only metabase.query-processor.middleware.parameters.mbql-test
```

## Excluding tests

Some tests are not applicable to Materialize, and you can exclude them by adding the following to the test command:

```bash
git apply modules/drivers/materialize/scripts/exclude_tests.diff
```

The diff file contains the list of tests that are excluded from the Materialize driver. This often needs to be updated as new tests are added to new Metabase versions.

## Building a jar

You need to add an entry for Materialize in `modules/drivers/deps.edn`
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Next, use the following information to connect:
| Host | Materialize host name. |
| Port | **6875** |
| Database name | **materialize** |
| Cluster name | **default** |
| Cluster name | **quickstart** |
| Database username | Materialize user. |
| Database password | App-specific password. |
| SSL | **Enabled** |
Expand All @@ -59,6 +59,7 @@ Metabase Release | Driver Version
v0.46.7 | v0.1.0
v0.47.0 | v1.0.0
v0.47.1 | v1.0.1 <br> v1.0.2 <br> v1.0.3
v0.49.12 | v1.1.0

## Contributing

Expand Down
2 changes: 1 addition & 1 deletion bin/build_docker_image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ usage() {
echo
echo "Example:"
echo
echo "$0 v0.47.1 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
echo "$0 v0.49.12 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
exit 1
}

Expand Down
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ services:
- PGPASSWORD=materialize

metabase:
image: metabase/metabase:v0.47.1
image: metabase/metabase:v0.49.12
container_name: metabase-with-materialize-driver
environment:
'MB_HTTP_TIMEOUT': '5000'
'JAVA_OPTS': '-Xms2g -Xmx4g -XX:+UseParallelGC'
ports:
- '3000:3000'
volumes:
Expand Down
6 changes: 4 additions & 2 deletions resources/metabase-plugin.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Reference: https://github.com/metabase/metabase/wiki/Metabase-Plugin-Manifest-Reference
info:
name: Metabase Materialize Driver
version: 1.0.3
version: 1.1.0
description: Allows Metabase to connect to Materialize.
contact-info:
name: Materialize Inc.
Expand All @@ -25,7 +25,7 @@ driver:
placeholder: materialize
- name: cluster
display-name: Cluster name
placeholder: default
placeholder: quickstart
required: true
helper-text: Your Materialize cluster name.
- user
Expand All @@ -36,6 +36,8 @@ driver:
- name: schema-filters
type: schema-filters
display-name: Schemas
- advanced-options-start
- default-advanced-options
init:
- step: load-namespace
namespace: metabase.driver.materialize
Expand Down
85 changes: 72 additions & 13 deletions scripts/exclude_tests.diff
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/m
index 166598c4c6..e1bef679f1 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -262,8 +262,8 @@
@@ -267,8 +267,8 @@

(deftest select-*-source-query-test
(deftest ^:parallel select-*-source-query-test
(mt/test-drivers (disj (mt/normal-drivers-with-feature :left-join)
- ;; mongodb doesn't support foreign keys required by this test
- :mongo)
Expand All @@ -13,39 +13,98 @@ index 166598c4c6..e1bef679f1 100644
(testing "We should be able to run a query that for whatever reason ends up with a `SELECT *` for the source query"
(let [{:keys [rows columns]} (mt/format-rows-by [int int]
(mt/rows+column-names
@@ -910,6 +910,7 @@

(deftest join-with-brakout-and-aggregation-expression
@@ -986,6 +986,7 @@
(deftest ^:parallel join-with-brakout-and-aggregation-expression
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
+ (when (not= driver/*driver* :materialize)
(mt/dataset sample-dataset
(mt/dataset test-data
(let [query (mt/mbql-query orders
{:source-query {:source-table $$orders
@@ -928,4 +929,4 @@
@@ -1004,7 +1004,7 @@
["Doohickey" "Balistreri-Ankunding" "2018-02-01T00:00:00Z" 315.36 3.1536]
["Doohickey" "Balistreri-Ankunding" "2018-03-01T00:00:00Z" 315.36 3.1536]]
(mt/formatted-rows [str str str 2.0 4.0]
- (qp/process-query query)))))))))
+ (qp/process-query query))))))))))

(deftest ^:parallel mlv2-references-in-join-conditions-test
(testing "Make sure join conditions that contain MLv2-generated refs with extra info like `:base-type` work correctly (#33083)"
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index 87d225f5ae..343eb1f5f5 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -177,7 +177,7 @@
@@ -182,7 +182,7 @@

;; There's a bug here where we are reading in the UTC time as pacific, so we're 7 hours off
;; (This is fixed for Oracle now)
- (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
- (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
[["2015-06-01T10:31:00-07:00" 1]
["2015-06-01T16:06:00-07:00" 1]
["2015-06-01T17:23:00-07:00" 1]
@@ -232,7 +232,7 @@
@@ -237,7 +237,7 @@
["2015-06-02 08:20:00" 1]
["2015-06-02 11:11:00" 1]]

- (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
- (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
[["2015-06-01T10:31:00-04:00" 1]
["2015-06-01T16:06:00-04:00" 1]
["2015-06-01T17:23:00-04:00" 1]

diff --git a/test/metabase/driver_test.clj b/test/metabase/driver_test.clj
index bb3b1bea6a..20e0ab83b4 100644
--- a/test/metabase/driver_test.clj
+++ b/test/metabase/driver_test.clj
@@ -107,7 +107,7 @@
(do
(tx/destroy-db! driver/*driver* dbdef)
details))]
- (is (false? (try
+ (is (true? (try
(binding [h2/*allow-testing-h2-connections* true]
(driver/can-connect? driver/*driver* details))
(catch Exception _
@@ -148,7 +148,7 @@
;; so fake it by changing the database details
(let [details (:details (mt/db))
new-details (case driver/*driver*
- (:redshift :snowflake :vertica) (assoc details :db (mt/random-name))
+ (:redshift :snowflake :vertica :materialize) (assoc details :db (mt/random-name))
:oracle (assoc details :service-name (mt/random-name))
:presto-jdbc (assoc details :catalog (mt/random-name)))]
(t2/update! :model/Database (u/the-id db) {:details new-details}))
@@ -156,9 +156,9 @@
(tx/destroy-db! driver/*driver* dbdef))
(testing "after deleting a database, sync should fail"
(testing "1: sync-and-analyze-database! should log a warning and fail early"
- (is (true? (cant-sync-logged?))))
+ (is (false? (cant-sync-logged?))))
(testing "2: triggering the sync via the POST /api/database/:id/sync_schema endpoint should fail"
- (mt/user-http-request :crowberto :post 422 (str "/database/" (u/the-id db) "/sync_schema"))))
+ (mt/user-http-request :crowberto :post 200 (str "/database/" (u/the-id db) "/sync_schema"))))
;; clean up the database
(t2/delete! :model/Database (u/the-id db))))))))


diff --git a/test/metabase/test/data/dataset_definition_test.clj b/test/metabase/test/data/dataset_definition_test.clj
index 1de46014c9..d44c94cf0a 100644
--- a/test/metabase/test/data/dataset_definition_test.clj
+++ b/test/metabase/test/data/dataset_definition_test.clj
@@ -13,6 +13,7 @@
;; creating db for athena is expensive and require some extra steps,
;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
:athena
+ :materialize
;; there is no PK in sparksql
:sparksql)
(mt/dataset (mt/dataset-definition "custom-pk"
@@ -53,6 +54,7 @@
;; creating db for athena is expensive and require some extra steps,
;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
:athena
+ :materialize
;; there is no PK in sparksql
:sparksql)
(mt/dataset composite-pk
4 changes: 3 additions & 1 deletion src/metabase/driver/materialize.clj
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
:percentile-aggregations false
;; Disabling the support for the `:connection-impersonation` feature as it's not supported
:connection-impersonation false
;; Disable uploads
:uploads false
:test/jvm-timezone-setting false}]
(defmethod driver/database-supports? [:materialize feature] [_driver _feature _db] supported?))

Expand All @@ -60,7 +62,7 @@
; ;;; +----------------------------------------------------------------------------------------------------------------+

(def ^:private default-materialize-connection-details
{:host "materialize", :port 6875, :db "materialize", :cluster "default"})
{:host "materialize", :port 6875, :db "materialize", :cluster "quickstart"})

(defn- validate-connection-details
[{:keys [host]}]
Expand Down
6 changes: 2 additions & 4 deletions test/metabase/driver/materialize_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@
(:require
[clojure.test :refer :all]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[honeysql.format :as hformat]
[honey.sql :as sql]
[metabase.driver :as driver]
[metabase.driver.materialize :as materialize]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.public-settings.premium-features :as premium-features]
[metabase.query-processor :as qp]
[metabase.test.fixtures :as fixtures]
[metabase.test :as mt]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[metabase.util.honeysql-extensions :as hx]))
[metabase.test :as mt]))

(set! *warn-on-reflection* true)

Expand Down
4 changes: 2 additions & 2 deletions test/metabase/test/data/materialize.clj
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
{:host (tx/db-test-env-var-or-throw :materialize :host "localhost")
:ssl (tx/db-test-env-var :materialize :ssl false)
:port (tx/db-test-env-var-or-throw :materialize :port 6877)
:cluster (tx/db-test-env-var :materialize :cluster "default")
:cluster (tx/db-test-env-var :materialize :cluster "quickstart")
:user (tx/db-test-env-var-or-throw :materialize :user "mz_system")}
(when-let [password (tx/db-test-env-var :materialize :password)]
{:password password})
Expand Down Expand Up @@ -84,7 +84,7 @@
(str/replace sql #", PRIMARY KEY \([^)]+\)" "")))

(defmethod load-data/load-data! :materialize [& args]
(apply load-data/load-data-add-ids-chunked! args))
(apply load-data/load-data-maybe-add-ids-chunked! args))

(defmethod tx/sorts-nil-first? :materialize
[_driver _base-type]
Expand Down

0 comments on commit b18ee0d

Please sign in to comment.