john-bodley commented on code in PR #26186:
URL: https://github.com/apache/superset/pull/26186#discussion_r1417841057
##########
superset/tasks/cache.py:
##########
@@ -130,28 +123,24 @@ def __init__(self, top_n: int = 5, since: str = "7 days
ago") -> None:
self.since = parse_human_datetime(since) if since else None
def get_payloads(self) -> list[dict[str, int]]:
- payloads = []
- session = db.create_scoped_session()
+ records = (
Review Comment:
Pretty much the same code as previously without the `try` block.
##########
superset/sql_lab.py:
##########
@@ -176,30 +170,27 @@ def get_sql_results( # pylint: disable=too-many-arguments
log_params: Optional[dict[str, Any]] = None,
) -> Optional[dict[str, Any]]:
"""Executes the sql query returns the results."""
- with session_scope(not ctask.request.called_directly) as session:
- with override_user(security_manager.find_user(username)):
- try:
- return execute_sql_statements(
- query_id,
- rendered_query,
- return_results,
- store_results,
- session=session,
- start_time=start_time,
- expand_data=expand_data,
- log_params=log_params,
- )
- except Exception as ex: # pylint: disable=broad-except
- logger.debug("Query %d: %s", query_id, ex)
- stats_logger.incr("error_sqllab_unhandled")
- query = get_query(query_id, session)
- return handle_query_error(ex, query, session)
+ with override_user(security_manager.find_user(username)):
Review Comment:
Pretty much the same code as previously without the outer `with` block.
##########
superset/tasks/cache.py:
##########
@@ -178,48 +167,44 @@ def __init__(self, tags: Optional[list[str]] = None) ->
None:
def get_payloads(self) -> list[dict[str, int]]:
payloads = []
- session = db.create_scoped_session()
-
- try:
- tags = session.query(Tag).filter(Tag.name.in_(self.tags)).all()
- tag_ids = [tag.id for tag in tags]
-
- # add dashboards that are tagged
- tagged_objects = (
- session.query(TaggedObject)
- .filter(
- and_(
- TaggedObject.object_type == "dashboard",
- TaggedObject.tag_id.in_(tag_ids),
- )
+ tags = db.session.query(Tag).filter(Tag.name.in_(self.tags)).all()
Review Comment:
Pretty much the same code as previously without the `try` block.
##########
superset/tasks/scheduler.py:
##########
@@ -46,35 +45,32 @@ def scheduler() -> None:
if not is_feature_enabled("ALERT_REPORTS"):
return
- with session_scope(nullpool=True) as session:
- active_schedules = ReportScheduleDAO.find_active(session)
- triggered_at = (
- datetime.fromisoformat(scheduler.request.expires)
- - app.config["CELERY_BEAT_SCHEDULER_EXPIRES"]
- if scheduler.request.expires
- else datetime.utcnow()
- )
- for active_schedule in active_schedules:
- for schedule in cron_schedule_window(
- triggered_at, active_schedule.crontab, active_schedule.timezone
+ active_schedules = ReportScheduleDAO.find_active()
Review Comment:
Pretty much the same code as previously without the outer `with` 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: [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]
