liurenjie1024 commented on code in PR #254:
URL: https://github.com/apache/iceberg-rust/pull/254#discussion_r1521174970
##########
crates/catalog/rest/tests/rest_catalog_test.rs:
##########
Review Comment:
Could you add some tests here to mock fetch token process?
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -113,6 +117,19 @@ impl RestCatalogConfig {
),
]);
+ if let Some(token) = self.props.get("token") {
Review Comment:
Fair enough.
##########
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()?;
Review Comment:
Sorry, I don't get your point. I know that we should recreate http client,
what I mean is that is it required that `update_config` happens before
`fetch_access_token`? Seems java does this in reverse order, so I guess it
doesn't matter.
##########
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(());
+ }
Review Comment:
Sorry, I still don't get your point. But currently we in fact don't refresh
tokens, so this logic is same.
--
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
