tustvold commented on code in PR #3109:
URL: https://github.com/apache/arrow-rs/pull/3109#discussion_r1028124654


##########
object_store/src/aws/mod.rs:
##########
@@ -561,6 +571,14 @@ impl AmazonS3Builder {
         let bucket = self.bucket_name.context(MissingBucketNameSnafu)?;
         let region = self.region.context(MissingRegionSnafu)?;
 
+        let clientbuilder = match self.proxy_url {

Review Comment:
   This currently only alters the client used to obtain credentials, it does 
not alter the client created in `S3Client::new` which likely needs the same 
treatment as `AzureClient`



##########
object_store/src/gcp/mod.rs:
##########
@@ -821,6 +847,7 @@ impl GoogleCloudStorageBuilder {
         // The cloud storage crate currently only supports authentication via
         // environment variables. Set the environment variable explicitly so
         // that we can optionally accept command line arguments instead.
+        println!("herereere");

Review Comment:
   ```suggestion
   ```



##########
object_store/src/gcp/mod.rs:
##########
@@ -790,19 +801,34 @@ impl GoogleCloudStorageBuilder {
             service_account_path,
             client,
             retry_config,
+            proxy_url,
         } = self;
 
         let bucket_name = bucket_name.ok_or(Error::MissingBucketName {})?;
         let service_account_path =
             service_account_path.ok_or(Error::MissingServiceAccountPath)?;
-        let client = client.unwrap_or_else(Client::new);
 
+        let client = match (proxy_url, client) {
+            (_, Some(client)) => client,
+            (Some(url), None) => {
+                let pr = Proxy::all(&url).map_err(|source| Error::ProxyUrl { 
source })?;
+                Client::builder()
+                    .proxy(pr)
+                    .build()
+                    .map_err(|source| Error::ProxyUrl { source })?
+            }
+            (None, None) => Client::new(),
+        };
+
+        println!("herereere");
         let credentials = reader_credentials_file(service_account_path)?;
+        println!("herereere");

Review Comment:
   ```suggestion
   ```



##########
object_store/src/gcp/mod.rs:
##########
@@ -790,19 +801,34 @@ impl GoogleCloudStorageBuilder {
             service_account_path,
             client,
             retry_config,
+            proxy_url,
         } = self;
 
         let bucket_name = bucket_name.ok_or(Error::MissingBucketName {})?;
         let service_account_path =
             service_account_path.ok_or(Error::MissingServiceAccountPath)?;
-        let client = client.unwrap_or_else(Client::new);
 
+        let client = match (proxy_url, client) {
+            (_, Some(client)) => client,
+            (Some(url), None) => {
+                let pr = Proxy::all(&url).map_err(|source| Error::ProxyUrl { 
source })?;
+                Client::builder()
+                    .proxy(pr)
+                    .build()
+                    .map_err(|source| Error::ProxyUrl { source })?
+            }
+            (None, None) => Client::new(),
+        };
+
+        println!("herereere");
         let credentials = reader_credentials_file(service_account_path)?;
+        println!("herereere");
 
         // TODO: 
https://cloud.google.com/storage/docs/authentication#oauth-scopes
         let scope = "https://www.googleapis.com/auth/devstorage.full_control";;
         let audience = 
"https://www.googleapis.com/oauth2/v4/token".to_string();
 
+        println!("herereere");

Review Comment:
   ```suggestion
   ```



##########
object_store/src/gcp/mod.rs:
##########
@@ -790,19 +801,34 @@ impl GoogleCloudStorageBuilder {
             service_account_path,
             client,
             retry_config,
+            proxy_url,
         } = self;
 
         let bucket_name = bucket_name.ok_or(Error::MissingBucketName {})?;
         let service_account_path =
             service_account_path.ok_or(Error::MissingServiceAccountPath)?;
-        let client = client.unwrap_or_else(Client::new);
 
+        let client = match (proxy_url, client) {
+            (_, Some(client)) => client,
+            (Some(url), None) => {
+                let pr = Proxy::all(&url).map_err(|source| Error::ProxyUrl { 
source })?;
+                Client::builder()
+                    .proxy(pr)
+                    .build()
+                    .map_err(|source| Error::ProxyUrl { source })?
+            }
+            (None, None) => Client::new(),
+        };
+
+        println!("herereere");

Review Comment:
   ```suggestion
   ```



##########
object_store/src/azure/client.rs:
##########
@@ -82,6 +82,9 @@ pub(crate) enum Error {
     Authorization {
         source: crate::azure::credential::Error,
     },
+
+    #[snafu(display("Unable to use proxy url: , {}", source))]

Review Comment:
   ```suggestion
       #[snafu(display("Unable to use proxy url: {}", source))]
   ```



-- 
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]

Reply via email to