dondaum commented on code in PR #56326:
URL: https://github.com/apache/airflow/pull/56326#discussion_r2430193401
##########
providers/atlassian/jira/src/airflow/providers/atlassian/jira/hooks/jira.py:
##########
@@ -88,3 +115,77 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"hidden_fields": ["schema", "extra"],
"relabeling": {},
}
+
+
+class JiraAsyncHook(HttpAsyncHook):
Review Comment:
That's a good question. I also checked your implementations before deciding
to add a new `AsyncHook`.
First, I checked the Airflow codebase and found many other providers that
isolate `AsyncHooks` in their own class. E.g. Google provider, Apache Beam
provider, Microsoft provider, Apache Livy provider, http provider,
Secondly, it makes more sense for me to add a new class and use
`HttpAsyncHook` as a single (direct) parent class instead of mixing them in one
class and then having to use multiple class hierarchies e.g.
```Python
Class JiraHook(BaseHook, HttpAsyncHook):
pass
```
I think it's also easier to understand that we are reusing the
`HttpAsyncHook`, which is the class, and its implementation is closer to this
hook than to the `JiraHook`.
Third, but not applicable here, since we are using `HttpAsyncHook`, class
initialization may require different parameters or functionalities than we need
to provide.
For me there is no need to follow a standard, it depends. We can take both
approaches.
If a Python library (e.g. Slack) already provides an asynchronous interface,
I wouldn't create a new `AsyncClass` at all, but rather implement it similarly
to what you did with the SlackWebhook.
But I'm also very open to changes if the opposite approach makes more sense.
--
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]
