vincbeck commented on code in PR #71214:
URL: https://github.com/apache/airflow/pull/71214#discussion_r4324794803
##########
airflow-core/src/airflow/jobs/scheduler_job_runner.py:
##########
@@ -1366,25 +1367,19 @@ def _enqueue_executor_callbacks(self, session: Session)
-> None:
# Can't happen since we queried ExecutorCallback, but
satisfies mypy.
continue
- # TODO: Add dagrun_id as a proper ORM foreign key on the
callback table instead of storing in data dict.
- # This would eliminate this reconstruction step. For
now, all ExecutorCallbacks
- # are expected to have dag_run_id set in their data dict
(e.g., by Deadline.handle_miss).
- if not isinstance(callback.data, dict) or "dag_run_id" not in
callback.data:
- self.log.error(
- "ExecutorCallback %s is missing required 'dag_run_id'
in data dict. "
- "This indicates a bug in callback creation. Skipping
callback.",
- callback.id,
+ if callback.dagrun_id is None:
+ self.log.warning(
Review Comment:
Maybe not related to this PR (cause I think it already exists today) but I
am wondering in which case we end up in this situation.
1. Is that an expected scenario? If no, why a warning?
2. If yes, how come we ended up there?
I am asking because it feels weird to me to log a warning (that nobody will
read) for something that feels wrong?
--
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]
