diff --git a/src/metabase/sync.clj b/src/metabase/sync.clj index a69f1af11986c0e9383ca1f43c97d89ac7d16984..5b434aa3d8e563208d3586ee6f8deabf3bfd64fd 100644 --- a/src/metabase/sync.clj +++ b/src/metabase/sync.clj @@ -34,6 +34,26 @@ [:name :string] [:steps [:maybe [:sequential sync-util/StepNameWithMetadata]]]]]]) +(def ^:private phase->fn + {:metadata sync-metadata/sync-db-metadata! + :analyze analyze/analyze-db! + :field-values field-values/update-field-values!}) + +(defn- scan-phases [scan] + (if (not= :full scan) + [:metadata] + [:metadata :analyze :field-values])) + +(defn- do-phase! [database phase] + (let [f (phase->fn phase) + result (f database)] + (if (instance? Throwable result) + ;; do nothing if we're configured to just move on. + (when-not sync-util/*log-exceptions-and-continue?* + ;; but if we didn't expect any suppressed exceptions, rethrow it + (throw result)) + (assoc result :name (name phase))))) + (mu/defn sync-database! :- SyncDatabaseResults "Perform all the different sync operations synchronously for `database`. @@ -49,10 +69,9 @@ {:keys [scan], :or {scan :full}} :- [:maybe [:map [:scan {:optional true} [:maybe [:enum :schema :full]]]]]] (sync-util/sync-operation :sync database (format "Sync %s" (sync-util/name-for-logging database)) - (cond-> [(assoc (sync-metadata/sync-db-metadata! database) :name "metadata")] - (= scan :full) - (conj (assoc (analyze/analyze-db! database) :name "analyze") - (assoc (field-values/update-field-values! database) :name "field-values")))))) + (->> (scan-phases scan) + (keep (partial do-phase! database)) + (doall))))) (mu/defn sync-table! "Perform all the different sync operations synchronously for a given `table`. Since often called on a sequence of diff --git a/test/metabase/sync_test.clj b/test/metabase/sync_test.clj index f279eae78e293e78e62adf57243c06d8bd465ffb..cdaa231ebcf06b3b853045f6e38c5170f28ad250 100644 --- a/test/metabase/sync_test.clj +++ b/test/metabase/sync_test.clj @@ -262,6 +262,21 @@ (is (= (expected-movie-table) movie)) (is (= (expected-studio-table) studio))))))))) +(driver/register! ::sync-database-error-test) + +(defmethod driver/describe-database ::sync-database-error-test + [_driver _database] + (throw (Exception. "OOPS!"))) + +(deftest sync-database!-error-test + (testing "Errors in sync-database! should be caught and handled correctly (#45848)" + (mt/with-temp [Database db {:engine ::sync-database-error-test}] + (binding [sync-util/*log-exceptions-and-continue?* true] + (let [results (sync/sync-database! db)] + (testing "Skips the metadata step" + (is (= ["analyze" "field-values"] + (map :name results))))))))) + ;; !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ;; !! !! ;; !! HEY! Your tests probably don't belong in this namespace! Put them in one appropriate to the specific part of !!