kosteev commented on code in PR #23773:
URL: https://github.com/apache/airflow/pull/23773#discussion_r877096966
##########
airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -248,6 +250,16 @@ def get_credentials_and_project(self) ->
Tuple[google.auth.credentials.Credentia
project_id =
_get_project_id_from_service_account_email(self.target_principal)
+ if isinstance(credentials, compute_engine.Credentials):
+ try:
+ credentials.refresh(_http_client.Request())
+ except RefreshError as msg:
+ """
+ If the Compute Engine metadata service can't be reached in
this case the instance has not
+ credentials.
+ """
+ self._log_debug(msg)
Review Comment:
This looks like a best-effort approach.
I we were able to retrieve credentials then good, otherwise also good but
credentials returned by method will be empty.
It means that the contract of the get_credentials_and_project method is that
sometimes it returns you credentials sometimes not, which is in general not a
good idea IMHO. At least we need to outline it in the docstring.
Or should we just fail and raise exception with proper message?, especially
if it is 4** error which is client error, not backend.
--
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]
