eladkal commented on code in PR #47563:
URL: https://github.com/apache/airflow/pull/47563#discussion_r2000289281
##########
providers/apache/kafka/src/airflow/providers/apache/kafka/hooks/base.py:
##########
@@ -70,6 +69,15 @@ def get_conn(self) -> Any:
and bootstrap_servers.find("cloud.goog") != -1
and bootstrap_servers.find("managedkafka") != -1
):
+ try:
+ from airflow.providers.google.cloud.hooks.managed_kafka import
ManagedKafkaHook
+ except ImportError:
+ from airflow.exceptions import
AirflowOptionalProviderFeatureException
+
+ raise AirflowOptionalProviderFeatureException(
+ "Failed to import ManagedKafkaHook. For using this
functionality google provider version "
+ ">= 14.1.0 should be pre-installed."
+ )
Review Comment:
I don't have time to dive deep into the architecture issues I raised the
problem only from release manager prospective. Using Kafka classes should not
force installing Google provider and should not raise Google related exceptions
unless we know the user is trying to integrate with Google. Any solution that
achieves that is fine from that perspective. The current code does not achieve
that, it assumes if Google provider is not installed then we raise exception
asking the users to install it regardless if they are Google users or not.
Maybe @o-nikolas and the AWS team would have comments on the specifics
should AWS would want to do something similar.
--
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]
