Copilot commented on code in PR #66510:
URL: https://github.com/apache/airflow/pull/66510#discussion_r3213005347
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -852,12 +860,18 @@ def __init__(
self.use_proxy = self._get_bool(self.extras.get("use_proxy", "False"))
self.use_ssl = self._get_bool(self.extras.get("use_ssl", "False"))
self.use_iam = self._get_bool(self.extras.get("use_iam", "False"))
+ self.sql_proxy_enable_iam_login = self._get_bool(
+ self.extras.get("sql_proxy_enable_iam_login", "False")
+ )
self.sql_proxy_use_tcp =
self._get_bool(self.extras.get("sql_proxy_use_tcp", "False"))
self.sql_proxy_version = self.extras.get("sql_proxy_version")
self.sql_proxy_binary_path = sql_proxy_binary_path
if self.use_iam:
self.user = self._get_iam_db_login()
self.password =
self._generate_login_token(service_account=self.cloudsql_connection.login)
+ elif self.sql_proxy_enable_iam_login:
+ self.user = self._get_iam_db_login()
+ self.password = self.cloudsql_connection.password or ""
Review Comment:
In `__init__`, `_generate_login_token()` is called when `use_iam` is true
before input validation runs. With the new mutual-exclusivity rule (`use_iam`
vs `sql_proxy_enable_iam_login`), an invalid config can still trigger a `gcloud
sql generate-login-token` invocation (potentially slow/failing) before the
constructor raises. Consider validating these flag combinations (and
`sql_proxy_enable_iam_login` requiring `use_proxy`) before generating the
token, so invalid configurations fail fast without side effects.
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -989,6 +1003,12 @@ def _validate_inputs(self) -> None:
" SSL is not needed as Cloud SQL Proxy "
"provides encryption on its own"
)
+ if self.use_iam and self.sql_proxy_enable_iam_login:
+ raise ValueError(
+ "use_iam (direct IAM token) and sql_proxy_enable_iam_login
(proxy IAM) are mutually exclusive"
+ )
+ if self.sql_proxy_enable_iam_login and not self.use_proxy:
+ raise ValueError("sql_proxy_enable_iam_login requires use_proxy to
be True")
Review Comment:
`_validate_inputs()` raises `ValueError` for connection misconfiguration,
but the rest of this hook primarily raises `AirflowException` for invalid
connection settings. Using `AirflowException` here would be more consistent and
ensures the error is surfaced uniformly in Airflow task logs/UI.
--
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]
