[GitHub] [druid] gianm commented on a diff in pull request #12220: Fix a few small errors in exception handling

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

gianm commented on code in PR #12220:
URL: https://github.com/apache/druid/pull/12220#discussion_r842182817


##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -723,12 +746,12 @@ public Response getTasks(
       createdTimeDuration = null;
     }
 
-    // Ideally, snapshotting in taskStorage and taskRunner should be done atomically,
+    // Ideally, snapshoting in taskStorage and taskRunner should be done atomically,

Review Comment:
   I think it was spelled correctly before?



##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -213,20 +216,30 @@ public Response apply(TaskQueue taskQueue)
               return Response.ok(ImmutableMap.of("task", task.getId())).build();
             }
             catch (EntryExistsException e) {
-              return Response.status(Response.Status.BAD_REQUEST)
-                             .entity(
-                                 ImmutableMap.of(
-                                     "error",
-                                     StringUtils.format("Task[%s] already exists!", task.getId())
-                                 )
-                             )
-                             .build();
+              return error(
+                  Response.Status.BAD_REQUEST,
+                  "Duplicate",

Review Comment:
   On the query side the `error` key is meant to be "well known" error code: https://druid.apache.org/docs/latest/querying/querying.html#query-errors
   
   Something like that on the Overlord side would be nice too. To encourage such standardization, how about creating an enum or a class-full-of-static-constants with all the possible error codes? That way, people updating or adding APIs won't be tempted to muck with the codes.



-- 
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: commits-unsubscribe@druid.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org