Skip to content

Commit

Permalink
Simplifying the flow to add a table (#1068)
Browse files Browse the repository at this point in the history
When specifying a table reference that can not be found, the system used
to still create the object, which would result in confusion and bad
error messages down the line. Now it will fail and not create the
object.

I also removed fields that are not necessary to worry about when
initially creating the table.
  • Loading branch information
mistercrunch authored Sep 9, 2016
1 parent 8eb4cbf commit 6aadc6e
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 19 deletions.
13 changes: 7 additions & 6 deletions caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -889,16 +889,17 @@ def query( # sqla
return QueryResult(
df=df, duration=datetime.now() - qry_start_dttm, query=sql)

def get_sqla_table_object(self):
return self.database.get_table(self.table_name, schema=self.schema)

def fetch_metadata(self):
"""Fetches the metadata for the table and merges it in"""
try:
table = self.database.get_table(self.table_name, schema=self.schema)
except Exception as e:
flasher(str(e))
flasher(
table = self.get_sqla_table_object()
except Exception:
raise Exception(
"Table doesn't seem to exist in the specified database, "
"couldn't fetch column information", "danger")
return
"couldn't fetch column information")

TC = TableColumn # noqa shortcut to class
M = SqlMetric # noqa
Expand Down
35 changes: 22 additions & 13 deletions caravel/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,24 +526,24 @@ class TableModelView(CaravelModelView, DeleteMixin): # noqa
'changed_by_', 'changed_on_']
order_columns = [
'table_link', 'database', 'is_featured', 'changed_on_']
add_columns = [
'table_name', 'database', 'schema',
'default_endpoint', 'offset', 'cache_timeout']
add_columns = ['table_name', 'database', 'schema']
edit_columns = [
'table_name', 'sql', 'is_featured', 'database', 'schema',
'description', 'owner',
'main_dttm_col', 'default_endpoint', 'offset', 'cache_timeout']
related_views = [TableColumnInlineView, SqlMetricInlineView]
base_order = ('changed_on', 'desc')
description_columns = {
'offset': "Timezone offset (in hours) for this datasource",
'schema': (
'offset': _("Timezone offset (in hours) for this datasource"),
'table_name': _(
"Name of the table that exists in the source database"),
'schema': _(
"Schema, as used only in some databases like Postgres, Redshift "
"and DB2"),
'description': Markup(
"Supports <a href='https://daringfireball.net/projects/markdown/'>"
"markdown</a>"),
'sql': (
'sql': _(
"This fields acts a Caravel view, meaning that Caravel will "
"run a query against this string as a subquery."
),
Expand All @@ -561,17 +561,26 @@ class TableModelView(CaravelModelView, DeleteMixin): # noqa
'cache_timeout': _("Cache Timeout"),
}

def post_add(self, table):
table_name = table.table_name
def pre_add(self, table):
# Fail before adding if the table can't be found
try:
table.fetch_metadata()
table.get_sqla_table_object()
except Exception as e:
logging.exception(e)
flash(
"Table [{}] doesn't seem to exist, "
"couldn't fetch metadata".format(table_name),
"danger")
raise Exception(
"Table [{}] could not be found, "
"please double check your "
"database connection, schema, and "
"table name".format(table.table_name))

def post_add(self, table):
table.fetch_metadata()
utils.merge_perm(sm, 'datasource_access', table.perm)
flash(_(
"The table was created. As part of this two phase configuration "
"process, you should now click the edit button by "
"the new table to configure it."),
"info")

def post_update(self, table):
self.post_add(table)
Expand Down

0 comments on commit 6aadc6e

Please sign in to comment.