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


##########
crates/iceberg/src/io/storage/mod.rs:
##########
@@ -139,4 +140,179 @@ pub trait StorageFactory: Debug + Send + Sync {
     /// A `Result` containing an `Arc<dyn Storage>` on success, or an error
     /// if the storage could not be created.
     fn build(&self, config: &StorageConfig) -> Result<Arc<dyn Storage>>;
+
+    /// Build a new Storage instance, optionally supplying a credential 
provider
+    /// that the backend can call to obtain and refresh short-lived 
credentials.
+    ///
+    /// Backends that cannot use the provider ignore it and use the credentials
+    /// in `config`, as they would without one. The default does exactly that.
+    #[allow(unused_variables)]
+    fn build_with_credential_provider(
+        &self,
+        config: &StorageConfig,
+        credential_provider: Option<Arc<dyn StorageCredentialProvider>>,
+    ) -> Result<Arc<dyn Storage>> {
+        self.build(config)
+    }
+}
+
+/// Supplies fresh, backend-specific storage credentials on demand.
+///
+/// A catalog that vends temporary credentials implements this trait so that
+/// storage backends can re-fetch credentials as they approach expiry instead
+/// of failing once the initial token's TTL runs out.
+///
+/// # Debug output
+///
+/// [`FileIO`](crate::io::FileIO) and storage implementations print the
+/// provider in their `Debug` output, so the provider's `Debug` implementation
+/// must not expose credentials or other secrets.
+///
+/// # Caching
+///
+/// [`load_credential`](Self::load_credential) may be called very frequently.
+/// Implementations must cache internally and only re-fetch when the current
+/// credential is at or near expiry; otherwise every object-store request could
+/// trigger a call back to the catalog.
+#[async_trait]
+pub trait StorageCredentialProvider: Debug + Send + Sync {
+    /// Return whether this provider supplies credentials for `path`.
+    ///
+    /// Backends replace their own credential chain with the provider only for
+    /// paths it supports, so a provider must return `false` for the storage it
+    /// does not serve, such as other schemes behind a resolving storage.
+    fn supports_path(&self, path: &str) -> bool;
+
+    /// Load a fresh credential for the storage location identified by `path`.
+    ///
+    /// `path` is an absolute location: usually the file being accessed, such
+    /// as `s3://bucket/warehouse/db/table/data/file.parquet`, but for a bulk
+    /// delete the location shared by a batch, which may be a storage root
+    /// (`s3://bucket/`) or the prefix of a previously returned credential
+    /// (`s3://bucket/warehouse/db/table`). Providers that vend distinct
+    /// credentials per location prefix use it to select the most specific
+    /// match, which must [cover](StorageCredential::covers) `path`.
+    ///
+    /// Backends read the credential's expiry from its config, such as
+    /// `s3.session-token-expires-at-ms`, and load a new one before it expires.
+    /// A credential without an expiry is used for as long as the backend 
lives.
+    async fn load_credential(&self, path: &str) -> Result<StorageCredential>;

Review Comment:
   done in #2976 (7586b8d11e9971f2cf5b3b5055d0732169f2ee48), will later update 
this PR with the latest changes in #2976



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