plusplusjiajia commented on code in PR #2651:
URL: https://github.com/apache/iceberg-rust/pull/2651#discussion_r4400726974
##########
crates/iceberg/src/io/storage/config/mod.rs:
##########
@@ -56,13 +56,17 @@ use serde::{Deserialize, Serialize};
pub struct StorageConfig {
Review Comment:
Done, `StorageConfig` now derives `TypedBuilder`.
##########
crates/iceberg/src/io/storage/config/mod.rs:
##########
@@ -121,6 +128,64 @@ impl StorageConfig {
.extend(props.into_iter().map(|(k, v)| (k.into(), v.into())));
self
}
+
+ /// Get the credentials that apply to locations under their prefixes.
+ pub fn storage_credentials(&self) -> &[StorageCredential] {
+ &self.storage_credentials
+ }
+
+ /// Add credentials that apply to locations under their prefixes.
+ ///
+ /// A factory that supports them overlays the props with the config of a
+ /// location's longest matching credential.
+ pub fn with_storage_credentials(
Review Comment:
Removed. I kept `FileIOBuilder::with_storage_credentials`, since catalogs
build the config through `FileIOBuilder`, which doesn't take a `StorageConfig`.
Happy to change that too if you prefer.
##########
crates/iceberg/src/io/storage/config/mod.rs:
##########
@@ -121,6 +128,64 @@ impl StorageConfig {
.extend(props.into_iter().map(|(k, v)| (k.into(), v.into())));
self
}
+
+ /// Get the credentials that apply to locations under their prefixes.
+ pub fn storage_credentials(&self) -> &[StorageCredential] {
+ &self.storage_credentials
+ }
+
+ /// Add credentials that apply to locations under their prefixes.
+ ///
+ /// A factory that supports them overlays the props with the config of a
+ /// location's longest matching credential.
+ pub fn with_storage_credentials(
+ mut self,
+ credentials: impl IntoIterator<Item = StorageCredential>,
+ ) -> Self {
+ self.storage_credentials.extend(credentials);
+ self
+ }
+}
+
+/// Storage properties, such as a vended credential, that apply to locations
+/// under a prefix, like Java's `StorageCredential`.
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct StorageCredential {
+ /// Location prefix, such as `s3://bucket/table`
+ prefix: String,
+ /// Storage properties holding the credential
+ config: HashMap<String, String>,
+}
+
+impl StorageCredential {
+ /// Create a credential for locations under `prefix`. An empty prefix
covers none.
+ pub fn new(prefix: impl Into<String>, config: HashMap<String, String>) ->
Self {
+ Self {
+ prefix: prefix.into(),
+ config,
+ }
+ }
+
+ /// Return the location prefix this credential applies to.
+ pub fn prefix(&self) -> &str {
+ &self.prefix
+ }
+
+ /// Return the storage properties holding the credential.
+ pub fn config(&self) -> &HashMap<String, String> {
+ &self.config
+ }
+}
+
+// The config values are secrets.
+impl std::fmt::Debug for StorageCredential {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let config_keys: BTreeSet<_> = self.config.keys().collect();
Review Comment:
Done, it now prints `self.config.keys()` directly.
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -987,6 +987,7 @@ impl RestSessionCatalog {
&self,
metadata_location: Option<&str>,
extra_config: Option<HashMap<String, String>>,
+ storage_credentials: Option<Vec<StorageCredential>>,
Review Comment:
Done, `LoadTableResult` now uses `iceberg::io::StorageCredential`. The JSON
shape is unchanged.
--
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]
