chore(sqllab): Cleanup /tables/... endpoint (apache#21284)

preset-io · Sep 13, 2022 · eac6fdc · eac6fdc
1 parent 59437ea
commit eac6fdc
Show file tree

Hide file tree

Showing 29 changed files with 115 additions and 467 deletions.
diff --git a/UPDATING.md b/UPDATING.md
@@ -28,7 +28,7 @@ assists people when migrating to a new version.
 - [20799](https://github.com/apache/superset/pull/20799): Presto and Trino engine will now display tracking URL for running queries in SQL Lab. If for some reason you don't want to show the tracking URL (for example, when your data warehouse hasn't enable access for to Presto or Trino UI), update `TRACKING_URL_TRANSFORMER` in `config.py` to return `None`.
 - [21002](https://github.com/apache/superset/pull/21002): Support Python 3.10 and bump pandas 1.4 and pyarrow 6.
 - [21163](https://github.com/apache/superset/pull/21163): When `GENERIC_CHART_AXES` feature flags set to `True`, the Time Grain control will move below the X-Axis control.
-
+- [21284](https://github.com/apache/superset/pull/21284): The non-functional `MAX_TABLE_NAMES` config key has been removed.
 
 ### Breaking Changes
 

diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json
@@ -3532,9 +3532,6 @@
       },
       "Database": {
         "properties": {
-          "allow_multi_schema_metadata_fetch": {
-            "type": "boolean"
-          },
           "allows_cost_estimate": {
             "type": "boolean"
           },
@@ -3679,10 +3676,6 @@
             "nullable": true,
             "type": "boolean"
           },
-          "allow_multi_schema_metadata_fetch": {
-            "nullable": true,
-            "type": "boolean"
-          },
           "allow_run_async": {
             "nullable": true,
             "type": "boolean"
@@ -3771,10 +3764,6 @@
             "nullable": true,
             "type": "boolean"
           },
-          "allow_multi_schema_metadata_fetch": {
-            "nullable": true,
-            "type": "boolean"
-          },
           "allow_run_async": {
             "nullable": true,
             "type": "boolean"
@@ -3870,10 +3859,6 @@
             "description": "Allow to upload CSV file data into this databaseIf selected, please set the schemas allowed for csv upload in Extra.",
             "type": "boolean"
           },
-          "allow_multi_schema_metadata_fetch": {
-            "description": "Allow SQL Lab to fetch a list of all tables and all views across all database schemas. For large data warehouse with thousands of tables, this can be expensive and put strain on the system.",
-            "type": "boolean"
-          },
           "allow_run_async": {
             "description": "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. Refer to the installation docs for more information.",
             "type": "boolean"
@@ -3971,10 +3956,6 @@
             "description": "Allow to upload CSV file data into this databaseIf selected, please set the schemas allowed for csv upload in Extra.",
             "type": "boolean"
           },
-          "allow_multi_schema_metadata_fetch": {
-            "description": "Allow SQL Lab to fetch a list of all tables and all views across all database schemas. For large data warehouse with thousands of tables, this can be expensive and put strain on the system.",
-            "type": "boolean"
-          },
           "allow_run_async": {
             "description": "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. Refer to the installation docs for more information.",
             "type": "boolean"

diff --git a/...nd/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/birthNames.json b/...nd/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/birthNames.json
@@ -13,7 +13,6 @@
       "id": 1,
       "name": "examples",
       "backend": "postgresql",
-      "allow_multi_schema_metadata_fetch": false,
       "allows_subquery": true,
       "allows_cost_estimate": null,
       "allows_virtual_table_explore": true,

diff --git a/superset-frontend/spec/fixtures/mockDatasource.js b/superset-frontend/spec/fixtures/mockDatasource.js
@@ -171,7 +171,6 @@ export default {
     name: 'birth_names',
     owners: [{ first_name: 'joe', last_name: 'man', id: 1 }],
     database: {
-      allow_multi_schema_metadata_fetch: null,
       name: 'main',
       backend: 'sqlite',
     },

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx
@@ -64,7 +64,6 @@ const store = mockStore({
         allow_cvas: false,
         allow_dml: false,
         allow_file_upload: false,
-        allow_multi_schema_metadata_fetch: false,
         allow_run_async: false,
         backend: 'postgresql',
         database_name: 'examples',

diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx
@@ -31,7 +31,6 @@ const createProps = (): DatabaseSelectorProps => ({
     id: 1,
     database_name: 'test',
     backend: 'test-postgresql',
-    allow_multi_schema_metadata_fetch: false,
   },
   formMode: false,
   isDatabaseSelectEnabled: true,
@@ -69,8 +68,6 @@ beforeEach(() => {
             allow_ctas: 'Allow Ctas',
             allow_cvas: 'Allow Cvas',
             allow_dml: 'Allow Dml',
-            allow_multi_schema_metadata_fetch:
-              'Allow Multi Schema Metadata Fetch',
             allow_run_async: 'Allow Run Async',
             allows_cost_estimate: 'Allows Cost Estimate',
             allows_subquery: 'Allows Subquery',
@@ -92,7 +89,6 @@ beforeEach(() => {
             'allow_ctas',
             'allow_cvas',
             'allow_dml',
-            'allow_multi_schema_metadata_fetch',
             'allow_run_async',
             'allows_cost_estimate',
             'allows_subquery',
@@ -126,7 +122,6 @@ beforeEach(() => {
               allow_ctas: false,
               allow_cvas: false,
               allow_dml: false,
-              allow_multi_schema_metadata_fetch: false,
               allow_run_async: false,
               allows_cost_estimate: null,
               allows_subquery: true,
@@ -147,7 +142,6 @@ beforeEach(() => {
               allow_ctas: false,
               allow_cvas: false,
               allow_dml: false,
-              allow_multi_schema_metadata_fetch: false,
               allow_run_async: false,
               allows_cost_estimate: null,
               allows_subquery: true,
@@ -272,7 +266,6 @@ test('Sends the correct db when changing the database', async () => {
         id: 2,
         database_name: 'test-mysql',
         backend: 'mysql',
-        allow_multi_schema_metadata_fetch: false,
       }),
     ),
   );

diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx
@@ -74,14 +74,12 @@ type DatabaseValue = {
   id: number;
   database_name: string;
   backend: string;
-  allow_multi_schema_metadata_fetch: boolean;
 };
 
 export type DatabaseObject = {
   id: number;
   database_name: string;
   backend: string;
-  allow_multi_schema_metadata_fetch: boolean;
 };
 
 type SchemaValue = { label: string; value: string };
@@ -199,8 +197,6 @@ export default function DatabaseSelector({
             id: row.id,
             database_name: row.database_name,
             backend: row.backend,
-            allow_multi_schema_metadata_fetch:
-              row.allow_multi_schema_metadata_fetch,
           }));
 
           return {

diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx
@@ -31,7 +31,6 @@ const createProps = (props = {}) => ({
     id: 1,
     database_name: 'main',
     backend: 'sqlite',
-    allow_multi_schema_metadata_fetch: false,
   },
   schema: 'test_schema',
   handleError: jest.fn(),

diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx
@@ -112,9 +112,9 @@ export interface TableOption {
 }
 
 export const TableOption = ({ table }: { table: Table }) => {
-  const { label, type, extra } = table;
+  const { value, type, extra } = table;
   return (
-    <TableLabel title={label}>
+    <TableLabel title={value}>
       {type === 'view' ? (
         <Icons.Eye iconSize="m" />
       ) : (
@@ -133,7 +133,7 @@ export const TableOption = ({ table }: { table: Table }) => {
           size="l"
         />
       )}
-      {label}
+      {value}
     </TableLabel>
   );
 };
@@ -286,9 +286,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   );
 
   function renderTableSelect() {
-    const disabled =
-      (currentSchema && !formMode && readOnly) ||
-      (!currentSchema && !database?.allow_multi_schema_metadata_fetch);
+    const disabled = (currentSchema && !formMode && readOnly) || !currentSchema;
 
     const header = sqlLabMode ? (
       <FormLabel>{t('See table schema')}</FormLabel>

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
@@ -148,24 +148,6 @@ const ExtraOptions = ({
                 />
               </div>
             </StyledInputContainer>
-            <StyledInputContainer css={no_margin_bottom}>
-              <div className="input-container">
-                <IndeterminateCheckbox
-                  id="allow_multi_schema_metadata_fetch"
-                  indeterminate={false}
-                  checked={!!db?.allow_multi_schema_metadata_fetch}
-                  onChange={onInputChange}
-                  labelText={t('Allow Multi Schema Metadata Fetch')}
-                />
-                <InfoTooltip
-                  tooltip={t(
-                    'Allow SQL Lab to fetch a list of all tables and all views across all database ' +
-                      'schemas. For large data warehouse with thousands of tables, this can be ' +
-                      'expensive and put strain on the system.',
-                  )}
-                />
-              </div>
-            </StyledInputContainer>
             <StyledInputContainer css={no_margin_bottom}>
               <div className="input-container">
                 <IndeterminateCheckbox

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx
@@ -573,12 +573,6 @@ describe('DatabaseModal', () => {
         name: /allow dml/i,
       });
       const allowDMLText = screen.getByText(/allow dml/i);
-      const allowMultiSchemaMDFetchCheckbox = screen.getByRole('checkbox', {
-        name: /allow multi schema metadata fetch/i,
-      });
-      const allowMultiSchemaMDFetchText = screen.getByText(
-        /allow multi schema metadata fetch/i,
-      );
       const enableQueryCostEstimationCheckbox = screen.getByRole('checkbox', {
         name: /enable query cost estimation/i,
       });
@@ -619,23 +613,20 @@ describe('DatabaseModal', () => {
         checkboxOffSVGs[4],
         checkboxOffSVGs[5],
         checkboxOffSVGs[6],
-        checkboxOffSVGs[7],
         tooltipIcons[0],
         tooltipIcons[1],
         tooltipIcons[2],
         tooltipIcons[3],
         tooltipIcons[4],
         tooltipIcons[5],
         tooltipIcons[6],
-        tooltipIcons[7],
         exposeInSQLLabText,
         allowCTASText,
         allowCVASText,
         CTASCVASLabelText,
         CTASCVASInput,
         CTASCVASHelperText,
         allowDMLText,
-        allowMultiSchemaMDFetchText,
         enableQueryCostEstimationText,
         allowDbExplorationText,
         disableSQLLabDataPreviewQueriesText,
@@ -646,7 +637,6 @@ describe('DatabaseModal', () => {
         allowCTASCheckbox,
         allowCVASCheckbox,
         allowDMLCheckbox,
-        allowMultiSchemaMDFetchCheckbox,
         enableQueryCostEstimationCheckbox,
         allowDbExplorationCheckbox,
         disableSQLLabDataPreviewQueriesCheckbox,
@@ -658,8 +648,8 @@ describe('DatabaseModal', () => {
       invisibleComponents.forEach(component => {
         expect(component).not.toBeVisible();
       });
-      expect(checkboxOffSVGs).toHaveLength(8);
-      expect(tooltipIcons).toHaveLength(8);
+      expect(checkboxOffSVGs).toHaveLength(7);
+      expect(tooltipIcons).toHaveLength(7);
     });
 
     test('renders the "Advanced" - PERFORMANCE tab correctly', async () => {

diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts
@@ -66,7 +66,6 @@ export type DatabaseObject = {
   allow_ctas?: boolean;
   allow_cvas?: boolean;
   allow_dml?: boolean;
-  allow_multi_schema_metadata_fetch?: boolean;
   force_ctas_schema?: string;
 
   // Security

diff --git a/superset/cli/update.py b/superset/cli/update.py
@@ -31,7 +31,6 @@
 
 import superset.utils.database as database_utils
 from superset.extensions import db
-from superset.utils.core import override_user
 from superset.utils.encrypt import SecretsMigrator
 
 logger = logging.getLogger(__name__)
@@ -53,38 +52,6 @@ def set_database_uri(database_name: str, uri: str, skip_create: bool) -> None:
     database_utils.get_or_create_db(database_name, uri, not skip_create)
 
 
-@click.command()
-@with_appcontext
-@click.option(
-    "--username",
-    "-u",
-    default=None,
-    help=(
-        "Specify which user should execute the underlying SQL queries. If undefined "
-        "defaults to the user registered with the database connection."
-    ),
-)
-def update_datasources_cache(username: Optional[str]) -> None:
-    """Refresh sqllab datasources cache"""
-    # pylint: disable=import-outside-toplevel
-    from superset import security_manager
-    from superset.models.core import Database
-
-    with override_user(security_manager.find_user(username)):
-        for database in db.session.query(Database).all():
-            if database.allow_multi_schema_metadata_fetch:
-                print("Fetching {} datasources ...".format(database.name))
-                try:
-                    database.get_all_table_names_in_database(
-                        force=True, cache=True, cache_timeout=24 * 60 * 60
-                    )
-                    database.get_all_view_names_in_database(
-                        force=True, cache=True, cache_timeout=24 * 60 * 60
-                    )
-                except Exception as ex:  # pylint: disable=broad-except
-                    print("{}".format(str(ex)))
-
-
 @click.command()
 @with_appcontext
 def sync_tags() -> None:

diff --git a/superset/config.py b/superset/config.py
@@ -751,9 +751,6 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
 # the SQL Lab UI
 DEFAULT_SQLLAB_LIMIT = 1000
 
-# Maximum number of tables/views displayed in the dropdown window in SQL Lab.
-MAX_TABLE_NAMES = 3000
-
 # Adds a warning message on sqllab save query and schedule query modals.
 SQLLAB_SAVE_WARNING_MESSAGE = None
 SQLLAB_SCHEDULE_WARNING_MESSAGE = None

diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
@@ -174,7 +174,6 @@ class DatabaseSchema(Schema):
     id = fields.Int()
     name = fields.String()
     backend = fields.String()
-    allow_multi_schema_metadata_fetch = fields.Bool()  # pylint: disable=invalid-name
     allows_subquery = fields.Bool()
     allows_cost_estimate = fields.Bool()
     allows_virtual_table_explore = fields.Bool()

diff --git a/superset/databases/api.py b/superset/databases/api.py
@@ -121,7 +121,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "allow_dml",
         "backend",
         "force_ctas_schema",
-        "allow_multi_schema_metadata_fetch",
         "impersonate_user",
         "masked_encrypted_extra",
         "extra",
@@ -136,7 +135,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "allow_ctas",
         "allow_cvas",
         "allow_dml",
-        "allow_multi_schema_metadata_fetch",
         "allow_run_async",
         "allows_cost_estimate",
         "allows_subquery",
@@ -167,7 +165,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "configuration_method",
         "force_ctas_schema",
         "impersonate_user",
-        "allow_multi_schema_metadata_fetch",
         "extra",
         "encrypted_extra",
         "server_cert",