pierrejeambrun commented on code in PR #23442:
URL: https://github.com/apache/airflow/pull/23442#discussion_r867219719
##########
airflow/providers/opsgenie/hooks/opsgenie.py:
##########
@@ -126,7 +126,7 @@ def close_alert(
identifier=identifier,
identifier_type=identifier_type,
close_alert_payload=close_alert_payload,
- kwargs=kwargs,
+ **kwargs,
Review Comment:
Hello guys,
Indeed we need to unpack and give extra params to the
`AlertApi.close_alert`. The issue here is by not unpacking when calling the
function, and giving directly `kwargs=kwargs`. (as a standard argument). It
gets collected as `kwargs={kwargs: kwargs}`, which is not what we want.
(`async_rec` is in the nested `kwargs` value dict and therefore ignored)
Unpacking like this should fix the issue.
I think we should change how the parent function pass values to the kwargs
parameter as it is a little bit confusing and unconventional.
```
# in OpsgenieCloseAlertOperator.execute
self.hook.close_alert(
identifier=self.identifier,
identifier_type=self.identifier_type,
payload=self._build_opsgenie_close_alert_payload(),
**self.close_alert_kwargs,
)
```
```
# In OpsgenieAlertHook.close_alert
def close_alert(
self,
identifier: str,
identifier_type: Optional[str] = 'id',
payload: Optional[dict] = None,
**kwargs,
) -> SuccessResponse:
```
The test needs to be updaded, I think we want it to be called like:
```
close_alert_mock.assert_called_once_with(
identifier=identifier,
identifier_type=identifier_type,
close_alert_payload=CloseAlertPayload(**pay_load),
async_req=True
)
```
--
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]
