chenboat commented on a change in pull request #5221: Add a new server api for download of segments. URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r408586697
########## File path: pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java ########## @@ -58,8 +59,8 @@ protected static final String TABLE_NAME = "testTable"; private final Map<String, TableDataManager> _tableDataManagerMap = new HashMap<>(); - protected final List<ImmutableSegment> _indexSegments = new ArrayList<>(); - + protected final List<ImmutableSegment> _realtimeIndexSegments = new ArrayList<>(); Review comment: To me, this new API and its test provides downloading of immutable (or non-consuming) segments. So I agree there is a subset of realtime segments not covered here. We can either rename the API or add notes for this important subtlety. Given this API is so new and our main design is to download immutable segments, I added notes to the API to highlight this point. Since the code still allows immutable realtime segment downloads, I feel the test should also cover this case. ---------------------------------------------------------------- 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]
