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: [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]
