moiseenkov commented on code in PR #39873:
URL: https://github.com/apache/airflow/pull/39873#discussion_r1624339226
##########
airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -350,6 +365,24 @@ def _get_credentials_using_credential_config_file(self):
return credentials, project_id
+ def _get_credentials_using_credential_config_file_and_token_supplier(self):
+ self._log_info(
+ "Getting connection using credential configuration file and
external Identity Provider."
+ )
+
+ if not self.credential_config_file:
+ raise AirflowException(
+ "Credential configuration is needed to use authentication by
External Identity Provider."
+ )
Review Comment:
I think it'd be very nice if the error message also explicitly provided an
attribute name that must be supplied here, so users were able to figure it out
without looking into the implementation.
##########
airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -419,3 +452,18 @@ def
_get_project_id_from_service_account_email(service_account_email: str) -> st
raise AirflowException(
f"Could not extract project_id from service account's email:
{service_account_email}."
)
+
+
+def
_get_info_from_credential_configuration_file(credential_configuration_file):
+ if isinstance(credential_configuration_file, str) and
os.path.exists(credential_configuration_file):
+ with open(credential_configuration_file) as file_obj:
+ try:
+ info = json.load(file_obj)
+ except ValueError:
+ raise AirflowException("Credentials Configuration File is not
a valid json file.")
+ else:
+ try:
+ info = json.loads(credential_configuration_file)
+ except json.decoder.JSONDecodeError:
+ raise AirflowException("Invalid JSON.")
Review Comment:
If I understand it correctly, the `credential_configuration_file` can be
`str` or `dict[str, str]`. How would it work in the case of a dictionary?
And this part is also missing unit test.
##########
airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -419,3 +452,18 @@ def
_get_project_id_from_service_account_email(service_account_email: str) -> st
raise AirflowException(
f"Could not extract project_id from service account's email:
{service_account_email}."
)
+
+
+def
_get_info_from_credential_configuration_file(credential_configuration_file):
Review Comment:
Could you please add type hint for the argument and the return type here?
--
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]
