[GitHub] [incubator-superset] villebro commented on a change in pull request #10266: feat: Implement explore functionality even if datasource doesn't exist

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #10266:
URL: https://github.com/apache/incubator-superset/pull/10266#discussion_r453215316



##########
File path: superset/datasets/api.py
##########
@@ -476,3 +479,68 @@ def related_objects(self, pk: int) -> Response:
             charts={"count": len(charts), "result": charts},
             dashboards={"count": len(dashboards), "result": dashboards},
         )
+
+    @expose(
+        "/explore/<int:database_id>/<datasource_type>/<datasource_name>/",
+        methods=["GET"],
+    )
+    @protect()
+    @safe
+    @statsd_metrics
+    def explore(
+        self, database_id: int, datasource_type: str, datasource_name: str
+    ) -> Union[Response, WerkZeugResponse]:
+        """Find or create and explore the dataset
+        ---
+        get:
+          description: >-
+            Finds or creates a datasource definition and redirects to the explore view.
+            Only table datasources are currently supported.
+          parameters:
+          - in: path
+            name: database_id
+            schema:
+              type: integer
+          - in: path
+            name: datasource_type
+            schema:
+              type: string
+          - in: path
+            name: datasource_name
+            schema:
+              type: string
+          responses:
+            302:
+              description: redirect to the superset explore
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        if datasource_type != "table":
+            return self.response_400(
+                f"Datasource type {datasource_type} is not supported."
+            )
+
+        schema_name, table_name = parse_table_full_name(datasource_name)
+        table = DatasetDAO.get_table_or_none(database_id, table_name, schema_name)
+        if table is None:

Review comment:
       Nit (not sure which is preferred, but I tend to do truthy/falsy when possible):
   ```suggestion
           if not table:
   ```

##########
File path: superset/datasets/api.py
##########
@@ -476,3 +479,68 @@ def related_objects(self, pk: int) -> Response:
             charts={"count": len(charts), "result": charts},
             dashboards={"count": len(dashboards), "result": dashboards},
         )
+
+    @expose(
+        "/explore/<int:database_id>/<datasource_type>/<datasource_name>/",
+        methods=["GET"],
+    )
+    @protect()
+    @safe
+    @statsd_metrics
+    def explore(
+        self, database_id: int, datasource_type: str, datasource_name: str
+    ) -> Union[Response, WerkZeugResponse]:
+        """Find or create and explore the dataset
+        ---
+        get:
+          description: >-
+            Finds or creates a datasource definition and redirects to the explore view.
+            Only table datasources are currently supported.
+          parameters:
+          - in: path
+            name: database_id
+            schema:
+              type: integer
+          - in: path
+            name: datasource_type
+            schema:
+              type: string
+          - in: path
+            name: datasource_name
+            schema:
+              type: string
+          responses:
+            302:
+              description: redirect to the superset explore
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'

Review comment:
       It would be preferable to define this as a marshmallow schema. Check #10287 for a recent similar PR.

##########
File path: tests/utils_tests.py
##########
@@ -1344,3 +1345,18 @@ def test_log_this(self) -> None:
             json.loads(record.json)["form_data"]["viz_type"],
             slc.viz.form_data["viz_type"],
         )
+
+
+def test_parse_table_full_name():
+    assert parse_table_full_name("table") == (None, "table")
+    assert parse_table_full_name("table", schema="schema") == ("schema", "table")
+    assert parse_table_full_name("table_schema.table", schema="schema") == (
+        "table_schema",
+        "table",
+    )
+    assert parse_table_full_name("table_schema.table") == ("table_schema", "table")
+    assert parse_table_full_name("hive.table_schema.table") == ("table_schema", "table")
+    assert parse_table_full_name("nonsense.hive.table_schema.table") == (
+        None,
+        "nonsense.hive.table_schema.table",
+    )

Review comment:
       For the sake of uniformity we should probably use `self.assertEqual` here?




----------------------------------------------------------------
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.

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