From 4d41c6d319a55e1fb148b776417ec84b508ef202 Mon Sep 17 00:00:00 2001
From: Simon Belak <simon.belak@gmail.com>
Date: Wed, 20 Sep 2017 14:50:51 +0200
Subject: [PATCH] Add error handling, (test) api endpoint

---
 src/metabase/api/x_ray.clj                    | 30 ++++++++++++++++---
 src/metabase/feature_extraction/async.clj     | 22 +++++++++++---
 .../models/computation_job_result.clj         |  2 +-
 src/metabase/models/interface.clj             |  4 ---
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/src/metabase/api/x_ray.clj b/src/metabase/api/x_ray.clj
index f72c3be15aa..cef67934bae 100644
--- a/src/metabase/api/x_ray.clj
+++ b/src/metabase/api/x_ray.clj
@@ -1,9 +1,12 @@
 (ns metabase.api.x-ray
   (:require [compojure.core :refer [GET]]
             [metabase.api.common :as api]
-            [metabase.feature-extraction.core :as fe]
+            [metabase.feature-extraction
+             [async :as async]
+             [core :as fe]]
             [metabase.models
              [card :refer [Card]]
+             [computation-job :refer [ComputationJob]]
              [field :refer [Field]]
              [metric :refer [Metric]]
              [segment :refer [Segment]]
@@ -50,6 +53,25 @@
                                                  max_computation_cost)})
        fe/x-ray))
 
+(api/defendpoint GET "/async/table/:id"
+  "Get x-ray for a `Tield` with ID."
+  [id max_query_cost max_computation_cost]
+  {max_query_cost       MaxQueryCost
+   max_computation_cost MaxComputationCost}
+  (let [table (api/read-check Table id)]
+    {:job-id (async/compute
+              #(->> table
+                    (fe/extract-features {:max-cost (max-cost max_query_cost
+                                                              max_computation_cost)})
+                    fe/x-ray))}))
+
+(api/defendpoint GET "/async/:id"
+  "Get x-ray for a `Tield` with ID."
+  [id]
+  (->> id
+       ComputationJob
+       async/result))
+
 (api/defendpoint GET "/segment/:id"
   "Get x-ray for a `Segment` with ID."
   [id max_query_cost max_computation_cost]
@@ -179,8 +201,8 @@
   "Get a list of model pairs that can be compared."
   []
   [["field" "field"]
-   ["segment" "segment"
-    "table" "table"
-    "segment" "table"]])
+   ["segment" "segment"]
+   ["table" "table"]
+   ["segment" "table"]])
 
 (api/define-routes)
diff --git a/src/metabase/feature_extraction/async.clj b/src/metabase/feature_extraction/async.clj
index 15cc6d1e331..a98117c8bba 100644
--- a/src/metabase/feature_extraction/async.clj
+++ b/src/metabase/feature_extraction/async.clj
@@ -5,12 +5,12 @@
              [computation-job-result :refer [ComputationJobResult]]]
             [toucan.db :as db]))
 
-(def ^:private running-jobs (atom {}))
+(defonce ^:private running-jobs (atom {}))
 
 (defn done?
   "Is the computation job done?"
   [{:keys [status]}]
-  (= :done status))
+  (#{:done :error} status))
 
 (defn running?
   "Is the computation job still running?"
@@ -27,6 +27,16 @@
     (db/update! ComputationJob id :status :done))
   (swap! running-jobs dissoc id))
 
+(defn- save-error
+  [{:keys [id]} error]
+  (db/transaction
+    (db/insert! ComputationJobResult
+      :job_id     id
+      :permanence :temporary
+      :payload    (Throwable->map error))
+    (db/update! ComputationJob id :status :error))
+  (swap! running-jobs dissoc id))
+
 (defn cancel
   "Cancel computation job (if still running)."
   [{:keys [id] :as job}]
@@ -44,7 +54,11 @@
                         :status     :running
                         :type       :simple-job)
         id  (:id job)]
-    (swap! running-jobs assoc id (future (save-result job (f))))
+    (swap! running-jobs assoc id (future
+                                   (try
+                                     (save-result job (f))
+                                     (catch Exception e
+                                       (save-error job e)))))
     id))
 
 (defn result
@@ -52,7 +66,7 @@
   [job]
   (if (done? job)
     (if-let [result (db/select-one ComputationJobResult :job_id (:id job))]
-      {:status :done
+      {:status (:status job)
        :result (:payload result)}
       {:status :result-not-available})
     {:status (:status job)}))
diff --git a/src/metabase/models/computation_job_result.clj b/src/metabase/models/computation_job_result.clj
index 692ed1c59b1..d1397726a69 100644
--- a/src/metabase/models/computation_job_result.clj
+++ b/src/metabase/models/computation_job_result.clj
@@ -9,5 +9,5 @@
   models/IModel
   (merge models/IModelDefaults
          {:types          (constantly {:permanence :keyword
-                                       :payload    :edn})
+                                       :payload    :json})
           :properties     (constantly {:timestamped? true})}))
diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj
index bc8a937a5dc..04b24be6318 100644
--- a/src/metabase/models/interface.clj
+++ b/src/metabase/models/interface.clj
@@ -37,10 +37,6 @@
   :in  identity
   :out u/jdbc-clob->str)
 
-(models/add-type! :edn
-  :in  pr-str
-  :out (comp edn/read-string u/jdbc-clob->str))
-
 (def ^:private encrypted-json-in  (comp encryption/maybe-encrypt json-in))
 (def ^:private encrypted-json-out (comp json-out encryption/maybe-decrypt))
 
-- 
GitLab