superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: Deprecate database attribute allow_run_sync (#4961)
Date Wed, 28 Nov 2018 05:08:49 GMT
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 2931baa  Deprecate database attribute allow_run_sync (#4961)
2931baa is described below

commit 2931baa2940d730f24dd0d5d44ad86488a1a0beb
Author: Maxime Beauchemin <maximebeauchemin@gmail.com>
AuthorDate: Tue Nov 27 21:08:44 2018 -0800

    Deprecate database attribute allow_run_sync (#4961)
    
    * Deprecate database attribute allow_run_sync
    
    There's really 2 modes of operations in SQL Lab, sync or async
    though currently there are 2 boolean flags: allow_run_sync and
    allow_run_async, leading to 4 potential combinations, only 2 of which
    actually makes sense.
    
    The original vision is that we'd expose the choice to users and they
    would decide which `Run` or `Run Async` button to hit.
    Later on we decided to have a
    single button and for each database to be either sync or async.
    
    This PR cleans up allow_run_sync by removing references to it.
    
    * Fix build
    
    * Add db migration
    
    * using batch_op
---
 .../assets/spec/javascripts/sqllab/fixtures.js     |  2 -
 .../src/SqlLab/components/RunQueryActionButton.jsx | 54 +++++++++-------------
 .../assets/src/SqlLab/components/SqlEditor.jsx     |  4 +-
 ...8b9b7_remove_coordinator_from_druid_cluster_.py | 27 +++++------
 .../versions/a61b40f9f57f_remove_allow_run_sync.py | 29 ++++++++++++
 superset/models/core.py                            |  5 +-
 superset/utils/core.py                             |  1 -
 superset/views/core.py                             | 22 ++++-----
 8 files changed, 80 insertions(+), 64 deletions(-)

diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js
index 99ba6a8..9bcbdfe 100644
--- a/superset/assets/spec/javascripts/sqllab/fixtures.js
+++ b/superset/assets/spec/javascripts/sqllab/fixtures.js
@@ -292,7 +292,6 @@ export const databases = {
       allow_ctas: true,
       allow_dml: true,
       allow_run_async: false,
-      allow_run_sync: true,
       database_name: 'main',
       expose_in_sqllab: true,
       force_ctas_schema: '',
@@ -302,7 +301,6 @@ export const databases = {
       allow_ctas: true,
       allow_dml: false,
       allow_run_async: true,
-      allow_run_sync: true,
       database_name: 'Presto - Gold',
       expose_in_sqllab: true,
       force_ctas_schema: 'tmp',
diff --git a/superset/assets/src/SqlLab/components/RunQueryActionButton.jsx b/superset/assets/src/SqlLab/components/RunQueryActionButton.jsx
index a5b8dae..ed2c974 100644
--- a/superset/assets/src/SqlLab/components/RunQueryActionButton.jsx
+++ b/superset/assets/src/SqlLab/components/RunQueryActionButton.jsx
@@ -29,7 +29,28 @@ export default function RunQueryActionButton(props) {
     disabled: !(props.dbId),
   };
 
-  const syncBtn = (
+  if (shouldShowStopBtn) {
+    return (
+      <Button
+        {...commonBtnProps}
+        onClick={props.stopQuery}
+      >
+        <i className="fa fa-stop" /> {t('Stop')}
+      </Button>
+    );
+  } else if (props.allowAsync) {
+    return (
+      <Button
+        {...commonBtnProps}
+        onClick={() => props.runQuery(true)}
+        key="run-async-btn"
+        tooltip={t('Run query asynchronously')}
+        disabled={!props.sql.trim()}
+      >
+        <i className="fa fa-table" /> {runBtnText}
+      </Button>);
+  }
+  return (
     <Button
       {...commonBtnProps}
       onClick={() => props.runQuery(false)}
@@ -40,37 +61,6 @@ export default function RunQueryActionButton(props) {
       <i className="fa fa-refresh" /> {runBtnText}
     </Button>
   );
-
-  const asyncBtn = (
-    <Button
-      {...commonBtnProps}
-      onClick={() => props.runQuery(true)}
-      key="run-async-btn"
-      tooltip={t('Run query asynchronously')}
-      disabled={!props.sql.trim()}
-    >
-      <i className="fa fa-table" /> {runBtnText}
-    </Button>
-  );
-
-  const stopBtn = (
-    <Button
-      {...commonBtnProps}
-      onClick={props.stopQuery}
-    >
-      <i className="fa fa-stop" /> {t('Stop')}
-    </Button>
-  );
-
-  let button;
-  if (shouldShowStopBtn) {
-    button = stopBtn;
-  } else if (props.allowAsync) {
-    button = asyncBtn;
-  } else {
-    button = syncBtn;
-  }
-  return button;
 }
 
 RunQueryActionButton.propTypes = propTypes;
diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx
index 63889ad..0ed5e93 100644
--- a/superset/assets/src/SqlLab/components/SqlEditor.jsx
+++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx
@@ -137,7 +137,9 @@ class SqlEditor extends React.PureComponent {
     this.props.actions.queryEditorSetQueryLimit(this.props.queryEditor, queryLimit);
   }
   runQuery() {
-    this.startQuery(!(this.props.database || {}).allow_run_sync);
+    if (this.props.database) {
+      this.startQuery(this.props.database.allow_run_async);
+    }
   }
   startQuery(runAsync = false, ctas = false) {
     const qe = this.props.queryEditor;
diff --git a/superset/migrations/versions/46f444d8b9b7_remove_coordinator_from_druid_cluster_.py
b/superset/migrations/versions/46f444d8b9b7_remove_coordinator_from_druid_cluster_.py
index 87fd1e6..d86084d 100644
--- a/superset/migrations/versions/46f444d8b9b7_remove_coordinator_from_druid_cluster_.py
+++ b/superset/migrations/versions/46f444d8b9b7_remove_coordinator_from_druid_cluster_.py
@@ -5,27 +5,28 @@ Revises: 4ce8df208545
 Create Date: 2018-11-26 00:01:04.781119
 
 """
+from alembic import op
+import sqlalchemy as sa
 
 # revision identifiers, used by Alembic.
 revision = '46f444d8b9b7'
 down_revision = '4ce8df208545'
 
-from alembic import op
-import sqlalchemy as sa
-import logging
-
 
 def upgrade():
-    try:
-        op.drop_column('clusters', 'coordinator_host')
-        op.drop_column('clusters', 'coordinator_endpoint')
-        op.drop_column('clusters', 'coordinator_port')
-    except Exception as e:
-        # Sqlite does not support drop column
-        logging.warning(str(e))
+    with op.batch_alter_table('clusters') as batch_op:
+        batch_op.drop_column('coordinator_host')
+        batch_op.drop_column('coordinator_endpoint')
+        batch_op.drop_column('coordinator_port')
 
 
 def downgrade():
-    op.add_column('clusters', sa.Column('coordinator_host', sa.String(length=256), nullable=True))
+    op.add_column(
+        'clusters',
+        sa.Column('coordinator_host', sa.String(length=256), nullable=True),
+    )
     op.add_column('clusters', sa.Column('coordinator_port', sa.Integer(), nullable=True))
-    op.add_column('clusters', sa.Column('coordinator_endpoint', sa.String(length=256), nullable=True))
+    op.add_column(
+        'clusters',
+        sa.Column('coordinator_endpoint', sa.String(length=256), nullable=True),
+    )
diff --git a/superset/migrations/versions/a61b40f9f57f_remove_allow_run_sync.py b/superset/migrations/versions/a61b40f9f57f_remove_allow_run_sync.py
new file mode 100644
index 0000000..cea8ac5
--- /dev/null
+++ b/superset/migrations/versions/a61b40f9f57f_remove_allow_run_sync.py
@@ -0,0 +1,29 @@
+"""remove allow_run_sync
+
+Revision ID: a61b40f9f57f
+Revises: 46f444d8b9b7
+Create Date: 2018-11-27 11:53:17.512627
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = 'a61b40f9f57f'
+down_revision = '46f444d8b9b7'
+
+
+def upgrade():
+    with op.batch_alter_table('dbs') as batch_op:
+        batch_op.drop_column('allow_run_sync')
+
+
+def downgrade():
+    op.add_column(
+        'dbs',
+        sa.Column(
+            'allow_run_sync',
+            sa.Integer(display_width=1),
+            autoincrement=False, nullable=True,
+        ),
+    )
diff --git a/superset/models/core.py b/superset/models/core.py
index b21c8c8..d7c71d2 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -629,8 +629,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
     password = Column(EncryptedType(String(1024), config.get('SECRET_KEY')))
     cache_timeout = Column(Integer)
     select_as_create_table_as = Column(Boolean, default=False)
-    expose_in_sqllab = Column(Boolean, default=False)
-    allow_run_sync = Column(Boolean, default=True)
+    expose_in_sqllab = Column(Boolean, default=True)
     allow_run_async = Column(Boolean, default=False)
     allow_csv_upload = Column(Boolean, default=False)
     allow_ctas = Column(Boolean, default=False)
@@ -648,7 +647,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
     perm = Column(String(1000))
     impersonate_user = Column(Boolean, default=False)
     export_fields = ('database_name', 'sqlalchemy_uri', 'cache_timeout',
-                     'expose_in_sqllab', 'allow_run_sync', 'allow_run_async',
+                     'expose_in_sqllab', 'allow_run_async',
                      'allow_ctas', 'allow_csv_upload', 'extra')
     export_children = ['tables']
 
diff --git a/superset/utils/core.py b/superset/utils/core.py
index c0960a7..be545b0 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -837,7 +837,6 @@ def get_or_create_main_db():
         dbobj = models.Database(database_name='main')
     dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
     dbobj.expose_in_sqllab = True
-    dbobj.allow_run_sync = True
     dbobj.allow_csv_upload = True
     db.session.add(dbobj)
     db.session.commit()
diff --git a/superset/views/core.py b/superset/views/core.py
index a3b2a97..33d31c1 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -136,15 +136,15 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):
 # noqa
     edit_title = _('Edit Database')
 
     list_columns = [
-        'database_name', 'backend', 'allow_run_sync', 'allow_run_async',
+        'database_name', 'backend', 'allow_run_async',
         'allow_dml', 'allow_csv_upload', 'expose_in_sqllab', 'creator', 'modified']
     order_columns = [
-        'database_name', 'allow_run_sync', 'allow_run_async', 'allow_dml',
+        'database_name', 'allow_run_async', 'allow_dml',
         'modified', 'allow_csv_upload', 'expose_in_sqllab',
     ]
     add_columns = [
         'database_name', 'sqlalchemy_uri', 'cache_timeout', 'expose_in_sqllab',
-        'allow_run_sync', 'allow_run_async', 'allow_csv_upload',
+        'allow_run_async', 'allow_csv_upload',
         'allow_ctas', 'allow_dml', 'force_ctas_schema', 'impersonate_user',
         'allow_multi_schema_metadata_fetch', 'extra',
     ]
@@ -175,14 +175,13 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):
 # noqa
             'database-urls) '
             'for more information on how to structure your URI.', True),
         'expose_in_sqllab': _('Expose this DB in SQL Lab'),
-        'allow_run_sync': _(
-            'Allow users to run synchronous queries, this is the default '
-            'and should work well for queries that can be executed '
-            'within a web request scope (<~1 minute)'),
         'allow_run_async': _(
-            'Allow users to run queries, against an async backend. '
+            'Operate the database in asynchronous mode, meaning  '
+            'that the queries are executed on remote workers as opposed '
+            'to on the web server itself. '
             'This assumes that you have a Celery worker setup as well '
-            'as a results backend.'),
+            'as a results backend. Refer to the installation docs '
+            'for more information.'),
         'allow_ctas': _('Allow CREATE TABLE AS option in SQL Lab'),
         'allow_dml': _(
             'Allow users to run non-SELECT statements '
@@ -240,8 +239,7 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): 
# noqa
         'sqlalchemy_uri': _('SQLAlchemy URI'),
         'cache_timeout': _('Chart Cache Timeout'),
         'extra': _('Extra'),
-        'allow_run_sync': _('Allow Run Sync'),
-        'allow_run_async': _('Allow Run Async'),
+        'allow_run_async': _('Asynchronous Query Execution'),
         'impersonate_user': _('Impersonate the logged on user'),
         'allow_csv_upload': _('Allow Csv Upload'),
         'modified': _('Modified'),
@@ -311,7 +309,7 @@ class DatabaseAsync(DatabaseView):
     list_columns = [
         'id', 'database_name',
         'expose_in_sqllab', 'allow_ctas', 'force_ctas_schema',
-        'allow_run_async', 'allow_run_sync', 'allow_dml',
+        'allow_run_async', 'allow_dml',
         'allow_multi_schema_metadata_fetch', 'allow_csv_upload',
         'allows_subquery',
     ]


Mime
View raw message