kylebarron commented on code in PR #322:
URL:
https://github.com/apache/arrow-rs-object-store/pull/322#discussion_r2027966497
##########
src/gcp/client.rs:
##########
@@ -640,9 +632,11 @@ impl GetClient for GoogleCloudStorageClient {
request = request.query(&[("generation", version)]);
}
- if !credential.bearer.is_empty() {
- request = request.bearer_auth(&credential.bearer);
- }
+ request = request.with_bearer_auth(credential.as_deref());
+
+ // if !credential.bearer.is_empty() {
+ // request = request.bearer_auth(&credential.bearer);
+ // }
Review Comment:
Reading this code, it looks like previously if `credential.bearer` was `""`,
it would also skip signing the request, but only for `GET` requests. Should we
maintain that functionality? I.e. should we change the diff below to
```rs
impl CredentialExt for HttpRequestBuilder {
fn with_bearer_auth(self, credential: Option<&GcpCredential>) -> Self {
match credential {
Some(credential) => {
if credential.bearer.is_empty() {
return self;
}
self.bearer_auth(&credential.bearer)
}
None => self,
}
}
}
```
--
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]
