huan233usc commented on code in PR #2794:
URL: https://github.com/apache/iceberg-rust/pull/2794#discussion_r3609952341
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -431,14 +434,51 @@ impl RestCatalog {
.get_or_try_init(|| async {
let client = HttpClient::new(&self.user_config)?;
let catalog_config = RestCatalog::load_config(&client,
&self.user_config).await?;
+ // Use the advertised endpoints as-is, falling back to
+ // `DEFAULT_ENDPOINTS` when absent or empty.
+ let endpoints = match &catalog_config.endpoints {
+ Some(advertised) if !advertised.is_empty() => {
+ advertised.iter().cloned().collect()
+ }
+ _ => crate::endpoint::DEFAULT_ENDPOINTS.clone(),
+ };
let config =
self.user_config.clone().merge_with_config(catalog_config);
let client = client.update_with(&config)?;
- Ok(RestContext { config, client })
+ Ok(RestContext {
+ config,
+ client,
+ endpoints,
+ })
})
.await
}
+ /// Returns whether the server supports `endpoint`, per the `endpoints` it
+ /// advertised in `GET /v1/config` (or a default base set when it
advertised
+ /// none).
+ pub(crate) async fn supports_endpoint(&self, endpoint: &Endpoint) ->
Result<bool> {
+ Ok(self.context().await?.endpoints.contains(endpoint))
+ }
+
+ /// Issue a `HEAD` request to `url` and interpret it as an existence check:
+ /// `2xx` means it exists, `404` means it doesn't.
+ async fn check_exists_via_head(&self, url: String) -> Result<bool> {
Review Comment:
Done — `check_exists_via_head` now takes `&RestContext`, and
`{table,namespace}_exists` fetch context once.
##########
crates/catalog/rest/src/endpoint.rs:
##########
@@ -128,6 +142,53 @@ impl<'de> Deserialize<'de> for Endpoint {
}
}
+/// The standard v1 endpoints assumed to be supported when a server's
+/// `GET /v1/config` response omits the `endpoints` field or sends an empty
list
+/// (the two are treated alike). These are the minimum a server is expected to
+/// support; a server that advertises a non-empty list is taken at its word
+/// instead.
+pub(crate) static DEFAULT_ENDPOINTS: LazyLock<HashSet<Endpoint>> =
LazyLock::new(|| {
Review Comment:
Adopted the `endpoints!` macro and named `LazyLock` constants;
`DEFAULT_ENDPOINTS` is built from them.
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -648,24 +688,22 @@ impl Catalog for RestCatalog {
}
async fn namespace_exists(&self, ns: &NamespaceIdent) -> Result<bool> {
- let context = self.context().await?;
-
- let request = context
- .client
- .request(Method::HEAD, context.config.namespace_endpoint(ns))
- .build()?;
-
- let http_response = context.client.query_catalog(request).await?;
-
- match http_response.status() {
- StatusCode::NO_CONTENT | StatusCode::OK => Ok(true),
- StatusCode::NOT_FOUND => Ok(false),
- _ => Err(deserialize_unexpected_catalog_error(
- http_response,
- context.client.disable_header_redaction(),
- )
- .await),
+ let head_endpoint = Endpoint::new(Method::HEAD,
"/v1/{prefix}/namespaces/{namespace}");
Review Comment:
Switched to the static `V1_NAMESPACE_EXISTS` constant.
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -909,24 +947,24 @@ impl Catalog for RestCatalog {
/// Check if a table exists in the catalog.
async fn table_exists(&self, table: &TableIdent) -> Result<bool> {
- let context = self.context().await?;
-
- let request = context
- .client
- .request(Method::HEAD, context.config.table_endpoint(table))
- .build()?;
-
- let http_response = context.client.query_catalog(request).await?;
-
- match http_response.status() {
- StatusCode::NO_CONTENT | StatusCode::OK => Ok(true),
- StatusCode::NOT_FOUND => Ok(false),
- _ => Err(deserialize_unexpected_catalog_error(
- http_response,
- context.client.disable_header_redaction(),
- )
- .await),
+ let head_endpoint = Endpoint::new(
+ Method::HEAD,
+ "/v1/{prefix}/namespaces/{namespace}/tables/{table}",
+ );
Review Comment:
Switched to the static `V1_TABLE_EXISTS` constant.
--
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]
