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 2023/01/10 11:35:51 UTC
[GitHub] [superset] EugeneTorap opened a new pull request, #22661: chore: Migrate SqlLab API to API v1
EugeneTorap opened a new pull request, #22661:
URL: https://github.com/apache/superset/pull/22661
### SUMMARY
Created `api/v1/sqllab/`API for future SPA migration of SQL Lab
Migrated `superset/sql_json/` to `/api/v1/sqllab/execute/sql/`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] trhagado commented on pull request #22661: chore: Migrate SqlLab API to API v1
Posted by "trhagado (via GitHub)" <gi...@apache.org>.
trhagado commented on PR #22661:
URL: https://github.com/apache/superset/pull/22661#issuecomment-1480243536
I'm trying to use the SqlLab API that has recently been migrated to api/v1/sqllab and am having some difficulties. I can get the api/v1/sqllab/execute endpoint to work fine but don't seem to be able to successfully do an async execute followed by a api/v1/sqllab/results call to get the resulting data rows of the execute. Also I'd like to get the rows in chunks instead of all at once but am unsure if this is supported. Example Python code of an async execute followed by one or more results calls would be very helpful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #22661: chore: Migrate SqlLab API to API v1
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22661:
URL: https://github.com/apache/superset/pull/22661#issuecomment-1377135235
# [Codecov](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#22661](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bd0389) into [master](https://codecov.io/gh/apache/superset/commit/3ffdad107458db0020193d812270b015a5e4dd98?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3ffdad1) will **decrease** coverage by `11.16%`.
> The diff coverage is `82.20%`.
```diff
@@ Coverage Diff @@
## master #22661 +/- ##
===========================================
- Coverage 67.00% 55.84% -11.17%
===========================================
Files 1859 1868 +9
Lines 71103 71422 +319
Branches 7782 7782
===========================================
- Hits 47643 39883 -7760
- Misses 21434 29513 +8079
Partials 2026 2026
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.58% <82.20%> (+0.12%)` | :arrow_up: |
| python | `57.97% <82.20%> (-23.33%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `51.55% <59.32%> (+0.12%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `63.56% <ø> (ø)` | |
| [superset/sqllab/api.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsbGFiL2FwaS5weQ==) | `74.35% <74.35%> (ø)` | |
| [superset/views/sql\_lab/views.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi92aWV3cy5weQ==) | `58.38% <90.90%> (-5.44%)` | :arrow_down: |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `91.88% <100.00%> (+0.13%)` | :arrow_up: |
| [superset/sqllab/tabs.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsbGFiL3RhYnMucHk=) | `100.00% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `35.09% <100.00%> (-39.95%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/tags/core.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFncy9jb3JlLnB5) | `4.54% <0.00%> (-95.46%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-87.88%)` | :arrow_down: |
| ... and [301 more](https://codecov.io/gh/apache/superset/pull/22661?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a diff in pull request #22661: chore: Migrate SqlLab API to API v1
Posted by GitBox <gi...@apache.org>.
villebro commented on code in PR #22661:
URL: https://github.com/apache/superset/pull/22661#discussion_r1066726396
##########
superset/sqllab/api.py:
##########
@@ -0,0 +1,161 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+import logging
+from typing import Any, cast, Dict, Optional
+
+from flask import request, Response
+from flask_appbuilder.api import BaseApi, expose, protect, safe
+
+from superset import conf, event_logger, is_feature_enabled
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.databases.dao import DatabaseDAO
+from superset.jinja_context import get_template_processor
+from superset.queries.dao import QueryDAO
+from superset.sql_lab import get_sql_results
+from superset.sqllab.command import CommandResult, ExecuteSqlCommand
+from superset.sqllab.command_status import SqlJsonExecutionStatus
+from superset.sqllab.exceptions import (
+ QueryIsForbiddenToAccessException,
+ SqlLabException,
+)
+from superset.sqllab.execution_context_convertor import ExecutionContextConvertor
+from superset.sqllab.query_render import SqlQueryRenderImpl
+from superset.sqllab.sql_json_executer import (
+ ASynchronousSqlJsonExecutor,
+ SqlJsonExecutor,
+ SynchronousSqlJsonExecutor,
+)
+from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext
+from superset.sqllab.tabs import _get_sqllab_tabs
+from superset.sqllab.validators import CanAccessQueryValidatorImpl
+from superset.superset_typing import FlaskResponse
+from superset.utils import core as utils
+from superset.utils.core import get_user_id
+from superset.views.base import json_error_response, json_success
+from superset.views.sql_lab.schemas import SqlJsonPayloadSchema
+
+logger = logging.getLogger(__name__)
+
+
+class SqlLabRestApi(BaseApi):
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ allow_browser_login = True
+ class_permission_name = "SqlLab"
+ resource_name = "sqllab"
+ openapi_spec_tag = "SqlLab"
+
+ @expose("/", methods=["GET"])
+ @protect()
+ @safe
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get",
+ log_to_statsd=True,
+ )
+ def get(self) -> Response:
+ """Assembles SqlLab related information (defaultDbId, tab_state_ids, active_tab)
+ in a single endpoint.
+ """
Review Comment:
We need to add the schema spec here, otherwise we won't get an API spec:
<img width="710" alt="image" src="https://user-images.githubusercontent.com/33317356/211761498-864b918b-a011-4d0d-83d4-3faee7d67a5b.png">
##########
superset/sqllab/tabs.py:
##########
@@ -0,0 +1,79 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict, Optional
+
+from superset import db, is_feature_enabled
+from superset.databases.dao import DatabaseDAO
+from superset.models.sql_lab import Query, TabState
+
+DATABASE_KEYS = [
+ "allow_file_upload",
+ "allow_ctas",
+ "allow_cvas",
+ "allow_dml",
+ "allow_run_async",
+ "allows_subquery",
+ "backend",
+ "database_name",
+ "expose_in_sqllab",
+ "force_ctas_schema",
+ "id",
+ "disable_data_preview",
+]
+
+
+def _get_sqllab_tabs(user_id: Optional[int]) -> Dict[str, Any]:
+ # send list of tab state ids
+ tabs_state = (
+ db.session.query(TabState.id, TabState.label).filter_by(user_id=user_id).all()
+ )
+ tab_state_ids = [str(tab_state[0]) for tab_state in tabs_state]
+ # return first active tab, or fallback to another one if no tab is active
+ active_tab = (
+ db.session.query(TabState)
+ .filter_by(user_id=user_id)
+ .order_by(TabState.active.desc())
+ .first()
+ )
+
+ databases: Dict[int, Any] = {}
+ for database in DatabaseDAO.find_all():
+ databases[database.id] = {
+ k: v for k, v in database.to_json().items() if k in DATABASE_KEYS
+ }
+ databases[database.id]["backend"] = database.backend
+ queries: Dict[str, Any] = {}
+
+ # These are unnecessary if sqllab backend persistence is disabled
+ if is_feature_enabled("SQLLAB_BACKEND_PERSISTENCE"):
Review Comment:
As `SQLLAB_BACKEND_PERSISTENCE` has been enabled for quite some time by default, I wonder if we should just remove the FF and clean up these conditionalities. Thoughts @dpgaspar @betodealmeida ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22661: chore: Migrate SqlLab API to API v1
Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #22661:
URL: https://github.com/apache/superset/pull/22661#issuecomment-1400552409
Superseded by https://github.com/apache/superset/pull/22809, closing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap closed pull request #22661: chore: Migrate SqlLab API to API v1
Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap closed pull request #22661: chore: Migrate SqlLab API to API v1
URL: https://github.com/apache/superset/pull/22661
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org