o-nikolas commented on code in PR #33084:
URL: https://github.com/apache/airflow/pull/33084#discussion_r1283778503
##########
airflow/jobs/job.py:
##########
@@ -117,6 +115,15 @@ def __init__(self, executor=None, heartrate=None,
**kwargs):
def executor(self):
return ExecutorLoader.get_default_executor()
+ @cached_property
+ def heartrate(self):
+ if self.job_type == "TriggererJob":
+ return conf.getfloat("triggerer", "JOB_HEARTBEAT_SEC")
+ else:
+ # Heartrate used to be hardcoded to scheduler, so in all other
+ # cases continue to use that value for back compat
+ return conf.getfloat("scheduler", "JOB_HEARTBEAT_SEC")
+
Review Comment:
> I wonder if we can avoid implementing this logic in the parent class (job
class), where doing so, we will have to add a case for each future job runner.
Yeah and the interaction between jobs and their runners are very blurry (a
job doesn't set it's own type even, it's the runner that does it!).
My aim with this PR was just to keep it scoped to the bug fix and not
rearchitect too much.
> Why do we need to override the default value too?
I'm not sure precisely which default value you're talking about. But the
heartrate needs to be set in the Job class because the heartrate is not stored
in the DB for Jobs, so it needs to be fetched from cconfig when sqlalchemy
builds the Python object from a query result.
--
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]
