TennyZhuang commented on code in PR #254:
URL: https://github.com/apache/iceberg-rust/pull/254#discussion_r1521635258
##########
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:
> what I mean is that is it required that update_config happens before
fetch_access_token?
`fetch_access_token` happens before `update_config` in my code, and also in
the python code.
Some config API also need authentication, such as
`https://api.tabular.io/ws/v1/config`
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -144,13 +161,15 @@ impl HttpClient {
.with_source(e)
})?)
} else {
+ let code = resp.status();
let text = resp.bytes().await?;
let e = serde_json::from_slice::<E>(&text).map_err(|e| {
Error::new(
ErrorKind::Unexpected,
"Failed to parse response from rest catalog server!",
)
.with_context("json", String::from_utf8_lossy(&text))
+ .with_context("code", code.to_string())
Review Comment:
`execute` already had that. It's inconsistent before the PR.
##########
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:
Anyway, the logic is same as a specified python version, I think it's
acceptable.
--
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
