From a5bf312a47b802e39076e43ebbb3e312a9d05158 Mon Sep 17 00:00:00 2001 From: gbrian Date: Wed, 6 Apr 2016 17:33:07 +0200 Subject: [PATCH 1/5] Granularity datetime formatting workaround --- caravel/models.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/caravel/models.py b/caravel/models.py index 8bcfb6cce002..b6a8a8c11bcf 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -565,7 +565,20 @@ def query( # sqla select_exprs += [timestamp_grain] groupby_exprs += [timestamp_grain] - tf = '%Y-%m-%d %H:%M:%S.%f' + # UGLY: I guess correct way is to delegate on SQLAlchemy dialect + # UPDATE: Datetime depends on each dialect and I haven't found an easy way to manage + # Maybe we can allow user to define its custome format at Database definition + def get_dtformat(type): + if type == 'SMALLDATETIME' or type == 'DATETIME': + return '%Y-%m-%d %H:%M:%S' + if type == 'DATE': + return '%Y-%m-%d' + if type == 'TIME': + return '%H:%M:%S' + return '%Y-%m-%d %H:%M:%S.%f' + + tf = get_dtformat(cols[granularity].type or 'DATE') + time_filter = [ timestamp >= from_dttm.strftime(tf), timestamp <= to_dttm.strftime(tf), From 0f3105e4670aefe0d6ea50dd44fb868e9455f811 Mon Sep 17 00:00:00 2001 From: gbrian Date: Sun, 10 Apr 2016 08:31:19 +0200 Subject: [PATCH 2/5] Manage "no results" in a different way as other exception --- caravel/assets/javascripts/modules/caravel.js | 6 ++++- caravel/assets/visualizations/big_number.js | 2 +- .../assets/visualizations/directed_force.js | 2 +- caravel/assets/visualizations/heatmap.js | 2 +- caravel/assets/visualizations/sankey.js | 2 +- caravel/assets/visualizations/sunburst.js | 2 +- caravel/assets/visualizations/word_cloud.js | 2 +- caravel/assets/visualizations/world_map.js | 2 +- caravel/utils.py | 4 ++++ caravel/views.py | 23 ++++++++++++------- caravel/viz.py | 8 +++++-- 11 files changed, 37 insertions(+), 18 deletions(-) diff --git a/caravel/assets/javascripts/modules/caravel.js b/caravel/assets/javascripts/modules/caravel.js index 7c9112fcd939..35d251cc6410 100644 --- a/caravel/assets/javascripts/modules/caravel.js +++ b/caravel/assets/javascripts/modules/caravel.js @@ -271,9 +271,13 @@ var px = (function () { $('.query-and-save button').removeAttr('disabled'); always(data); }, - error: function (msg) { + error: function (error) { + var msg = error.responseText; token.find("img.loading").hide(); var err = '
' + msg + '
'; + if(error.getResponseHeader("Caravel-Exception") == "NoResultsException"){ + err = '
' + msg + '
'; + } container.html(err); container.show(); $('span.query').removeClass('disabled'); diff --git a/caravel/assets/visualizations/big_number.js b/caravel/assets/visualizations/big_number.js index a9d2b6331b4e..a957d8249af5 100644 --- a/caravel/assets/visualizations/big_number.js +++ b/caravel/assets/visualizations/big_number.js @@ -13,7 +13,7 @@ function bigNumberVis(slice) { d3.json(slice.jsonEndpoint(), function (error, payload) { //Define the percentage bounds that define color from red to green if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var fd = payload.form_data; diff --git a/caravel/assets/visualizations/directed_force.js b/caravel/assets/visualizations/directed_force.js index a0252067e636..eb1c7703b3ce 100644 --- a/caravel/assets/visualizations/directed_force.js +++ b/caravel/assets/visualizations/directed_force.js @@ -16,7 +16,7 @@ function directedForceVis(slice) { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var links = json.data; diff --git a/caravel/assets/visualizations/heatmap.js b/caravel/assets/visualizations/heatmap.js index c9100e2c0f91..2184c5d7751a 100644 --- a/caravel/assets/visualizations/heatmap.js +++ b/caravel/assets/visualizations/heatmap.js @@ -27,7 +27,7 @@ function heatmapVis(slice) { d3.json(slice.jsonEndpoint(), function (error, payload) { var matrix = {}; if (error) { - slice.error(error.responseText); + slice.error(error); return ''; } var fd = payload.form_data; diff --git a/caravel/assets/visualizations/sankey.js b/caravel/assets/visualizations/sankey.js index 4c85ff1414ba..e1230b595f33 100644 --- a/caravel/assets/visualizations/sankey.js +++ b/caravel/assets/visualizations/sankey.js @@ -36,7 +36,7 @@ function sankeyVis(slice) { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var links = json.data; diff --git a/caravel/assets/visualizations/sunburst.js b/caravel/assets/visualizations/sunburst.js index 25df68aa2d6e..f71b5e983022 100644 --- a/caravel/assets/visualizations/sunburst.js +++ b/caravel/assets/visualizations/sunburst.js @@ -54,7 +54,7 @@ function sunburstVis(slice) { d3.json(slice.jsonEndpoint(), function (error, rawData) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } diff --git a/caravel/assets/visualizations/word_cloud.js b/caravel/assets/visualizations/word_cloud.js index 7efd7c03193c..7f72b9a2072f 100644 --- a/caravel/assets/visualizations/word_cloud.js +++ b/caravel/assets/visualizations/word_cloud.js @@ -8,7 +8,7 @@ function wordCloudChart(slice) { function refresh() { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var data = json.data; diff --git a/caravel/assets/visualizations/world_map.js b/caravel/assets/visualizations/world_map.js index 87f09b30be38..b5ee11c3a084 100644 --- a/caravel/assets/visualizations/world_map.js +++ b/caravel/assets/visualizations/world_map.js @@ -17,7 +17,7 @@ function worldMapChart(slice) { var fd = json.form_data; if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var ext = d3.extent(json.data, function (d) { diff --git a/caravel/utils.py b/caravel/utils.py index a67ea650f224..421ee2fee037 100644 --- a/caravel/utils.py +++ b/caravel/utils.py @@ -258,3 +258,7 @@ def readfile(filepath): with open(filepath) as f: content = f.read() return content + + +class NoResultsException(Exception): + pass diff --git a/caravel/views.py b/caravel/views.py index 267957c89d22..95d83d53ef47 100644 --- a/caravel/views.py +++ b/caravel/views.py @@ -23,6 +23,7 @@ from werkzeug.routing import BaseConverter from caravel import appbuilder, db, models, viz, utils, app, sm, ascii_art +from caravel.utils import NoResultsException config = app.config log_this = models.Log.log_this @@ -36,12 +37,12 @@ def validate_json(form, field): # noqa raise ValidationError("json isn't valid") -def generate_download_headers(extension): +def generate_download_headers(extension, headers=None): filename = datetime.now().strftime("%Y%m%d_%H%M%S") content_disp = "attachment; filename={}.{}".format(filename, extension) - headers = { - "Content-Disposition": content_disp, - } + if headers is None: + headers = {} + headers["Content-Disposition"] = content_disp return headers @@ -477,18 +478,24 @@ def explore(self, datasource_type, datasource_id): slice=slc) if request.args.get("json") == "true": status = 200 + headers = {} try: payload = obj.get_json() except Exception as e: + t = type(e) + headers["Caravel-Exception"] = t.__name__ logging.exception(e) - if config.get("DEBUG"): - raise e - payload = str(e) + if t is NoResultsException: + payload = "No results" + else: + if config.get("DEBUG"): + raise e + payload = str(e) status = 500 resp = Response( payload, status=status, - headers=generate_download_headers("json"), + headers=generate_download_headers("json", headers), mimetype="application/json") return resp elif request.args.get("csv") == "true": diff --git a/caravel/viz.py b/caravel/viz.py index 20224dc44693..0c6bfa0b86e8 100644 --- a/caravel/viz.py +++ b/caravel/viz.py @@ -23,6 +23,8 @@ from six import string_types +from caravel.utils import NoResultsException + config = app.config @@ -135,8 +137,10 @@ def get_df(self, query_obj=None): self.results = self.datasource.query(**query_obj) self.query = self.results.query df = self.results.df - if df is None or df.empty: - raise Exception("No data, review your incantations!") + if df is None: + raise Exception("Error retrieving data") + elif df.empty: + raise NoResultsException() else: if 'timestamp' in df.columns: df.timestamp = pd.to_datetime(df.timestamp, utc=False) From 9ae72b8b204fabfc355a84e605b9109c14cf06da Mon Sep 17 00:00:00 2001 From: gbrian Date: Wed, 6 Apr 2016 17:33:07 +0200 Subject: [PATCH 3/5] Granularity datetime formatting workaround --- caravel/models.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/caravel/models.py b/caravel/models.py index ed95467e50e9..89736a9b9976 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -576,7 +576,20 @@ def query( # sqla select_exprs += [timestamp_grain] groupby_exprs += [timestamp_grain] - tf = '%Y-%m-%d %H:%M:%S.%f' + # UGLY: I guess correct way is to delegate on SQLAlchemy dialect + # UPDATE: Datetime depends on each dialect and I haven't found an easy way to manage + # Maybe we can allow user to define its custome format at Database definition + def get_dtformat(type): + if type == 'SMALLDATETIME' or type == 'DATETIME': + return '%Y-%m-%d %H:%M:%S' + if type == 'DATE': + return '%Y-%m-%d' + if type == 'TIME': + return '%H:%M:%S' + return '%Y-%m-%d %H:%M:%S.%f' + + tf = get_dtformat(cols[granularity].type or 'DATE') + time_filter = [ timestamp >= from_dttm.strftime(tf), timestamp <= to_dttm.strftime(tf), From 60b6a39420353ba6796d8ef9ea139fbd07ff495d Mon Sep 17 00:00:00 2001 From: gbrian Date: Sun, 10 Apr 2016 08:31:19 +0200 Subject: [PATCH 4/5] Manage "no results" in a different way as other exception --- caravel/assets/javascripts/modules/caravel.js | 6 ++++- caravel/assets/visualizations/big_number.js | 2 +- .../assets/visualizations/directed_force.js | 2 +- caravel/assets/visualizations/heatmap.js | 2 +- caravel/assets/visualizations/sankey.js | 2 +- caravel/assets/visualizations/sunburst.js | 2 +- caravel/assets/visualizations/word_cloud.js | 2 +- caravel/assets/visualizations/world_map.js | 2 +- caravel/utils.py | 4 ++++ caravel/views.py | 23 ++++++++++++------- caravel/viz.py | 9 ++++++-- 11 files changed, 38 insertions(+), 18 deletions(-) diff --git a/caravel/assets/javascripts/modules/caravel.js b/caravel/assets/javascripts/modules/caravel.js index 88c11e7967fd..000631d55101 100644 --- a/caravel/assets/javascripts/modules/caravel.js +++ b/caravel/assets/javascripts/modules/caravel.js @@ -272,9 +272,13 @@ var px = (function () { $('.query-and-save button').removeAttr('disabled'); always(data); }, - error: function (msg) { + error: function (error) { + var msg = error.responseText; token.find("img.loading").hide(); var err = '
' + msg + '
'; + if(error.getResponseHeader("Caravel-Exception") == "NoResultsException"){ + err = '
' + msg + '
'; + } container.html(err); container.show(); $('span.query').removeClass('disabled'); diff --git a/caravel/assets/visualizations/big_number.js b/caravel/assets/visualizations/big_number.js index 676c9b46c225..6d4dc62b3500 100644 --- a/caravel/assets/visualizations/big_number.js +++ b/caravel/assets/visualizations/big_number.js @@ -13,7 +13,7 @@ function bigNumberVis(slice) { d3.json(slice.jsonEndpoint(), function (error, payload) { //Define the percentage bounds that define color from red to green if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var fd = payload.form_data; diff --git a/caravel/assets/visualizations/directed_force.js b/caravel/assets/visualizations/directed_force.js index a0252067e636..eb1c7703b3ce 100644 --- a/caravel/assets/visualizations/directed_force.js +++ b/caravel/assets/visualizations/directed_force.js @@ -16,7 +16,7 @@ function directedForceVis(slice) { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var links = json.data; diff --git a/caravel/assets/visualizations/heatmap.js b/caravel/assets/visualizations/heatmap.js index 55ec7ef46efd..7209b0b71051 100644 --- a/caravel/assets/visualizations/heatmap.js +++ b/caravel/assets/visualizations/heatmap.js @@ -23,7 +23,7 @@ function heatmapVis(slice) { d3.json(slice.jsonEndpoint(), function (error, payload) { var matrix = {}; if (error) { - slice.error(error.responseText); + slice.error(error); return ''; } var fd = payload.form_data; diff --git a/caravel/assets/visualizations/sankey.js b/caravel/assets/visualizations/sankey.js index 4c85ff1414ba..e1230b595f33 100644 --- a/caravel/assets/visualizations/sankey.js +++ b/caravel/assets/visualizations/sankey.js @@ -36,7 +36,7 @@ function sankeyVis(slice) { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var links = json.data; diff --git a/caravel/assets/visualizations/sunburst.js b/caravel/assets/visualizations/sunburst.js index 25df68aa2d6e..f71b5e983022 100644 --- a/caravel/assets/visualizations/sunburst.js +++ b/caravel/assets/visualizations/sunburst.js @@ -54,7 +54,7 @@ function sunburstVis(slice) { d3.json(slice.jsonEndpoint(), function (error, rawData) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } diff --git a/caravel/assets/visualizations/word_cloud.js b/caravel/assets/visualizations/word_cloud.js index 7efd7c03193c..7f72b9a2072f 100644 --- a/caravel/assets/visualizations/word_cloud.js +++ b/caravel/assets/visualizations/word_cloud.js @@ -8,7 +8,7 @@ function wordCloudChart(slice) { function refresh() { d3.json(slice.jsonEndpoint(), function (error, json) { if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var data = json.data; diff --git a/caravel/assets/visualizations/world_map.js b/caravel/assets/visualizations/world_map.js index 87f09b30be38..b5ee11c3a084 100644 --- a/caravel/assets/visualizations/world_map.js +++ b/caravel/assets/visualizations/world_map.js @@ -17,7 +17,7 @@ function worldMapChart(slice) { var fd = json.form_data; if (error !== null) { - slice.error(error.responseText); + slice.error(error); return ''; } var ext = d3.extent(json.data, function (d) { diff --git a/caravel/utils.py b/caravel/utils.py index 891964e0d523..492b421e8107 100644 --- a/caravel/utils.py +++ b/caravel/utils.py @@ -224,3 +224,7 @@ def readfile(filepath): with open(filepath) as f: content = f.read() return content + + +class NoResultsException(Exception): + pass diff --git a/caravel/views.py b/caravel/views.py index 738f80f9d50d..c9dfba1de8d4 100644 --- a/caravel/views.py +++ b/caravel/views.py @@ -27,6 +27,7 @@ from wtforms.validators import ValidationError from caravel import appbuilder, db, models, viz, utils, app, sm, ascii_art +from caravel.utils import NoResultsException config = app.config log_this = models.Log.log_this @@ -40,12 +41,12 @@ def validate_json(form, field): # noqa raise ValidationError("json isn't valid") -def generate_download_headers(extension): +def generate_download_headers(extension, headers=None): filename = datetime.now().strftime("%Y%m%d_%H%M%S") content_disp = "attachment; filename={}.{}".format(filename, extension) - headers = { - "Content-Disposition": content_disp, - } + if headers is None: + headers = {} + headers["Content-Disposition"] = content_disp return headers @@ -481,18 +482,24 @@ def explore(self, datasource_type, datasource_id): slice=slc) if request.args.get("json") == "true": status = 200 + headers = {} try: payload = obj.get_json() except Exception as e: + t = type(e) + headers["Caravel-Exception"] = t.__name__ logging.exception(e) - if config.get("DEBUG"): - raise e - payload = str(e) + if t is NoResultsException: + payload = "No results" + else: + if config.get("DEBUG"): + raise e + payload = str(e) status = 500 resp = Response( payload, status=status, - headers=generate_download_headers("json"), + headers=generate_download_headers("json", headers), mimetype="application/json") return resp elif request.args.get("csv") == "true": diff --git a/caravel/viz.py b/caravel/viz.py index 9592235e36b4..efb867fe673d 100644 --- a/caravel/viz.py +++ b/caravel/viz.py @@ -26,6 +26,9 @@ from caravel import app, utils, cache from caravel.forms import FormFactory + +from caravel.utils import NoResultsException + config = app.config @@ -139,8 +142,10 @@ def get_df(self, query_obj=None): self.results = self.datasource.query(**query_obj) self.query = self.results.query df = self.results.df - if df is None or df.empty: - raise Exception("No data, review your incantations!") + if df is None: + raise Exception("Error retrieving data") + elif df.empty: + raise NoResultsException() else: if 'timestamp' in df.columns: df.timestamp = pd.to_datetime(df.timestamp, utc=False) From 0c0b6f221a446cdeac5239742b7c76ceb93f18fc Mon Sep 17 00:00:00 2001 From: gbrian Date: Sun, 10 Apr 2016 09:24:09 +0200 Subject: [PATCH 5/5] jslint --- caravel/assets/javascripts/modules/caravel.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/caravel/assets/javascripts/modules/caravel.js b/caravel/assets/javascripts/modules/caravel.js index 000631d55101..07059b20bd16 100644 --- a/caravel/assets/javascripts/modules/caravel.js +++ b/caravel/assets/javascripts/modules/caravel.js @@ -276,8 +276,8 @@ var px = (function () { var msg = error.responseText; token.find("img.loading").hide(); var err = '
' + msg + '
'; - if(error.getResponseHeader("Caravel-Exception") == "NoResultsException"){ - err = '
' + msg + '
'; + if (error.getResponseHeader("Caravel-Exception") === "NoResultsException") { + err = '
' + msg + '
'; } container.html(err); container.show();