Skip to content

Adapt Notifier for sla_miss_callback#31887

Merged
ephraimbuddy merged 18 commits into
apache:mainfrom
utkarsharma2:FixNotifierSLA
Jun 23, 2023
Merged

Adapt Notifier for sla_miss_callback#31887
ephraimbuddy merged 18 commits into
apache:mainfrom
utkarsharma2:FixNotifierSLA

Conversation

@utkarsharma2

Copy link
Copy Markdown
Contributor

Currently, the notifier doesn't work with sla_miss_callback base notifier since the __call__ only expects a single parameter

def __call__(self, context: Context) -> None:

But when the sla_miss_callback are called we pass multiple params

callback(dag, task_list, blocking_task_list, slas, blocking_tis)

Because of this, they are failing silently.

@boring-cyborg boring-cyborg Bot added the area:Scheduler including HA (high availability) scheduler label Jun 14, 2023
@utkarsharma2 utkarsharma2 changed the title Fix Notifier issue with sla_miss_callback Adapt Notifier for sla_miss_callback Jun 14, 2023
@utkarsharma2

utkarsharma2 commented Jun 14, 2023

Copy link
Copy Markdown
Contributor Author

@ephraimbuddy Please review :)

Also, I have tested this with SMTPNotifier with sla_miss_callback.

@utkarsharma2
utkarsharma2 marked this pull request as draft June 14, 2023 05:56
@utkarsharma2
utkarsharma2 marked this pull request as ready for review June 14, 2023 06:33
Comment thread airflow/dag_processing/processor.py Outdated
@utkarsharma2
utkarsharma2 requested a review from eladkal as a code owner June 14, 2023 13:12
@utkarsharma2 utkarsharma2 modified the milestone: Airlfow 2.6.3 Jun 15, 2023
Comment thread airflow/notifications/basenotifier.py Outdated
Comment thread airflow/utils/context.py Outdated
Comment thread tests/dag_processing/test_processor.py Outdated
@ephraimbuddy ephraimbuddy added this to the Airlfow 2.6.3 milestone Jun 23, 2023
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Jun 23, 2023
@ephraimbuddy
ephraimbuddy merged commit e4ca688 into apache:main Jun 23, 2023
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jun 27, 2023
* Fix Notifier issue with sla_miss_callback

* Add testcase to ensure the old pattern is working

* Update document for existing notifier

* Made notifier backward compatible with sla_miss_callback

* Remove unwanted imports

* Fix testcases

* Fix static check

* Fix PR comments

* Removed unwanted code

* Remove uneanted change

* Updated var name

* Remove kwargs

* Fix documentation

* Fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Scheduler including HA (high availability) scheduler type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants