drcrallen commented on a change in pull request #5492: Native parallel batch
indexing without shuffle
URL: https://github.com/apache/incubator-druid/pull/5492#discussion_r203101307
##########
File path:
extensions-core/kafka-indexing-service/src/main/java/io/druid/indexing/kafka/KafkaIndexTaskClient.java
##########
@@ -180,18 +113,17 @@ public boolean resume(final String id)
if (response.getStatus().equals(HttpResponseStatus.OK)) {
log.info("Task [%s] paused successfully", id);
- return jsonMapper.readValue(response.getContent(), new
TypeReference<Map<Integer, Long>>()
+ return deserialize(response.getContent(), new
TypeReference<Map<Integer, Long>>()
Review comment:
This and a few other comments are to see if the type contract can be
hardened a little bit. having type references with deeply nested types always
gets me worried for future changes where a change in a field type in a class
has unexpected knock on effects. When those fields are explicit in a class
somewhere, or the class is effectively an alias for the type, then it helps
prevent unexpected changes from showing up in odd places in the code.
It is easier to find `KafkaIndexCheckpointMap` than `Map<Integer, Long>`
when doing code grepping
((this suggestion is optional and more of a discussion so I can better
understand the limitations of where these values are intended to be used))
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
