dstandish commented on a change in pull request #14085:
URL: https://github.com/apache/airflow/pull/14085#discussion_r574227085
##########
File path: airflow/jobs/scheduler_job.py
##########
@@ -1292,13 +1290,12 @@ def _execute(self) -> None:
)
models.DAG.deactivate_stale_dags(execute_start_time)
- self.executor.end()
-
settings.Session.remove() # type: ignore
except Exception: # pylint: disable=broad-except
self.log.exception("Exception when executing
SchedulerJob._run_scheduler_loop")
finally:
self.processor_agent.end()
Review comment:
what if `self.processor_agent.end()` fails? will `self.executor.end()`
be reached? maybe you need something like an exit stack
##########
File path: airflow/executors/local_executor.py
##########
@@ -383,6 +383,10 @@ def end(self) -> None:
raise AirflowException(NOT_STARTED_MESSAGE)
if not self.manager:
raise AirflowException(NOT_STARTED_MESSAGE)
+ self.log.info(
+ "Shutting down LocalExecutor"
+ " - this will wait for running tasks to finish, or signal again if
you don't want to wait."
+ )
Review comment:
```suggestion
self.log.info(
"Attempting to shut down LocalExecutor"
"; waiting for running tasks to finish. Signal again if you
don't want to wait."
)
```
main point here is to clarify grammatically that it's not `_this_ will wait
or signal` but `_this_ will wait and _you_ must signal`
----------------------------------------------------------------
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:
[email protected]
