o-nikolas commented on code in PR #37264:
URL: https://github.com/apache/airflow/pull/37264#discussion_r1484626120
##########
airflow/executors/base_executor.py:
##########
@@ -211,6 +215,14 @@ def heartbeat(self) -> None:
else:
open_slots = self.parallelism - len(self.running)
+ # Call child class health_check method.
+ try:
+ # Update the executor's health status.
+ self.check_health(verbose=True)
+ except AirflowException:
+ # Do not stop execution for an unhealthy executor as it can
recover.
+ pass
Review Comment:
I wonder if it should be up to the concrete classes not to throw in that
case?
##########
airflow/executors/base_executor.py:
##########
@@ -134,6 +135,9 @@ def __repr__(self):
def start(self): # pragma: no cover
"""Executors may need to get things started."""
+ def check_health(self, verbose: bool = True):
Review Comment:
Do we need the "public" field and the method?
We could either have just the field and it's up to the concrete executors to
keep it updated how they see fit. Or we can keep this method as well and then
the concrete classes just implement it and we set the (newly private) field to
result of this method call in the heartbeat below.
I just don't want these two to get out of sync.
--
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]
