Vitor-Avila commented on code in PR #32231: URL: https://github.com/apache/superset/pull/32231#discussion_r1958646996
########## superset/commands/database/utils.py: ########## @@ -17,15 +17,33 @@ from __future__ import annotations import logging +import sqlite3 +from contextlib import closing + +from flask import current_app as app +from sqlalchemy.engine import Engine from superset import security_manager from superset.databases.ssh_tunnel.models import SSHTunnel from superset.db_engine_specs.base import GenericDBException from superset.models.core import Database +from superset.utils.core import timeout logger = logging.getLogger(__name__) +def ping(engine: Engine) -> bool: + try: + time_delta = app.config["TEST_DATABASE_CONNECTION_TIMEOUT"] + with timeout(int(time_delta.total_seconds())): + with closing(engine.raw_connection()) as conn: + return engine.dialect.do_ping(conn) + except (sqlite3.ProgrammingError, RuntimeError): + # SQLite can't run on a separate thread, so ``utils.timeout`` fails + # RuntimeError catches the equivalent error from duckdb. + return engine.dialect.do_ping(engine) Review Comment: @korbit-ai This haven't changed as part of this PR. This method was actually defined like this, but this PR moves it to `utils.py` as it also uses it. From the comment block there, it seems these two exceptions are expected in specific scenarios, in which case we're defaulting to `engine.dialect.do_ping(engine)` as a fallback. Since this seems to be expected for these cases, I wouldn't add logging there as these would be expected exceptions. Any other exception would bubble up accordingly. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
