-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(sqllab): Add /sqllab endpoint to the v1 api #24983
feat(sqllab): Add /sqllab endpoint to the v1 api #24983
Conversation
Thanks! Can you try to fix the tests? |
{"SQLLAB_BACKEND_PERSISTENCE": False}, | ||
clear=True, | ||
) | ||
def test_get_from_bootstrap_data(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this trying to test a scenario where there's no data available? If so, could you improve the test name to something like test_empty_bootstrap_data
?
{"SQLLAB_BACKEND_PERSISTENCE": True}, | ||
clear=True, | ||
) | ||
def test_get_from_backend_persistence_payload(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename to test_bootstrap_data_queries
?
@@ -37,11 +37,76 @@ | |||
from superset.models.sql_lab import Query | |||
|
|||
from tests.integration_tests.base_tests import SupersetTestCase | |||
from tests.integration_tests.fixtures.users import create_gamma_sqllab_no_data | |||
|
|||
QUERIES_FIXTURE_COUNT = 10 | |||
|
|||
|
|||
class TestSqlLabApi(SupersetTestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test for forbidden access?
superset/sqllab/api.py
Outdated
{"result": result}, | ||
default=utils.json_iso_dttm_ser, | ||
ignore_nan=True, | ||
encoding=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encoding=None, |
superset/sqllab/api.py
Outdated
get: | ||
summary: Get the bootstrap data for SqlLab page | ||
description: >- | ||
Assembles SQLLab bootstrap data(active_tab, databases, queries, tab_state_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assembles SQLLab bootstrap data(active_tab, databases, queries, tab_state_ids | |
Assembles SQLLab bootstrap data (active_tab, databases, queries, tab_state_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SUMMARY
Part 1-b of SQLLab SPA migration: detail link
This commit introduces new api endpoint (
/api/v1/sqllab/
) and re-use the sqllab_bootstrap_data util to share the bootstrap data with the legacy landing page.We will migrate the legacy bootstrap util by a command class after the SPA initiative is completed.
TESTING INSTRUCTIONS
1 - Execute the API tests
2 - All tests should pass
ADDITIONAL INFORMATION