potiuk commented on code in PR #31277:
URL: https://github.com/apache/airflow/pull/31277#discussion_r1193133427
##########
airflow/jobs/job.py:
##########
@@ -130,10 +130,13 @@ def is_alive(self, grace_multiplier=2.1):
:param grace_multiplier: multiplier of heartrate to require heart beat
within
"""
+ if self.job_type == "SchedulerJob":
+ health_check_threshold: int = conf.getint("scheduler",
"scheduler_health_check_threshold")
Review Comment:
Yeah, I saw it - but this is actually not used in our code in any place and
we have no `SchedulerJob` any more, so if someone would - actually use it, it
would be broken (and in the Public interface description we have
https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html
- we even now explicitly state taht Job (belonging to DB structure) is not a
public interface, so we can change it's behaviour any time.
I even considered to remove the muitiplier altogether and made it a constant
(Which we might actually do as a follow-up).
I can add such newsfragment, but it woudl be misleading as it woudl refer to
change in something that does not exist any more :)
##########
airflow/jobs/job.py:
##########
@@ -130,10 +130,13 @@ def is_alive(self, grace_multiplier=2.1):
:param grace_multiplier: multiplier of heartrate to require heart beat
within
"""
+ if self.job_type == "SchedulerJob":
+ health_check_threshold: int = conf.getint("scheduler",
"scheduler_health_check_threshold")
Review Comment:
Yeah, I saw it - but this is actually not used in our code in any place and
we have no `SchedulerJob` any more, so if someone would - actually use it, it
would be broken (and in the Public interface description we have
https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html
- we even now explicitly state that Job (belonging to DB structure) is not a
public interface, so we can change it's behaviour any time.
I even considered to remove the muitiplier altogether and made it a constant
(Which we might actually do as a follow-up).
I can add such newsfragment, but it woudl be misleading as it woudl refer to
change in something that does not exist any more :)
--
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]
