You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/08/31 16:39:25 UTC
[GitHub] [airflow] jedcunningham opened a new pull request, #26089: Only send an SlaCallbackRequest if the DAG is scheduled
jedcunningham opened a new pull request, #26089:
URL: https://github.com/apache/airflow/pull/26089
We don't need to send an SlaCallbackRequest to then have the processor skip it, SchedulerJob can simply skip it on it's own.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #26089: Only send an SlaCallbackRequest if the DAG is scheduled
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #26089:
URL: https://github.com/apache/airflow/pull/26089#discussion_r959846034
##########
tests/jobs/test_scheduler_job.py:
##########
@@ -3000,6 +3000,26 @@ def test_send_sla_callbacks_to_processor_sla_with_task_slas(self, dag_maker):
expected_callback = SlaCallbackRequest(full_filepath=dag.fileloc, dag_id=dag.dag_id)
self.scheduler_job.executor.callback_sink.send.assert_called_once_with(expected_callback)
+ @pytest.mark.parametrize(
+ "schedule",
+ [
+ None,
+ [Dataset("foo")],
Review Comment:
It does exist [here](https://github.com/apache/airflow/blob/3a7e89dbddf5fc60dbcb9582acf5e3a79d702c7c/tests/jobs/test_scheduler_job.py#L2988), but I should probably expand it to cover more cases. I'll do that in a bit here.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a diff in pull request #26089: Only send an SlaCallbackRequest if the DAG is scheduled
Posted by GitBox <gi...@apache.org>.
kaxil commented on code in PR #26089:
URL: https://github.com/apache/airflow/pull/26089#discussion_r959843386
##########
tests/jobs/test_scheduler_job.py:
##########
@@ -3000,6 +3000,26 @@ def test_send_sla_callbacks_to_processor_sla_with_task_slas(self, dag_maker):
expected_callback = SlaCallbackRequest(full_filepath=dag.fileloc, dag_id=dag.dag_id)
self.scheduler_job.executor.callback_sink.send.assert_called_once_with(expected_callback)
+ @pytest.mark.parametrize(
+ "schedule",
+ [
+ None,
+ [Dataset("foo")],
Review Comment:
Worth adding an inverse of this? pass in timedelta/cron and see if it `self.scheduler_job.executor.callback_sink.send.assert_called_once_with(` --- maybe the test for that might already exist -- in which case I am ok 👍
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #26089: Only send an SlaCallbackRequest if the DAG is scheduled
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #26089:
URL: https://github.com/apache/airflow/pull/26089#discussion_r959833069
##########
airflow/jobs/scheduler_job.py:
##########
@@ -1318,6 +1318,10 @@ def _send_sla_callbacks_to_processor(self, dag: DAG) -> None:
self.log.debug("Skipping SLA check for %s because no tasks in DAG have SLAs", dag)
return
+ if not dag.timetable.periodic:
+ self.log.debug("Skipping SLA check for %s because DAG is not scheduled", dag)
Review Comment:
I've bumped this down to debug, otherwise it happened way too frequently.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #26089: Only send an SlaCallbackRequest if the DAG is scheduled
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26089:
URL: https://github.com/apache/airflow/pull/26089
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org