potiuk commented on code in PR #23773:
URL: https://github.com/apache/airflow/pull/23773#discussion_r886614387
##########
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:
I think we cannot assume we have Metadata server around. This code should
work fine whether we have the metadada server or not. IMHO this method should
do what it says and return credentials when they are set.
Is there a way to actually check if the credentials we got at this place are
ACTUALLY empty? As I understand - we can get the credentials in many ways
before - from file, secret, keyfile dictionary or even using "google default"
authentication - which does not need refreshing (and that's why Breeze and
other places where no meta-data server is available will continue to work -
because they got credentials using one of those).
The credentials retrieved this way are perfectly valid. Raising error is not
good.
But there is one case that actually **should** throw an error here:
1) none of the methods above were configured
2) default authentication did not provide any authenticatio
3) refresh did not work
In this case, yeah - we should throw an error, as there is no sense
continuing and returnign "empty credentials" in this case does not help anyone
(and fail-early is the best approach for that).
So - can we throw an error when none-of the above methods actually returned
valid credentials and refresh did not do it either?
--
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]
