flyrain commented on code in PR #254:
URL: https://github.com/apache/iceberg-rust/pull/254#discussion_r1520276689


##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -626,6 +688,14 @@ mod _serde {
         }
     }
 
+    #[derive(Debug, Serialize, Deserialize)]
+    pub(super) struct TokenResponse {
+        pub(super) access_token: String,
+        pub(super) token_type: String,
+        pub(super) expires_in: u64,
+        pub(super) issued_token_type: String,

Review Comment:
   `issued_token_type` should be optional, here is a python fix for reference, 
https://github.com/apache/iceberg-python/pull/466



##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -497,13 +516,56 @@ impl RestCatalog {
             client: config.try_create_rest_client()?,
             config,
         };
-
+        catalog.fetch_access_token().await?;
+        catalog.client = catalog.config.try_create_rest_client()?;
         catalog.update_config().await?;
         catalog.client = catalog.config.try_create_rest_client()?;
 
         Ok(catalog)
     }
 
+    async fn fetch_access_token(&mut self) -> Result<()> {
+        if self.config.props.contains_key("token") {
+            return Ok(());
+        }
+        if let Some(credential) = self.config.props.get("credential") {
+            let (client_id, client_secret) = if credential.contains(':') {
+                let (client_id, client_secret) = 
credential.split_once(':').unwrap();
+                (Some(client_id), client_secret)
+            } else {
+                (None, credential.as_str())
+            };
+            let mut params = HashMap::with_capacity(4);
+            params.insert("grant_type", "client_credentials");
+            if let Some(client_id) = client_id {
+                params.insert("client_id", client_id);
+            }
+            params.insert("client_secret", client_secret);
+            params.insert("scope", "catalog");

Review Comment:
   `Scope` should be configurable, and we will also need optional configurable 
`audience` and `resource`. Here is the python PR for reference, 
https://github.com/apache/iceberg-python/pull/486. 
   
   I'd also recommend to group required fields and optional fields, so that the 
request is more extendable, which makes it easy to add new parameters in the 
future. Here are related discussion, 
https://github.com/apache/iceberg/pull/9839#discussion_r1511897858
   cc @himadripal



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to