dwsmith1983 commented on code in PR #6059:
URL: https://github.com/apache/datafusion-comet/pull/6059#discussion_r4081330532


##########
native/core/src/parquet/objectstore/azure.rs:
##########
@@ -118,12 +133,92 @@ pub fn create_store(
             .map(|(k, _)| k.as_ref())
             .collect::<Vec<_>>()
     );
+
+    let provider_type =
+        account_scoped_value(configs, HADOOP_OAUTH_PROVIDER_TYPE, 
account.as_deref());
+    let store = build_builder(url, &translated, provider_type.as_deref(), 
env_pairs()).build()?;
+    Ok((Box::new(store), path))
+}
+
+/// Process environment as UTF-8 `(key, value)` pairs, skipping entries that 
are not UTF-8.
+fn env_pairs() -> impl Iterator<Item = (String, String)> {
+    std::env::vars_os()
+        .filter_map(|(k, v)| Some((k.to_str()?.to_string(), 
v.to_str()?.to_string())))
+}
+
+/// Assemble the builder from the environment, the URL and the translated 
Hadoop keys.
+///
+/// When the Hadoop keys configure an auth mechanism the environment is not 
consulted at
+/// all, so nothing ambient can outrank, combine with or redirect the 
configured identity.
+/// Otherwise the environment is read the way 
`MicrosoftAzureBuilder::from_env` reads it.
+/// `provider_type` is the resolved `fs.azure.account.oauth.provider.type`, if 
any.
+fn build_builder(
+    url: &Url,
+    translated: &[(AzureConfigKey, String)],
+    provider_type: Option<&str>,
+    env: impl Iterator<Item = (String, String)>,
+) -> MicrosoftAzureBuilder {
+    let mut builder = MicrosoftAzureBuilder::new();
+    if !hadoop_auth_present(translated, provider_type) {
+        builder = apply_env(builder, env);
+    }

Review Comment:
   > Could we return a configuration error at that boundary and add a 
regression checking the final provider/error selection?
   
   In 2b044b483. A client secret or token file without the client id and tenant 
is a configuration error naming the missing keys, raised before the builder can 
select IMDS, and the tests pin both the error text and that no provider is 
chosen for that configuration.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to