Skip to content
Snippets Groups Projects
Unverified Commit f4e49dbd authored by Case Nelson's avatar Case Nelson Committed by GitHub
Browse files

Validate datasets are found when checking bigquery (#22144)

* Validate datasets are found when checking bigquery

Fixes #19709

* Address PR feedback

Made a general mechanism to pass expected messages to users in
api/database via ex-info. This allows us to suppress logging for
"unexceptional" exceptions that one can expect to hit while setting up
drivers.

* Only validate when filters are set

Also removed the dataset list from the exception as it's not surfaced to
users.
parent 8f9b5957
No related branches found
No related tags found
No related merge requests found
......@@ -55,25 +55,32 @@
(defn- ^Iterable list-tables
"Fetch all tables (new pages are loaded automatically by the API)."
(^Iterable [{{:keys [project-id dataset-filters-type dataset-filters-patterns]} :details, :as database}]
(list-tables (database->client database)
(or project-id (bigquery.common/database-details->credential-project-id (:details database)))
dataset-filters-type
dataset-filters-patterns))
(^Iterable [^BigQuery client ^String project-id ^String filter-type ^String filter-patterns]
(let [datasets (.listDatasets client project-id (u/varargs BigQuery$DatasetListOption))
inclusion-patterns (when (= "inclusion" filter-type) filter-patterns)
exclusion-patterns (when (= "exclusion" filter-type) filter-patterns)]
(apply concat (for [^Dataset dataset (.iterateAll datasets)
:let [^DatasetId dataset-id (.. dataset getDatasetId)]
:when (driver.s/include-schema? inclusion-patterns
exclusion-patterns
(.getDataset dataset-id))]
(-> (.listTables client dataset-id (u/varargs BigQuery$TableListOption))
.iterateAll
.iterator
iterator-seq))))))
(^Iterable [database]
(list-tables database false))
(^Iterable [{{:keys [project-id dataset-filters-type dataset-filters-patterns]} :details, :as database} validate-dataset?]
(list-tables (database->client database)
(or project-id (bigquery.common/database-details->credential-project-id (:details database)))
dataset-filters-type
dataset-filters-patterns
(boolean validate-dataset?)))
(^Iterable [^BigQuery client ^String project-id ^String filter-type ^String filter-patterns ^Boolean validate-dataset?]
(let [datasets (.listDatasets client project-id (u/varargs BigQuery$DatasetListOption))
inclusion-patterns (when (= "inclusion" filter-type) filter-patterns)
exclusion-patterns (when (= "exclusion" filter-type) filter-patterns)
dataset-iter (for [^Dataset dataset (.iterateAll datasets)
:let [^DatasetId dataset-id (.. dataset getDatasetId)]
:when (driver.s/include-schema? inclusion-patterns
exclusion-patterns
(.getDataset dataset-id))]
dataset-id)]
(when (and (not= filter-type "all") validate-dataset? (zero? (count dataset-iter)))
(throw (ex-info (tru "Looks like we cannot find any matching datasets.")
{::driver/can-connect-message? true})))
(apply concat (for [^DatasetId dataset-id dataset-iter]
(-> (.listTables client dataset-id (u/varargs BigQuery$TableListOption))
.iterateAll
.iterator
iterator-seq))))))
(defmethod driver/describe-database :bigquery-cloud-sdk
[_ database]
......@@ -86,8 +93,10 @@
(defmethod driver/can-connect? :bigquery-cloud-sdk
[_ details-map]
;; check whether we can connect by seeing whether listing tables succeeds
(try (some? (list-tables {:details details-map}))
(try (some? (list-tables {:details details-map} ::validate-dataset))
(catch Exception e
(when (::driver/can-connect-message? (ex-data e))
(throw e))
(log/errorf e (trs "Exception caught in :bigquery-cloud-sdk can-connect?"))
false)))
......
......@@ -20,8 +20,9 @@
(deftest can-connect?-test
(mt/test-driver :bigquery-cloud-sdk
(let [db-details (:details (mt/db))
fake-proj-id "definitely-not-a-real-project-id-way-no-how"]
(let [db-details (:details (mt/db))
fake-proj-id "definitely-not-a-real-project-id-way-no-how"
fake-dataset-id "definitely-not-a-real-dataset-id-way-no-how"]
(testing "can-connect? returns true in the happy path"
(is (true? (driver/can-connect? :bigquery-cloud-sdk db-details))))
(testing "can-connect? returns false for bogus credentials"
......@@ -29,7 +30,12 @@
(testing "can-connect? returns true for a valid dataset-id even with no tables"
(with-redefs [bigquery/list-tables (fn [& _]
[])]
(is (true? (driver/can-connect? :bigquery-cloud-sdk db-details))))))))
(is (true? (driver/can-connect? :bigquery-cloud-sdk db-details)))))
(testing "can-connect? returns an appropriate exception message if no datasets are found"
(is (thrown-with-msg? Exception
#"Looks like we cannot find any matching datasets."
(driver/can-connect? :bigquery-cloud-sdk
(assoc db-details :dataset-filters-patterns fake-dataset-id))))))))
(deftest table-rows-sample-test
(mt/test-driver :bigquery-cloud-sdk
......
......@@ -528,7 +528,7 @@
:else
(invalid-response-handler :db (tru "Unable to connect to database.")))
(catch Throwable e
(when log-exception
(when (and log-exception (not (some->> e ex-cause ex-data ::driver/can-connect-message?)))
(log/error e (trs "Cannot connect to Database")))
(invalid-response-handler :dbname (.getMessage e))))))
......
......@@ -247,7 +247,8 @@
"Check whether we can connect to a `Database` with `details-map` and perform a simple query. For example, a SQL
database might try running a query like `SELECT 1;`. This function should return truthy if a connection to the DB
can be made successfully, otherwise it should return falsey or throw an appropriate Exception. Exceptions if a
connection cannot be made."
connection cannot be made. Throw an `ex-info` containing a truthy `::can-connect-message?` in `ex-data`
in order to suppress logging expected driver validation messages during setup."
{:arglists '([driver details])}
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment