arno-fukuda commented on code in PR #73646:
URL: https://github.com/apache/airflow/pull/73646#discussion_r4197886165
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -588,14 +603,42 @@ def __init__(
self.cloud_sql_proxy_socket_directory = self.path_prefix
self.sql_proxy_path = sql_proxy_binary_path or
f"{self.path_prefix}_cloud_sql_proxy"
self.credentials_path = self.path_prefix + "_credentials.json"
+ self._validate_sql_proxy_configuration()
self._build_command_line_parameters()
+ def _validate_sql_proxy_configuration(self) -> None:
+ # Validated here rather than in start_proxy(): callers stop the proxy
on failure, and
+ # stop_proxy() on a proxy that never started raises and hides the
original error.
+ if self.sql_proxy_major_version == 1:
+ if self.sql_proxy_version and
self.sql_proxy_version.lstrip("v").startswith("2."):
+ raise ValueError(
+ f"The sql_proxy_version {self.sql_proxy_version!r} is a
Cloud SQL Auth Proxy v2 "
+ "release. Set sql_proxy_major_version to 2 to use it!"
+ )
+ return
+ if not self.instance_specification:
+ raise ValueError(
+ "Cloud SQL Auth Proxy v2 does not support forwarding all
instances of a project. "
+ "The instance_specification must be provided!"
+ )
+ if not os.path.isfile(self.sql_proxy_path):
+ self._get_sql_proxy_download_url()
+
def _build_command_line_parameters(self) -> None:
+ if self.sql_proxy_major_version == 2:
+ self._build_v2_command_line_parameters()
+ return
self.command_line_parameters.extend(["-dir",
self.cloud_sql_proxy_socket_directory])
self.command_line_parameters.extend(["-instances",
self.instance_specification])
if self.sql_proxy_enable_iam_login:
self.command_line_parameters.append("-enable_iam_login")
+ def _build_v2_command_line_parameters(self) -> None:
+ self.command_line_parameters.extend(["--unix-socket",
self.cloud_sql_proxy_socket_directory])
+ if self.sql_proxy_enable_iam_login:
+ self.command_line_parameters.append("--auto-iam-authn")
+ self.command_line_parameters.append(self.instance_specification)
Review Comment:
Thanks, confirmed. v1 defaults to `-ip_address_types=PUBLIC,PRIVATE`, and v2
only uses the public IP unless an IP-type flag is set. Since v2 doesn't contact
the instance before logging that it's ready, the failure only showed up at
query time.
I went with the first option: the runner now always passes `--auto-ip` to
v2, and the connection docs explain why. The help text for `--auto-ip` says it
uses the "first IP address returned by the SQL Admin API". The Go connector
actually tries the public IP first and then the private one, which matches v1's
default. That means private-IP-only instances behave the same on both versions.
I left out a separate IP-type option (`--private-ip` / `--psc`) for now.
Airflow never exposed IP-type selection for v1, so it would be a new feature
rather than part of the migration. If it's needed later, it can be added with
auto as the default without breaking anything.
---
<sub>Drafted-by: Claude Code (Opus 5.5); reviewed by @arno-fukuda before
posting</sub>
--
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]
