ratmice commented on code in PR #23413:
URL: https://github.com/apache/datafusion/pull/23413#discussion_r3550760106
##########
datafusion-cli/src/object_storage.rs:
##########
@@ -20,500 +20,553 @@ pub(crate) mod stdin;
pub use stdin::{StdinCarriesCommands, is_stdin_location};
-use async_trait::async_trait;
-use aws_config::BehaviorVersion;
-use aws_credential_types::provider::{
- ProvideCredentials, SharedCredentialsProvider, error::CredentialsError,
-};
use datafusion::{
- common::{
- config::ConfigEntry, config::ConfigExtension, config::ConfigField,
- config::ExtensionOptions, config::TableOptions, config::Visit,
config_err,
- exec_datafusion_err, exec_err,
- },
+ common::{config::TableOptions, exec_datafusion_err},
error::{DataFusionError, Result},
execution::context::SessionState,
};
-use log::debug;
-use object_store::{
- ClientOptions, CredentialProvider,
- Error::Generic,
- ObjectStore,
- aws::{AmazonS3Builder, AmazonS3ConfigKey, AwsCredential},
- gcp::GoogleCloudStorageBuilder,
- http::HttpBuilder,
-};
-use std::{
- any::Any,
- error::Error,
- fmt::{Debug, Display},
- sync::Arc,
-};
+use datafusion_common::exec_err;
Review Comment:
If you do want to remove this unused import complaint when compiling without
default features, it is possible to get rid of the clippy lint on this one with
a line like:
```suggestion
#[cfg(any(feature = "s3", feature = "gcs"))]
use datafusion_common::exec_err;
```
##########
datafusion-cli/src/catalog.rs:
##########
@@ -169,11 +173,13 @@ impl SchemaProvider for DynamicObjectStoreSchemaProvider {
// Register the store for this URL. Here we don't have access
// to any command options so the only choice is to use an
empty collection
match scheme {
+ #[cfg(feature = "s3")]
Review Comment:
Only way I could think of to disable this one was,
```
#[cfg_attr(not(any(feature = "s3", feature = "gcs")),
expect(unused_mut))]
let mut builder = SessionStateBuilder::from(state.clone());
```
##########
datafusion-cli/src/object_storage.rs:
##########
@@ -524,6 +577,7 @@ pub(crate) async fn get_object_store(
resolve_region: bool,
) -> Result<Arc<dyn ObjectStore>, DataFusionError> {
let store: Arc<dyn ObjectStore> = match scheme {
+ #[cfg(feature = "s3")]
Review Comment:
Not sure how to add the suggestion in the github UI, it sadly ends up as a
massive diff that doesn't look right. However, maybe you can fix these if you
feel it's worthwhile with the following additions to the `get_object_store`.
```
#[cfg_attr(not(any(feature = "s3", feature = "gcs")),
expect(unused_variables))]
table_options: &TableOptions,
#[cfg_attr(not(any(feature = "s3", feature = "gcs")),
expect(unused_variables))]
resolve_region: bool,
```
--
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]
