Copilot commented on code in PR #3376:
URL: https://github.com/apache/iceberg-rust/pull/3376#discussion_r4226436182


##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1064,14 +1068,54 @@ impl SessionCatalog for RestSessionCatalog {
 
     async fn update_namespace(
         &self,
-        _context: &SessionContext,
-        _namespace: &NamespaceIdent,
-        _properties: HashMap<String, String>,
+        context: &SessionContext,
+        namespace: &NamespaceIdent,
+        properties: HashMap<String, String>,
     ) -> Result<()> {
-        Err(Error::new(
-            ErrorKind::FeatureUnsupported,
-            "Updating namespace not supported yet!",
-        ))
+        let current = self.get_namespace(context, namespace).await?;
+        let removals = current
+            .properties()
+            .keys()
+            .filter(|key| !properties.contains_key(*key))
+            .cloned()
+            .collect_vec();
+        let updates: HashMap<String, String> = properties
+            .into_iter()
+            .filter(|(key, value)| current.properties().get(key) != 
Some(value))
+            .collect();
+
+        if removals.is_empty() && updates.is_empty() {
+            return Ok(());
+        }
+
+        let client = self.client().await?;
+
+        let request = HttpRequest::build(
+            client
+                .http_client
+                .request(
+                    Method::POST,
+                    client.config.namespace_properties_endpoint(namespace),
+                )
+                .json(&UpdateNamespacePropertiesRequest {
+                    removals: Some(removals),
+                    updates,
+                }),
+        )?;
+
+        let http_response = client.query_catalog(context, request).await?;
+
+        match http_response.status() {
+            StatusCode::OK => Ok(()),
+            StatusCode::NOT_FOUND => Err(Error::new(
+                ErrorKind::NamespaceNotFound,
+                "Tried to update a namespace that does not exist",
+            )),
+            _ => Err(deserialize_unexpected_catalog_error(
+                http_response,
+                client.http_client.disable_header_redaction(),
+            )),

Review Comment:
   A 422 currently falls through to `deserialize_unexpected_catalog_error`, 
which always produces `ErrorKind::Unexpected`. Issue #3339 explicitly requires 
invalid property updates to surface as `DataInvalid`; deserialize the REST 
error model in an explicit `UNPROCESSABLE_ENTITY` arm and add a regression test 
for the error kind.



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