comphead commented on code in PR #7127:
URL: https://github.com/apache/datafusion-comet/pull/7127#discussion_r4332092549
##########
native/core/src/parquet/objectstore/azure.rs:
##########
@@ -120,9 +122,12 @@ pub fn create_store(
url: &Url,
configs: &HashMap<String, String>,
) -> Result<(Box<dyn ObjectStore>, Path), object_store::Error> {
- let (scheme, path) = ObjectStoreScheme::parse(url)?;
+ let (scheme, path) = ObjectStoreScheme::parse(&without_credentials(url))?;
if scheme != ObjectStoreScheme::MicrosoftAzure {
- return Err(config_error(format!("Scheme of URL is not Azure: {url}")));
+ return Err(config_error(format!(
+ "Scheme of URL is not Azure: {}",
+ redacted(url)
+ )));
Review Comment:
`create_store` is only called from `parquet_support.rs` after
`is_azure_scheme`, and `ObjectStoreScheme::parse` returns `MicrosoftAzure` for
`abfs` and `abfss` URLs with a host, so I could not find a way for this `Scheme
of URL is not Azure` branch to fire from that caller. The `redacted` helper
exists only for it, along with the `s3://` and `foo://` rows of
`create_store_errors_do_not_echo_userinfo_or_query` and part of
`create_store_checks_the_scheme_and_returns_the_container_path`. Is the branch
kept because `create_store` is `pub`? If not, naming only the scheme here would
let `redacted` go.
--
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]
