shahar1 commented on code in PR #59535:
URL: https://github.com/apache/airflow/pull/59535#discussion_r2715745076
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -597,10 +597,14 @@ def _get_sql_proxy_download_url(self):
return download_url
def _get_credential_parameters(self) -> list[str]:
- extras =
GoogleBaseHook.get_connection(conn_id=self.gcp_conn_id).extra_dejson
+ connection = GoogleBaseHook.get_connection(conn_id=self.gcp_conn_id)
+ extras = connection.extra_dejson
key_path = get_field(extras, "key_path")
keyfile_dict = get_field(extras, "keyfile_dict")
- if key_path:
+ if connection.login:
+ # if the connection already has an associated user account, then
we can use auto auth
+ credential_params = ["--auto-iam-authn", self.gcp_conn_id]
Review Comment:
From a user's prespective it might be a breaking change if the keyfile is
associated with one user and the IAM with another, so we need to be a bit
cautious here. Nontheless, since IAM is safer choice than keyfiles which makes
sense a a future default behavior - I suggest to do the following:
1. Make the `if connection.login` as the fallback option of this `if` (at
the bottom).
2. Add a deprecation warning to all other statements that that the default
behavior will use the IAM starting in the next major version.
Then, after merging this PR - you could already prepare a PR for breaking
this beahvior (making the `connection.login` as default and remove the
deprecation warnings), and we could merge it after the upcoming release.
--
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]
