Skip to content
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

fix(20428): Address-Presto/Trino-Poll-Issue-Refactor #20434

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions superset/db_engine_specs/presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,11 +949,7 @@ def get_create_view(
sql = f"SHOW CREATE VIEW {schema}.{table}"
try:
cls.execute(cursor, sql)
polled = cursor.poll()
Copy link
Member

@john-bodley john-bodley Jun 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Thelin90 let me test this using Presto to ensure this works. I'll report back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed, via the /extra_table_metadata/<int:database_id>/<table_name>/<schema>/ route—which is invokes said method for views, that the method works as expected without the polling.


while polled:
time.sleep(0.2)
polled = cursor.poll()
except DatabaseError: # not a VIEW
return None
rows = cls.fetch_data(cursor, 1)
Expand Down
38 changes: 37 additions & 1 deletion superset/db_engine_specs/trino.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@
# specific language governing permissions and limitations
# under the License.
import logging
from typing import Any, Dict, Optional, TYPE_CHECKING
import time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should revert any of the now unnecessary imports.

from typing import Any, Dict, List, Optional, TYPE_CHECKING

import simplejson as json
from flask import current_app
from sqlalchemy.engine.reflection import Inspector
from sqlalchemy.engine.url import URL
from sqlalchemy.orm import Session

from superset.common.db_query_status import QueryStatus
from superset.databases.utils import make_url_safe
from superset.db_engine_specs.base import BaseEngineSpec
from superset.db_engine_specs.presto import PrestoEngineSpec
from superset.models.sql_lab import Query
from superset.utils import core as utils

if TYPE_CHECKING:
Expand Down Expand Up @@ -77,6 +82,37 @@ def modify_url_for_impersonation(
def get_allow_cost_estimate(cls, extra: Dict[str, Any]) -> bool:
return True

@classmethod
def get_table_names(
cls,
database: "Database",
inspector: Inspector,
schema: Optional[str],
) -> List[str]:
return BaseEngineSpec.get_table_names(
database=database,
inspector=inspector,
schema=schema,
)

@classmethod
def get_view_names(
cls,
database: "Database",
inspector: Inspector,
schema: Optional[str],
) -> List[str]:
return BaseEngineSpec.get_view_names(
database=database,
inspector=inspector,
schema=schema,
)

@classmethod
def handle_cursor(cls, cursor: Any, query: Query, session: Session) -> None:
"""Updates progress information"""
BaseEngineSpec.handle_cursor(cursor=cursor, query=query, session=session)

@staticmethod
def get_extra_params(database: "Database") -> Dict[str, Any]:
"""
Expand Down