mbutrovich commented on code in PR #2976:
URL: https://github.com/apache/iceberg-rust/pull/2976#discussion_r4210146424
##########
crates/iceberg/src/io/storage/local_fs.rs:
##########
@@ -318,6 +318,16 @@ impl StorageFactory for LocalFsStorageFactory {
fn build(&self, _config: &StorageConfig) -> Result<Arc<dyn Storage>> {
Ok(Arc::new(LocalFsStorage::new()))
}
+
+ /// Local filesystem storage needs no credentials.
+ #[allow(unused_variables)]
+ fn build_with_credential_provider(
+ &self,
+ config: &StorageConfig,
+ credential_provider: Option<Arc<dyn StorageCredentialProvider>>,
Review Comment:
Could this name the parameter `_credential_provider` instead of using
`#[allow(unused_variables)]`? `build` just above already marks its unused
parameter as `_config`. The attribute also covers the whole function, so it
would hide any unused variable added here later.
`crates/iceberg/public-api.txt` would pick up the new parameter name.
```suggestion
fn build_with_credential_provider(
&self,
config: &StorageConfig,
_credential_provider: Option<Arc<dyn StorageCredentialProvider>>,
```
##########
crates/iceberg/src/io/storage/memory.rs:
##########
@@ -242,6 +242,16 @@ impl StorageFactory for MemoryStorageFactory {
fn build(&self, _config: &StorageConfig) -> Result<Arc<dyn Storage>> {
Ok(Arc::new(MemoryStorage::new()))
}
+
+ /// In-memory storage needs no credentials.
+ #[allow(unused_variables)]
+ fn build_with_credential_provider(
+ &self,
+ config: &StorageConfig,
+ credential_provider: Option<Arc<dyn StorageCredentialProvider>>,
Review Comment:
Could this also use `_credential_provider` in place of
`#[allow(unused_variables)]`, to match `build(&self, _config: ...)` above?
```suggestion
fn build_with_credential_provider(
&self,
config: &StorageConfig,
_credential_provider: Option<Arc<dyn StorageCredentialProvider>>,
```
--
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]
