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 2022/07/21 00:37:35 UTC

[GitHub] [superset] ktmud opened a new pull request, #20799: feat(sql lab): display presto and trino tracking url

ktmud opened a new pull request, #20799:
URL: https://github.com/apache/superset/pull/20799

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   
   Enable tracking URL for Presto and Trino (previously it is only available for Hive queries) and also change `TRACKING_URL_TRANSFORMER` to run at runtime (as opposed to when queries are fetched) so that we can display different tracking URL based on query age (most tracking URLs will expire after a certain amount of time---in Trino/Preso, this is configured by [`query.min-expire-age`](https://trino.io/docs/current/admin/properties-query-management.html#query-min-expire-age).
   
   Had to update query execution logics to add end time for failed executions, too.
   
   While debugging, I also realized the TIMED_OUT status isn't actually in use---and now the frontend also cannot handle that status. We should probably add it back, but let's save it for another PR.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   When queries are still running:
   
   <img width="746" alt="image" src="https://user-images.githubusercontent.com/335541/180104968-4bf67e62-5b7f-4105-bebb-a1f5d6f8ccc5.png">
   
   
   We will now display the tracking url even for failed jobs, this is useful for users to debug their failed queries:
   
   <img width="837" alt="Xnip2022-07-20_14-46-55" src="https://user-images.githubusercontent.com/335541/180104883-b13f72b4-4a88-443d-aa15-cf39edf7715f.png">
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   1. Connect to a Presto or Trino dataset and run some long-running or erroneous queries
   2. Optionally, update Superset config to test customize `TRACKING_URL_TRANSFORMER`. For example, following setting will only display tracking urls for queries finished within the last 5 minutes:
      ```python
       def TRACKING_URL_TRANSFORMER(url: str, query: "Query") -> Optional[str]:
           if not query.end_time or (
               now_as_float() - float(query.end_time)
               < timedelta(minutes=5).total_seconds() * 1000
           ):
               return re.sub(
                   r"^https?://([^/]+)/", "https://trino.datalake.example.org/", url
               )
           return None
      ```
   
   ### 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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. `[QUERY_1, QUERY_2, QUERY_3]` are executed consequently, with supposedly increasing `start_time`. 
   
   Since the original code `int(...)` basically rounded down the float epoch, the actual query results returned `[QUERY_1, QUERY_2]`---even though we filtered `second_query_time` by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time, so if QUERY_1 and QUERY_2 are executed within the same second, we wouldn't have QUERY_2 in the filtered results. E.g. when the start time for the queries are `1.1, 1.5, 2`---we'd be filtered to between `[1, 1]`.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r927024431


##########
superset/models/sql_lab.py:
##########
@@ -279,6 +284,27 @@ def default_endpoint(self) -> str:
     def get_extra_cache_keys(query_obj: Dict[str, Any]) -> List[str]:
         return []
 
+    @property
+    def tracking_url(self) -> Optional[str]:

Review Comment:
   @dpgaspar @betodealmeida do you have any recommendation on what's the best practice for such use case?



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r926249376


##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +964,22 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        if cursor.last_query_id:
+            # pylint: disable=protected-access
+            return (
+                f"{cursor._protocol}://{cursor._host}:{cursor._port}"

Review Comment:
   Pylint complains line too long.



##########
superset/models/sql_lab.py:
##########
@@ -279,6 +284,27 @@ def default_endpoint(self) -> str:
     def get_extra_cache_keys(query_obj: Dict[str, Any]) -> List[str]:
         return []
 
+    @property
+    def tracking_url(self) -> Optional[str]:

Review Comment:
   I want the FAB-based API to always return the transformed url but I couldn't find a clean way to do it without adding an (unnecessary) new field.



##########
superset/sql_lab.py:
##########
@@ -96,8 +96,13 @@ def handle_query_error(
     msg = f"{prefix_message} {str(ex)}".strip()
     troubleshooting_link = config["TROUBLESHOOTING_LINK"]
     query.error_message = msg
-    query.status = QueryStatus.FAILED
     query.tmp_table_name = None
+    query.status = QueryStatus.FAILED
+    # TODO: re-enable this after updating the frontend to properly display timeout status
+    # if query.status != QueryStatus.TIMED_OUT:
+    #   query.status = QueryStatus.FAILED

Review Comment:
   Timed out query should be marked as timed out---since there was that TIMED_OUT status in the code we must have intended to use it. Currently they are just always marked as failed and the frontend cannot really display a timed out query properly.
   
   I added this comment as a reminder but didn't really change the behavior here. Only "end_time" was added for failed and stopped queries.



##########
superset/db_engine_specs/trino.py:
##########
@@ -109,8 +114,24 @@ def get_view_names(
         )
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            return cursor.info_uri

Review Comment:
   `info_uri` is only available in the latest version of the Trino client, which we haven't upgrade to yet.



-- 
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 #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20799:
URL: https://github.com/apache/superset/pull/20799#issuecomment-1190912972

   # [Codecov](https://codecov.io/gh/apache/superset/pull/20799?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 [#20799](https://codecov.io/gh/apache/superset/pull/20799?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0575406) into [master](https://codecov.io/gh/apache/superset/commit/3311128c5e6c5de2ea5d6a2dfeb01ea3179e9af8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3311128) will **decrease** coverage by `11.66%`.
   > The diff coverage is `63.49%`.
   
   > :exclamation: Current head 0575406 differs from pull request most recent head fe967b8. Consider uploading reports for the commit fe967b8 to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #20799       +/-   ##
   ===========================================
   - Coverage   66.29%   54.62%   -11.67%     
   ===========================================
     Files        1758     1758               
     Lines       66799    66847       +48     
     Branches     7055     7055               
   ===========================================
   - Hits        44286    36517     -7769     
   - Misses      20713    28530     +7817     
     Partials     1800     1800               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.12% <60.34%> (+0.01%)` | :arrow_up: |
   | python | `57.44% <60.34%> (-24.06%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `50.22% <36.20%> (-0.03%)` | :arrow_down: |
   
   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/20799?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/components/Button/index.tsx](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQnV0dG9uL2luZGV4LnRzeA==) | `100.00% <ø> (ø)` | |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `35.13% <ø> (-50.75%)` | :arrow_down: |
   | [superset/utils/dates.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZXMucHk=) | `72.72% <ø> (-27.28%)` | :arrow_down: |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/20799/diff?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==) | `34.44% <0.00%> (-41.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/trino.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3RyaW5vLnB5) | `38.70% <15.78%> (-50.63%)` | :arrow_down: |
   | [superset/sqllab/exceptions.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsbGFiL2V4Y2VwdGlvbnMucHk=) | `68.08% <50.00%> (+0.69%)` | :arrow_up: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `40.00% <61.53%> (-49.03%)` | :arrow_down: |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `54.07% <100.00%> (ø)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.14% <100.00%> (-0.33%)` | :arrow_down: |
   | [superset/models/sql\_lab.py](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `74.26% <100.00%> (-3.12%)` | :arrow_down: |
   | ... and [291 more](https://codecov.io/gh/apache/superset/pull/20799/diff?src=pr&el=tree-more&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] john-bodley commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930458250


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,18 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))
-        params = [from_time, to_time]
-        resp = self.get_resp("/superset/search_queries?" + "&".join(params))
-        data = json.loads(resp)
-        self.assertEqual(2, len(data))
+        # pylint: disable=line-too-long

Review Comment:
   I would add this at the end of line #420, otherwise I think it disables the error for the entire block.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. `[QUERY_1, QUERY_2, QUERY_3]` are executed consequently, with supposedly increasing `start_time`. 
   
   Since the original code `int(...)` basically rounded down the float epoch, the actual query results returned `[QUERY_1, QUERY_2]`---even though we filtered `second_query_time` by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time, so if the start time for the queries are `1.1, 1.5, 2`---we'd filtered to between `[1, 1]` and return only `[QUERY_1]`.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. `[QUERY_1, QUERY_2, QUERY_3]` are executed consequently, with supposedly increasing `start_time`. 
   
   Since the original code `int(...)` basically rounded down the float epoch, the actual query results returned `[QUERY_1, QUERY_2]`---even though we filtered `second_query_time` by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time, so if the start time for the queries are `[1.1, 1.5, 2]`---we'd filtered to between `[1, 1]` and return only `[QUERY_1]`.



-- 
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] john-bodley commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r926172621


##########
superset/db_engine_specs/presto.py:
##########
@@ -64,6 +64,13 @@
     # prevent circular imports
     from superset.models.core import Database
 
+    # need try/catch because pyhive may not be installed
+    try:
+        # pylint: disable=unused-import
+        from pyhive.presto import Cursor

Review Comment:
   ```suggestion
           from pyhive.presto import Cursor  # pylint: disable=unused-import
   ```



##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +964,22 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        if cursor.last_query_id:
+            # pylint: disable=protected-access
+            return (
+                f"{cursor._protocol}://{cursor._host}:{cursor._port}"

Review Comment:
   Why not just one line?



##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +964,22 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        if cursor.last_query_id:
+            # pylint: disable=protected-access
+            return (
+                f"{cursor._protocol}://{cursor._host}:{cursor._port}"
+                + f"/ui/query.html?{cursor.last_query_id}"
+            )
+        return None
+
+    @classmethod
+    def handle_cursor(cls, cursor: "Cursor", query: Query, session: Session) -> None:
         """Updates progress information"""
+        tracking_url = cls.get_tracking_url(cursor)
+        if tracking_url:
+            query.tracking_url = tracking_url

Review Comment:
   We may need a commit here.



##########
superset/models/sql_lab.py:
##########
@@ -279,6 +284,27 @@ def default_endpoint(self) -> str:
     def get_extra_cache_keys(query_obj: Dict[str, Any]) -> List[str]:
         return []
 
+    @property
+    def tracking_url(self) -> Optional[str]:

Review Comment:
   Could this just be a method called `get_tracking_url` and then you wouldn't need to rename the column virtually or have a getter/setter.



##########
superset/db_engine_specs/presto.py:
##########
@@ -64,6 +64,13 @@
     # prevent circular imports
     from superset.models.core import Database
 
+    # need try/catch because pyhive may not be installed
+    try:
+        # pylint: disable=unused-import

Review Comment:
   ```suggestion
   ```



##########
superset/db_engine_specs/trino.py:
##########
@@ -109,8 +115,27 @@ def get_view_names(
         )
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            return cursor.info_uri
+        except AttributeError:
+            try:
+                conn = cursor.connection
+                # pylint: disable=protected-access
+                return (
+                    f"{conn.http_scheme}://{conn.host}:{conn.port}/"

Review Comment:
   See previous comment.



##########
superset/config.py:
##########
@@ -993,7 +993,7 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC(  # pylint: disable=invalid-name
 # into a proxied one
 
 
-TRACKING_URL_TRANSFORMER = lambda x: x
+TRACKING_URL_TRANSFORMER = lambda url: url

Review Comment:
   Nice!



##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +964,22 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        if cursor.last_query_id:
+            # pylint: disable=protected-access
+            return (
+                f"{cursor._protocol}://{cursor._host}:{cursor._port}"
+                + f"/ui/query.html?{cursor.last_query_id}"
+            )
+        return None
+
+    @classmethod
+    def handle_cursor(cls, cursor: "Cursor", query: Query, session: Session) -> None:
         """Updates progress information"""
+        tracking_url = cls.get_tracking_url(cursor)

Review Comment:
   I'm itching for us to drop Python 3.8 so we can use the walrus operator.



##########
superset/db_engine_specs/trino.py:
##########
@@ -32,6 +32,12 @@
 if TYPE_CHECKING:
     from superset.models.core import Database
 
+    try:
+        # pylint: disable=unused-import
+        from trino.dbapi import Cursor

Review Comment:
   See previous comment.



##########
UPDATING.md:
##########
@@ -25,6 +25,7 @@ assists people when migrating to a new version.
 ## Next
 
 - [20606](https://github.com/apache/superset/pull/20606): When user clicks on chart title or "Edit chart" button in Dashboard page, Explore opens in the same tab. Clicking while holding cmd/ctrl opens Explore in a new tab. To bring back the old behaviour (always opening Explore in a new tab), flip feature flag `DASHBOARD_EDIT_CHART_IN_NEW_TAB` to `True`.
+- [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 an empty string or `None`.

Review Comment:
   ```suggestion
   - [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`.
   ```
   
   From both a code and communication perspective `None` seems clearer/cleaner.



##########
superset/sql_lab.py:
##########
@@ -96,8 +96,13 @@ def handle_query_error(
     msg = f"{prefix_message} {str(ex)}".strip()
     troubleshooting_link = config["TROUBLESHOOTING_LINK"]
     query.error_message = msg
-    query.status = QueryStatus.FAILED
     query.tmp_table_name = None
+    query.status = QueryStatus.FAILED
+    # TODO: re-enable this after updating the frontend to properly display timeout status
+    # if query.status != QueryStatus.TIMED_OUT:
+    #   query.status = QueryStatus.FAILED

Review Comment:
   I'm a tad scared of this change. Why is the query no longer being marked as `FAILED`?



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. Since the original code `int(...)` basically rounded down the float epoch, we actually queried for `QUERY_1` + `QUERY_2`---even though `second_query_time` is filtered by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r929093918


##########
superset/db_engine_specs/hive.py:
##########
@@ -366,21 +366,14 @@ def handle_cursor(  # pylint: disable=too-many-locals
                     query.progress = progress
                     needs_commit = True
                 if not tracking_url:
-                    tracking_url = cls.get_tracking_url(log_lines)
+                    tracking_url = cls.get_tracking_url_from_logs(log_lines)
                     if tracking_url:
                         job_id = tracking_url.split("/")[-2]
                         logger.info(
                             "Query %s: Found the tracking url: %s",
                             str(query_id),
                             tracking_url,
                         )
-                        transformer = current_app.config["TRACKING_URL_TRANSFORMER"]

Review Comment:
   Yes, there is a test case for it: https://github.com/apache/superset/pull/20799/files#diff-bbedc6245e80199586380e684695b5a8e275fd95c46f6770b91eb0bb918e2bf5R52-R53



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. `[QUERY_1, QUERY_2, QUERY_3]` are executed consequently, with supposedly increasing `start_time`. 
   
   Since the original code `int(...)` basically rounded down the float epoch, the actual query results returned `[QUERY_1, QUERY_2]`---even though we filtered `second_query_time` by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time, so if QUERY_1 and QUERY_2 are executed within the same second, we wouldn't have QUERY_2 in the filtered results. E.g. when the start time for the queries are `1.1, 1.5, 2`---we'd filtered to between `[1, 1]`.



-- 
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] john-bodley commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r929088420


##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +963,23 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            if cursor.last_query_id:
+                # pylint: disable=protected-access, line-too-long
+                return f"{cursor._protocol}://{cursor._host}:{cursor._port}/ui/query.html?{cursor.last_query_id}"
+        except AttributeError:
+            pass
+        return None

Review Comment:
   ```suggestion
   ```



##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +963,23 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            if cursor.last_query_id:
+                # pylint: disable=protected-access, line-too-long
+                return f"{cursor._protocol}://{cursor._host}:{cursor._port}/ui/query.html?{cursor.last_query_id}"
+        except AttributeError:
+            pass

Review Comment:
   ```suggestion
               return None
   ```



##########
superset/db_engine_specs/hive.py:
##########
@@ -366,21 +366,14 @@ def handle_cursor(  # pylint: disable=too-many-locals
                     query.progress = progress
                     needs_commit = True
                 if not tracking_url:
-                    tracking_url = cls.get_tracking_url(log_lines)
+                    tracking_url = cls.get_tracking_url_from_logs(log_lines)
                     if tracking_url:
                         job_id = tracking_url.split("/")[-2]
                         logger.info(
                             "Query %s: Found the tracking url: %s",
                             str(query_id),
                             tracking_url,
                         )
-                        transformer = current_app.config["TRACKING_URL_TRANSFORMER"]

Review Comment:
   @ktmud per you PR description this logic has changed. Can we confirmed that the previous behavior is preserved? I just want to confirm this isn't a breaking change.



##########
superset/db_engine_specs/trino.py:
##########
@@ -109,8 +114,25 @@ def get_view_names(
         )
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            return cursor.info_uri
+        except AttributeError:
+            try:
+                conn = cursor.connection
+                # pylint: disable=protected-access, line-too-long
+                return f"{conn.http_scheme}://{conn.host}:{conn.port}/ui/query.html?{cursor._query.query_id}"
+            except AttributeError:
+                pass

Review Comment:
   See previous comment.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r929094751


##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +963,23 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            if cursor.last_query_id:
+                # pylint: disable=protected-access, line-too-long
+                return f"{cursor._protocol}://{cursor._host}:{cursor._port}/ui/query.html?{cursor.last_query_id}"
+        except AttributeError:
+            pass

Review Comment:
   Return is expected to be explicit if you have a return statement earlier.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r927974409


##########
superset/sqllab/exceptions.py:
##########
@@ -25,7 +25,7 @@
 MSG_FORMAT = "Failed to execute {}"
 
 if TYPE_CHECKING:
-    from superset.utils.sqllab_execution_context import SqlJsonExecutionContext
+    from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext

Review Comment:
   Bycatch: wrong import



-- 
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] ktmud commented on pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #20799:
URL: https://github.com/apache/superset/pull/20799#issuecomment-1192925809

   @john-bodley I addressed your comments and added some integration tests. Can you take another look?


-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r929094751


##########
superset/db_engine_specs/presto.py:
##########
@@ -957,8 +963,23 @@ def get_create_view(
         return rows[0][0]
 
     @classmethod
-    def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
+    def get_tracking_url(cls, cursor: "Cursor") -> Optional[str]:
+        try:
+            if cursor.last_query_id:
+                # pylint: disable=protected-access, line-too-long
+                return f"{cursor._protocol}://{cursor._host}:{cursor._port}/ui/query.html?{cursor.last_query_id}"
+        except AttributeError:
+            pass

Review Comment:
   Final return is expected to be explicit if you have a return statement earlier. I think there is a pylint or mypy warning for this.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930445557


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))

Review Comment:
   Bycatch: this test is flaky. Since the original code `int(...)` basically rounded down the float epoch, we actually queried for `QUERY_1` + `QUERY_2`---even though `second_query_time` filtered by `QUERY_3`. But this still sometimes fail because the rounded down time can be anywhere between QUERY_1 start time and QUERY_3 start time.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930443744


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,19 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))
-        params = [from_time, to_time]
-        resp = self.get_resp("/superset/search_queries?" + "&".join(params))
-        data = json.loads(resp)
-        self.assertEqual(2, len(data))
+        print(first_query_time, second_query_time)
+        # pylint: disable=line-too-long
+        url = f"/superset/search_queries?from={floor(first_query_time)}&to={ceil(second_query_time)}"

Review Comment:
   Bycatch: test may fail if `to` time is smaller than second query start time.



-- 
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] ktmud commented on a diff in pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #20799:
URL: https://github.com/apache/superset/pull/20799#discussion_r930480682


##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -406,22 +407,18 @@ def test_search_query_on_text(self):
         self.assertEqual(2, len(data))
         self.assertIn("birth", data[0]["sql"])
 
-    def test_search_query_on_time(self):
+    def test_search_query_filter_by_time(self):
         self.run_some_queries()
         self.login("admin")
         first_query_time = (
             db.session.query(Query).filter_by(sql=QUERY_1).one()
         ).start_time
         second_query_time = (
-            db.session.query(Query).filter_by(sql=QUERY_3).one()
+            db.session.query(Query).filter_by(sql=QUERY_2).one()
         ).start_time
-        # Test search queries on time filter
-        from_time = "from={}".format(int(first_query_time))
-        to_time = "to={}".format(int(second_query_time))
-        params = [from_time, to_time]
-        resp = self.get_resp("/superset/search_queries?" + "&".join(params))
-        data = json.loads(resp)
-        self.assertEqual(2, len(data))
+        # pylint: disable=line-too-long

Review Comment:
   I reformatted it so pylint disable isn't necessary.



-- 
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] ktmud merged pull request #20799: feat(sql lab): display presto and trino tracking url

Posted by GitBox <gi...@apache.org>.
ktmud merged PR #20799:
URL: https://github.com/apache/superset/pull/20799


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