nazq commented on code in PR #531:
URL:
https://github.com/apache/arrow-rs-object-store/pull/531#discussion_r2499080110
##########
src/gcp/builder.rs:
##########
@@ -491,8 +491,16 @@ impl GoogleCloudStorageBuilder {
};
// Then try to initialize from the application credentials file, or
the environment.
- let application_default_credentials =
-
ApplicationDefaultCredentials::read(self.application_credentials_path.as_deref())?;
+ // Only fail on ADC errors if no explicit credentials were provided
+ let application_default_credentials = if
service_account_credentials.is_none()
+ && self.credentials.is_none() {
+ // No explicit credentials, so ADC errors should be propagated
+
ApplicationDefaultCredentials::read(self.application_credentials_path.as_deref())?
+ } else {
+ // We have explicit credentials, so ignore ADC errors
+
ApplicationDefaultCredentials::read(self.application_credentials_path.as_deref())
+ .unwrap_or(None)
Review Comment:
Good catch! You're absolutely right - there's no need to attempt reading ADC
at all when explicit credentials are already provided.
I've updated the code to skip the ADC read entirely in that case, changing:
```rust
} else {
ApplicationDefaultCredentials::read(self.application_credentials_path.as_deref())
.unwrap_or(None)
}
```
to:
```rust
} else {
// Explicit credentials provided, skip ADC reading entirely
None
}
```
This is cleaner, more efficient (no unnecessary file I/O)
--
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]
