[GitHub] [incubator-superset] villebro commented on a change in pull request #10819: fix: use nullpool in the celery workers

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #10819:
URL: https://github.com/apache/incubator-superset/pull/10819#discussion_r486530908



##########
File path: superset/tasks/alerts/observer.py
##########
@@ -20,22 +20,22 @@
 from typing import Optional
 
 import pandas as pd
+from sqlalchemy.orm import Session
 
-from superset import db
 from superset.models.alerts import Alert, SQLObservation
 from superset.sql_parse import ParsedQuery
 
 logger = logging.getLogger("tasks.email_reports")
 
 
-def observe(alert_id: int) -> Optional[str]:
+def observe(alert_id: int, session: Session) -> Optional[str]:

Review comment:
       I think the idea was to mostly remove references to sessions in sigs (there's lots of them scattered around), as they can usually just be picked up from `db`, so it might be a good idea to add a comment here that this really needs to be passed along here.

##########
File path: superset/cli.py
##########
@@ -619,6 +620,11 @@ def alert() -> None:
     from superset.tasks.schedules import schedule_window
 
     click.secho("Processing one alert loop", fg="green")
-    schedule_window(
-        ScheduleType.alert, datetime.now() - timedelta(1000), datetime.now(), 6000
-    )
+    with session_scope(nullpool=True) as session:
+        schedule_window(
+            ScheduleType.alert,
+            datetime.now() - timedelta(1000),
+            datetime.now(),
+            6000,
+            session,
+        )

Review comment:
       Not yours, but I usually like using named arguments in cases like this to make stuff more readable. 6000 what?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org