ephraimbuddy commented on code in PR #31887:
URL: https://github.com/apache/airflow/pull/31887#discussion_r1234083682
##########
airflow/notifications/basenotifier.py:
##########
@@ -79,15 +79,30 @@ def notify(self, context: Context) -> None:
"""
...
- def __call__(self, context: Context) -> None:
+ def __call__(self, *args) -> None:
"""
Send a notification.
:param context: The airflow context
"""
- context = self._update_context(context)
- self.render_template_fields(context)
+ # Currently, there are two ways a callback is invoked
+ # 1. callback(context) - for on_*_callbacks
+ # 2. callback(dag, task_list, blocking_task_list, slas, blocking_tis)
- for sla_miss_callback
+ # we have to distinguish between the two calls so that we can prepare
the correct context,
+ # comparing len(args) is one straightforward way of checking this.
+ if len(args) == 1:
+ _context = args[0]
Review Comment:
I spent time today with this, here's my suggestion:
Add sla_context in the list of callback args in dag_processing:
`callback(dag, task_list, blocking_task_list, slas, blocking_tis,
sla_context=context)`
The `context` should be from the `processor.py` module. There's a function
`ti.get_template_context(session=session)` that you can use to get the context
you pass to sla_context.
Also add all the args: `dag, task_list, blocking_task_list, slas,
blocking_tis` to the context that you got there.
So in base notifier, you do something like:
```python
def __call__(self, *args, sla_context=None) -> None:
"""
Send a notification.
:param context: The airflow context
"""
context = args if len(args) == 1 else None
if not context:
context = sla_context
...
```
I think the challenge is getting the ti to use to get the templates but it's
doable
--
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]
