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: [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]
