jfrag1 commented on code in PR #23729:
URL: https://github.com/apache/superset/pull/23729#discussion_r1173975410
##########
superset-frontend/src/explore/actions/datasourcesActions.ts:
##########
@@ -49,7 +49,7 @@ export function saveDataset({
const {
json: { data },
} = await SupersetClient.post({
- endpoint: '/superset/sqllab_viz/',
+ endpoint: '/api/v1/dataset/sqllab_viz/',
Review Comment:
This one will need be updated as well
##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -485,102 +485,6 @@ def test_pa_conversion_dict(self):
self.assertEqual(len(data), results.size)
self.assertEqual(len(cols), len(results.columns))
- def test_sqllab_viz(self):
Review Comment:
nit: I think it's fine to leave in tests for deprecated endpoints, we can
just delete them when they are eventually removed for good
##########
superset-frontend/src/explore/actions/datasourcesActions.ts:
##########
@@ -49,7 +49,7 @@ export function saveDataset({
const {
json: { data },
} = await SupersetClient.post({
- endpoint: '/superset/sqllab_viz/',
+ endpoint: '/api/v1/dataset/sqllab_viz/',
Review Comment:
Also since this is explore, the response payload from `POST
/api/v1/dataset/` maybe be missing keys that explore expects to be there in
order to work properly. After this post we may need to make a request to `GET
/api/v1/dataset/{id}`, which will return all needed keys. See here in a recent
PR of mine where I had to do something similar:
https://github.com/apache/superset/pull/23678/files#diff-ccfa293ffd1a07eab82f8646fcb2732ff5d001fec273656b18064db5115ac9fbR165
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -1511,9 +1511,19 @@ export function createDatasourceFailed(err) {
export function createDatasource(vizOptions) {
return dispatch => {
dispatch(createDatasourceStarted());
+ const { dbId, schema, datasourceName, sql } = vizOptions;
return SupersetClient.post({
- endpoint: '/superset/sqllab_viz/',
- postPayload: { data: vizOptions },
+ endpoint: '/api/v1/dataset/',
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({
+ database: dbId,
+ schema: schema,
+ table_name: datasourceName,
+ sql: sql,
+ owners: [1],
Review Comment:
the `owners` key can probably just be omitted from the payload here, then
the backend should default to making the current user owner I believe
##########
superset/datasets/schemas.py:
##########
@@ -239,6 +239,21 @@ class GetOrCreateDatasetSchema(Schema):
template_params = fields.String(description="Template params for the
table")
+class SqllabVizColumnSchema(Schema):
+ name = fields.String(required=True, description="Name of table")
+ type = fields.String(required=True, description="Name of table")
+ is_dttm = fields.Boolean(required=True, description="Name of table")
+
+
+class SqllabVizSchema(Schema):
Review Comment:
these can be removed
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
