eladkal commented on a change in pull request #20263:
URL: https://github.com/apache/airflow/pull/20263#discussion_r772531440
##########
File path: airflow/providers/opsgenie/hooks/opsgenie_alert.py
##########
@@ -46,46 +51,50 @@ class OpsgenieAlertHook(HttpHook):
conn_type = 'opsgenie'
hook_name = 'Opsgenie'
- def __init__(self, opsgenie_conn_id: str = 'opsgenie_default', *args,
**kwargs) -> None:
- super().__init__(http_conn_id=opsgenie_conn_id, *args, **kwargs) #
type: ignore[misc]
+ def __init__(self, opsgenie_conn_id: str = 'opsgenie_default') -> None:
+ super().__init__() # type: ignore[misc]
+ self.conn_id = opsgenie_conn_id
+ configuration = Configuration()
+ conn = self.get_connection(self.conn_id)
+ configuration.api_key['Authorization'] = conn.password
+ configuration.host = conn.host or 'https://api.opsgenie.com'
+ self.alert_api_instance = AlertApi(ApiClient(configuration))
def _get_api_key(self) -> str:
- """Get Opsgenie api_key for creating alert"""
- conn = self.get_connection(self.http_conn_id)
- api_key = conn.password
- if not api_key:
- raise AirflowException(
- 'Opsgenie API Key is required for this hook, please check your
conn_id configuration.'
- )
- return api_key
-
- def get_conn(self, headers: Optional[dict] = None) -> requests.Session:
"""
- Overwrite HttpHook get_conn because this hook just needs base_url
- and headers, and does not need generic params
+ Get the API key from the connection
- :param headers: additional headers to be passed through as a dictionary
- :type headers: dict
+ :return: API key
+ :rtype: str
"""
- conn = self.get_connection(self.http_conn_id)
- self.base_url = conn.host if conn.host else 'https://api.opsgenie.com'
- session = requests.Session()
- if headers:
- session.headers.update(headers)
- return session
-
- def execute(self, payload: Optional[dict] = None) -> Any:
+ conn = self.get_connection(self.conn_id)
+ return conn.password
+
+ def get_conn(self) -> AlertApi:
+ """
+ Get the underlying AlertApi client
+
+ :return: AlertApi client
+ :rtype: opsgenie_sdk.AlertApi
+ """
+ return self.alert_api_instance
+
+ def execute(self, payload: Optional[dict] = None) -> SuccessResponse:
Review comment:
I would also say that this is a very good opportunity to change this
function name.
execute isn't self explanatory and also the sdk itself doesn't have it. If
in the future we will add more functions to the hook it would be harder to
understand what execute does.
I think this function should be called `create_alert`
--
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]
