zakariya-s commented on code in PR #2932:
URL: https://github.com/apache/iceberg-rust/pull/2932#discussion_r4185342996


##########
crates/iceberg/src/io/file_io.rs:
##########
@@ -123,14 +132,28 @@ impl FileIO {
     ///
     /// All storage configuration properties are included in the serialized 
representation. These
     /// properties may contain credentials or other sensitive values, so the 
returned bytes must be
-    /// protected in transit and at rest by the application embedding this 
crate.
+    /// protected in transit and at rest by the application embedding this 
crate. A serialized
+    /// credential provider may likewise carry catalog authentication and 
vended credentials.
     ///
     /// Storage factories are serialized through 
[`typetag`](https://docs.rs/typetag). Third-party
     /// factories must use `#[typetag::serde]` on their [`StorageFactory`] 
implementation.
+    ///
+    /// A credential provider is serialized as the
+    /// [`StorageCredentialProviderFactory`] returned by
+    /// [`StorageCredentialProvider::factory`], and rebuilt on 
deserialization. Serialization fails
+    /// when the provider cannot be rebuilt in another process; use
+    /// [`FileIO::without_credential_provider`] to serialize without it.
     pub fn serialize_all(&self) -> Result<Vec<u8>> {
+        let credential_provider = self
+            .credential_provider
+            .as_ref()
+            .map(|provider| provider.factory())
+            .transpose()?;

Review Comment:
   Sounds good, I'm happy with emitting a warning instead



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