You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/11/28 05:08:45 UTC

[GitHub] mistercrunch closed pull request #4961: Deprecate database attribute allow_run_sync

mistercrunch closed pull request #4961: Deprecate database attribute allow_run_sync
URL: https://github.com/apache/incubator-superset/pull/4961
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js
index 99ba6a8e68..9bcbdfe28c 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 a5b8daed46..ed2c974caf 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 63889ad8e6..0ed5e933e3 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 87fd1e6e55..d86084d42e 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 @@
 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 0000000000..cea8ac5caa
--- /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 b21c8c8b4e..d7c71d253a 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 c0960a7f1b..be545b06be 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 a3b2a978af..33d31c1bca 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',
     ]


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org