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


##########
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(
+        &self,
+        _context: &SessionContext,

Review Comment:
   Good point! Addresed it in 
https://github.com/apache/iceberg-rust/pull/3081/changes/db6d1069689b243dfaaf2015c06f21526b35753a



##########
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(
+        &self,
+        _context: &SessionContext,

Review Comment:
   Good point! Addressed it in 
https://github.com/apache/iceberg-rust/pull/3081/changes/db6d1069689b243dfaaf2015c06f21526b35753a



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