DerGut commented on code in PR #3081:
URL: https://github.com/apache/iceberg-rust/pull/3081#discussion_r4206630201


##########
crates/catalog/rest/src/auth/mod.rs:
##########
@@ -73,6 +75,26 @@ pub trait AuthManager: Debug + Send + Sync {
         client: &HttpClient,
         props: &HashMap<String, String>,
     ) -> Result<Arc<dyn AuthSession>>;
+
+    /// Returns the authentication session for a specific context.
+    ///
+    /// The catalog calls this method only after [`Self::catalog_session`] has
+    /// succeeded. `catalog_session` is the catalog session returned by this
+    /// manager. If the context does not require different authentication,
+    /// implementations should return `catalog_session` unchanged.
+    ///
+    /// The catalog does not cache the returned session. Implementations should
+    /// cache context-specific sessions internally using
+    /// [`SessionContext::session_id`] and are responsible for eviction and
+    /// releasing any associated resources. Reusing a session ID with different
+    /// context may therefore return the previously cached session.
+    async fn contextual_session(

Review Comment:
   You're right! With Rust we get a good default behavior already without an 
explicit API contract. I removed the comment about releasing resources in 
https://github.com/apache/iceberg-rust/pull/3081/changes/8d137319ee53d0497e2c4faa2019128030fb3d45



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