leventov commented on a change in pull request #8110: Response context
refactoring
URL: https://github.com/apache/incubator-druid/pull/8110#discussion_r305919720
##########
File path:
processing/src/main/java/org/apache/druid/query/spec/SpecificSegmentQueryRunner.java
##########
@@ -151,12 +150,13 @@ public void close() throws IOException
);
}
- private void appendMissingSegment(Map<String, Object> responseContext)
+ private void appendMissingSegment(ResponseContext responseContext)
{
- List<SegmentDescriptor> missingSegments = (List<SegmentDescriptor>)
responseContext.get(Result.MISSING_SEGMENTS_KEY);
+ List<SegmentDescriptor> missingSegments =
Review comment:
If `ResponseContext` exposed `computeIfAbsent()` this block might look like
```java
responseContext
.computeIfAbsent(ResponseContext.CTX_MISSING_SEGMENTS, k -> new
ArrayList<>())
.add(specificSpec.getDescriptor());
```
Unless I miss something.
Generally, yes, I see a value in moving methods into `ResponseContext`
because the keys are already defined there. It doesn't seem to me like "god
object" or leaky abstraction antipattern, at least not anymore than it is
already. But this is not a strong opinion, so feel free to leave things as is
if you think otherwise.
----------------------------------------------------------------
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.
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]
