[GitHub] [iceberg] nastra commented on a diff in pull request #5407: Core: Add RESTScanReporter to send scan report to REST endpoint

[GitBox <gi...@apache.org>]

nastra commented on code in PR #5407:
URL: https://github.com/apache/iceberg/pull/5407#discussion_r962646966


##########
core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java:
##########
@@ -74,4 +74,16 @@ public String table(TableIdentifier ident) {
   public String rename() {
     return SLASH.join("v1", prefix, "tables", "rename");
   }
+
+  public String scanReport(TableIdentifier identifier) {
+    return SLASH.join(
+        "v1",
+        prefix,
+        "namespaces",
+        RESTUtil.encodeNamespace(identifier.namespace()),
+        "tables",
+        RESTUtil.encodeString(identifier.name()),
+        "metrics",
+        "scan");

Review Comment:
   see my comment in https://github.com/apache/iceberg/pull/5407#discussion_r940060609 about the initial reasoning for a `/metrics/scan` endpoint (rather than just `/metrics`).
   Obviously we could post simply to `/metrics` with a type but eventually you might want to also do a `GET` and retrieve metrics of a particular type, so using `/metrics/scan` seems more in-line with how REST would model post/get requests.
   
   Example: We could have a `/metrics/commit` endpoint where we'd `POST` commit metrics to and could also perform a `GET` to retrieve them
   
   I understand the reasoning for this, since we'd like to keep changes to the REST spec to a minimum. I don't have a strong opinion on this, but I'd like us to at least consider this. IMO this is a bit cleaner and aligns better with REST to have it this way. However, I'm interested in what others think about this



-- 
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-unsubscribe@iceberg.apache.org

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


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