dpgaspar commented on code in PR #31315:
URL: https://github.com/apache/superset/pull/31315#discussion_r1873107598
##########
superset/models/core.py:
##########
@@ -691,27 +731,30 @@ def _log_query(sql: str) -> None:
with self.get_raw_connection(catalog=catalog, schema=schema) as conn:
Review Comment:
WDYT about introducing the `temporarily_disconnect_db` logic inside
`get_raw_connection`, if possible, could introduce several benefits, for
example would make sure that all analytics db queries would obey the
disconnect, easier to refactor in the future
##########
superset/models/core.py:
##########
@@ -691,27 +731,30 @@ def _log_query(sql: str) -> None:
with self.get_raw_connection(catalog=catalog, schema=schema) as conn:
cursor = conn.cursor()
df = None
- for i, sql_ in enumerate(sqls):
- sql_ = self.mutate_sql_based_on_config(sql_, is_split=True)
- _log_query(sql_)
- with event_logger.log_context(
- action="execute_sql",
- database=self,
- object_ref=__name__,
- ):
- self.db_engine_spec.execute(cursor, sql_, self)
- if i < len(sqls) - 1:
- # If it's not the last, we don't keep the results
- cursor.fetchall()
- else:
- # Last query, fetch and process the results
- data = self.db_engine_spec.fetch_data(cursor)
- result_set = SupersetResultSet(
- data, cursor.description, self.db_engine_spec
- )
- df = result_set.to_pandas_df()
- if mutator:
- df = mutator(df)
+ with temporarily_disconnect_db():
+ if is_feature_enabled("SIMULATE_SLOW_ANALYTICS_DATABASE"):
+ time.sleep(30)
Review Comment:
nit: would be nice to have a configuration key for this value also, or just
remove this functionality and rely on other types of tests, for example using
charts that query pg_sleep on a PG analytics db
##########
superset/config.py:
##########
@@ -563,6 +567,11 @@ class D3TimeFormat(TypedDict, total=False):
# If on, you'll want to add "https://avatars.slack-edge.com"; to the list
of allowed
# domains in your TALISMAN_CONFIG
"SLACK_ENABLE_AVATARS": False,
+ # This feature flag works only in combination with NullPool, and
disconnects the metadata db
+ # connection temporarily during the execution of analytics queries,
avoiding bottlenecks
+ "DISABLE_METADATA_DB_DURING_ANALYTICS": False,
+ # if true, we add a one minute delay to the database queries for both
Explore and SQL Lab
+ "SIMULATE_SLOW_ANALYTICS_DATABASE": True,
Review Comment:
switch to False?
--
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]
