pierrejeambrun commented on a change in pull request #20263:
URL: https://github.com/apache/airflow/pull/20263#discussion_r769521116



##########
File path: tests/providers/opsgenie/hooks/test_opsgenie_alert.py
##########
@@ -45,7 +45,7 @@ class TestOpsgenieAlertHook(unittest.TestCase):
             {'id': '80564037-1984-4f38-b98e-8a1f662df552', 'type': 'schedule'},
             {'name': 'First Responders Schedule', 'type': 'schedule'},
         ],
-        'visibleTo': [
+        'visible_to': [

Review comment:
       Indeed, the official sdk take different parameters (python style), so 
`visibleTo` becomes `visible_to` and use an internal mapping to construct the 
request. Now it's the same parameter as the corresponding operator.
   
   The API is expecting `visibleTo` but the operator was directly passing to 
the hook a 'visible_to' parameter, [see 
here](https://github.com/apache/airflow/blob/866a601b76e219b3c043e1dbbc8fb22300866351/airflow/providers/opsgenie/operators/opsgenie_alert.py#L122).
 To be honest I am not even sure the operator was working before with this 
parameter.
   The test for the hook was correct though.
   
   I will still upgrade the major version so it's not ambiguous, what do you 
thing ?
   
   More info in the 
[CreateAlertPayload](https://github.com/opsgenie/opsgenie-python-sdk/blob/master/opsgenie_sdk/api/alert/create_alert_payload.py#L50)




-- 
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]


Reply via email to